1

I'm working on a problem about modifying strings with dynamic memory allocation. The applicable parts of my code are as follows:

./dma 5
    #include <stdio.h>
    #include <stdlib.h>

char* strcopy(char* destination, char* source);
char *strconcat(char* destination, char* source);

int main(int argc, char *argv[]) {

int cmd, a=1, b, length_of_str, n, n2;
char* pstring[atoi(argv[1])];

for (b=0; b<atoi(argv[1]); b++) {
    printf("Enter the length of string %d: ", b+1);
    scanf("%d", &length_of_str);
    pstring[b]=(char *)malloc(length_of_str*sizeof(char));
    printf("Please enter string %d: ", b+1);
    scanf("%s", &pstring[b]);
}

while (a!=0) {
printf("Your strings are: \n");
for (b=0; b<atoi(argv[1]); b++) {
    printf("String number %d - \"%s\"\n", b+1, &pstring[b]);
}

printf("Options:\n");
printf("1 - Find string length\n");
printf("2 - Compare strings\n");
printf("3 - Copy strings\n");
printf("4 - Concatenate strings\n");
printf("5 - Quit\n");
printf("Please enter your option: ");
scanf("%d", &cmd);

switch (cmd) {

case 3:
    printf("Enter the number of the source string: ");
    scanf("%d", &n); 
    printf("Enter the number of the destination string: ");
    scanf("%d", &n2);
    strcopy(pstring[n-1], pstring[n2-1]);
    break;
case 4:
    printf("Enter the number of the source string: ");
    scanf("%d", &n); 
    printf("Enter the number of the destination string: ");
    scanf("%d", &n2);
    strconcat(pstring[n-1], pstring[n2-1]);
    break;
case 5:
    a=0;
    break;
default:
    printf("Invalid Option.\n");
    break;
}
}

free(pstring);
return 0; 
}
char* strcopy(char* destination, char* source) {
destination=(char *)realloc(*source, sizeof(char)*strlength(destination));
for (; *source!='\0'; source++) {
    *destination=*source;
    destination++;
    }
*destination='\0';
return destination;
}

char* strconcat(char* destination, char* source) {
destination=(char *)realloc(*source, sizeof(char)*strlength(destination));
for (; *destination!='\0'; destination++) {
    }
for (; *source!='\0'; source++) {
    *destination=*source;
    destination++;
    }
*destination='\0';
return destination;
}

I need to incorporate realloc into my concatenation and copy functions (which should be fine since they worked in a separate problem). I've tried a number of ways and I've tried different syntax but I only seem to get segmentation faults or invalid pointers. How exactly am I supposed to incorporate realloc? The intended result should look like this:

Your strings are:
String number 1 – “first”
String number 2 – “second”
String number 3 – “third”
String number 4 – “fourth”
String number 5 – “fifth”
Options:
1 – Find string length
2 – Compare strings
3 – Copy strings
4 – Concatenate strings
5 – Quit
Please enter your option: 3
Enter the number of the source string: 2
Enter the number of the destination string: 5
Your strings are:
String number 1 – “first”
String number 2 – “second”
String number 3 – “third”
String number 4 – “fourth”
String number 5 – “second”
Options:
1 – Find string length
2 – Compare strings
3 – Copy strings
4 – Concatenate strings
5 – Quit
Please enter your option:
Sinclair
  • 23
  • 4
  • The functions return a string, but your call of the function doesn't store the returned string. Also, your realloc in strconcat seems to make the source string the size of the destination string, instead of the size of both strings. – Hack Saw Dec 05 '18 at 07:07
  • Not to mention if `atoi(argv[1])` fails. `atoi` provides zero error reporting. Use `strtol` instead and validate the conversion before creating the VLA of pointers. – David C. Rankin Dec 05 '18 at 07:32

1 Answers1

0

One major problem is how you read the string:

scanf("%s", &pstring[b])

Here pstring[b] is of type char *, and due to the malloc call it points to some memory (unless malloc fails, which you forget to check for).

But &pstring[b] is a pointer to the pointer, and is of type char **. This is hardly what you want, and will cause scanf to write to memory somewhere it wasn't supposed to write, and can even write out of bounds of allocated memory. This of course leads to undefined behavior.

Once you solve that, you need to remember that char strings in C are really called null-terminated byte strings. The null-terminator is what all standard string functions look for to find the end of the string. Of course, that means that a string for x characters needs space for x + 1 to fit the terminator. So if the user says he or she want a string of length 6 (for example) and then give foobar as input, that needs space for 7 characters with the terminator. This terminator problem (or rather missing to allocate space for it) you have in other places as well (like the strcopy function).

Your scanf call also doesn't hinder the user to input a string longer that the user said. If the user said that the string should be 3 characters, but then enter foobar, that will write out of bounds. Unfortunately there's no way to solve this with only scanf, as the field width modifier must be part of the format string. You could use the scanf_s function which solves this problem, but it's not mandated by the C specification, and needs you to define a specific macro for it to be available if the implementation have it (see e..g this scanf and family reference for details). Otherwise you need to programatically construct the format string to include a field width.

You also do free(pstring) which is invalid, since you didn't allocate pstring itself, it's an array. You do need to loop over all the strings in the array pstring and free them. Trying to pass a pointer not returned by malloc (et al) to free also leads to undefined behavior.

Lastly, in C you should not cast the result of malloc (and related functions).

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621