patterncMinor
Simple string repeater program
Viewed 0 times
programsimplestringrepeater
Problem
I'm new to C, want to know if I'm doing this right.
Is there anything that could be otherwise improved? Am I handling memory correctly?
Review would be very appreciated.
//repstring.c
#include
#include
#include
int main(int argc, char **argv)
{
//Initialize variables and pointers
int i;
int loop;
char *deststr;
char *str;
//Change second argument to integer
loop = atoi(argv[2]);
//If user requests to repeat string more than 9999 times, terminate
if(loop > 9999)
{
printf("Error: Asked to repeat too many times.\n");
printf("Hardcoded maximum: 9999\n");
return 1;
}
if(argc == 3)
{
//Dynamically allocate memory to strings
deststr = malloc(1024 * 1024 * 1024 * sizeof(char));
str = malloc(1024 * 1024 * 1024 * sizeof(char));
if(str == NULL || deststr == NULL)
{
printf("Error allocating memory!\n");
free(deststr);
free(str);
return 1;
}
//Copy string in first argument to str
strncpy(str, argv[1], 50);
//Append str to deststr for amount of times specified in second argument
for(i = 0; i \n");
return 1;
}
return 0;
}Is there anything that could be otherwise improved? Am I handling memory correctly?
Review would be very appreciated.
Solution
-
Standard guarantees
-
-
Length of a destination string is
-
Standard guarantees
sizeof(char) to be 1.-
argv[1] is a perfectly good string. There is no need to copy it anywhere, let alone allocate a megabyte for it.-
Length of a destination string is
strlen(argv[1]) * loop + 1. Do not hardcode a megabyte.-
strcat has to find the end of destination string over and over again, making the execution time quadratic. Trace the end of the destination string as you go and strcpy to the right location instead.Context
StackExchange Code Review Q#121217, answer score: 3
Revisions (0)
No revisions yet.