Malloc, dynamically allocating wrong size memory for a string

Question

int main() {
    char A[200], B[200];
    printf("Enter 2 words or sentences.\n");
    gets(A);
    gets(B);
    char* C = (char*)malloc((strlen(A) + strlen(B)) * sizeof(char));
    for (int i = 0; i < strlen(A); i++)
        C[i] = A[i];
    for (int i = 0; i < strlen(B); i++)
        C[i + strlen(A)] = B[i];
    printf("%s", C);
}

The initial value of C is ÍÍÍÍÍÍÍÍÍÍýýýý, which is 4 symbols longer than requested and the 4 symbols also show up as output when printing C. I have no idea why there are 4 symbols, which is why I'm here seeking an explanation.

the memory returned by malloc contains junk, you have to set it to something, printf (I assume you are doing that) on unitiialized memory produces undefined behavior — pm100, Dec 15 '22 at 19:31
I give values for the first 10 symbols and print the whole string which I expect to contain just the 10 symbols. — Genata339, Dec 15 '22 at 19:33
since you dont show any code for a) loading values or b) printing its hard to work out what your issue is. My guess, you are using %s output and did not 0 terminate. PLease post all your code in the question — pm100, Dec 15 '22 at 19:39
Obligatory: [Why is the `gets` function so dangerous that it should not be used?](https://stackoverflow.com/q/1694036/2505965) — Oka, Dec 15 '22 at 20:01

chux - Reinstate Monica · Answer 1 · 2022-12-16T12:14:15.470

Problems:

Not null character terminated

Code attempts printf("%s", C); which is undefined behavior as C[] is not a string as needed by "%s".

// Append a \0
C[strlen(A) + strlen(B)] = '\0';

Insufficient memory allocated

Make room for the null character.

// (strlen(A) + strlen(B)) * sizeof(char)
strlen(A) + strlen(B) + 1

gets() is no longer part of the standard C library since C11

Use fgets() and lop off a potential trailing '\n' for similar behavior.

int vs. size_t

int is insufficient for very long strings. size_t works for all strings.

Avoid potentially recalculating the string length

int main(void) {
    char A[200], B[200];
    printf("Enter 2 words or sentences.\n");

    // Code should check the return value of fgets()
    // Omitted for brevity.
    fgets(A, sizeof A, stdin);

    A[strcspn(A, "\n")] = '\0'; // Lop off potential \n
    fgets(B, sizeof B, stdin);
    B[strcspn(B, "\n")] = '\0';

    size_t A_len = strlen(A);
    size_t B_len = strlen(B);
    char* C = malloc(A_len + B_len + 1);

    if (C) {
      for (size_t i = 0; A[i]; i++) {
        C[i] = A[i];
      }
      for (size_t i = 0; B[i]; i++) {
        C[A_len + i] = B[i];
      }
      C[A_len + B_len + i] = '\0';

      printf("%s\n", C);
      free(C);  // Good housekeeping to free allocations.
    }
}

Corner cases: if `fgets` fails, there's no telling what's in the buffer. — cHao, Dec 16 '22 at 12:11
@cHao Almost true. When `fgets(A)` returns `NULL` due to input error, `A[]` is indeterminate. When `fgets(A)` returns `NULL` due to end-of-file, `A[]` is unchanged. IAC, checking the return value of `fgets()` is more robust. — chux - Reinstate Monica, Dec 16 '22 at 12:13

cHao · Answer 2 · 2022-12-16T12:14:50.683

malloc just returns a pointer to some memory it allocated for you. It doesn't initialize that memory, zero it out, or anything like that. So what you're seeing when you print it out, is whatever junk was in there before.

Frankly, you're lucky you didn't open up a wormhole or something. C strings are nul-teminated, so when you pass that pointer around, you're technically not passing a string yet. When you pass it to a function that expects a string, all kinds of wackiness can ensue.

You should initialize the memory when you get it. The simplest initialization would be something like *C = '\0'; or C[0] = '\0';, which turns the memory into a zero-length string. But you probably already have something to put there, or why would you be allocating memory in the first place? :P

Now that there's code, we can tweak it a bit to fix the issue...

int main() {
    char A[200], B[200];
    printf("Enter 2 words or sentences.\n");

    // BTW: you should never, ever be using `gets`.
    // use `fgets` and pass the size of your buffer to avoid overruns.
    // note: it returns a null pointer if it fails...at which point you
    // can't trust that A and B are strings, and should probably bail
    if (!fgets(A, sizeof A, stdin)) return 1;
    if (!fgets(B, sizeof B, stdin)) return 1;

    // you don't want to call `strlen` over and over. save these lengths
    size_t Alen = strlen(A);
    size_t Blen = strlen(B);

    // lop off the newlines
    if (Alen && A[Alen - 1] == '\n') A[--Alen] = '\0';
    if (Blen && B[Blen - 1] == '\n') B[--Blen] = '\0';

    // You need enough space for both strings, plus a nul at the end.
    // side note: you don't need to cast the pointer to a `char *`.
    // also, sizeof(char) is 1 by definition, so no need to multiply by it.
    char* C = malloc(Alen + Blen + 1);
    if (!C) return 1;

    // compare to the length variable instead
    for (int i = 0; i < Alen; i++)
        C[i] = A[i];
    
    for (int i = 0; i < Blen; i++)
        C[i + Alen] = B[i];

    // important: nul-terminate the string
    C[Alen + Blen] = '\0';

    printf("%s", C);

    // not strictly necessary at the end of main on a modern OS, but
    // you should free what you malloc
    free(C);
}

@chux : ooo....crafty. Didn't even consider feeding a nul to stdin. :P Thanks — cHao, Dec 16 '22 at 12:16

score -1 · Answer 3 · answered Dec 15 '22 at 19:59

-1

C[strlen(A) + strlen(B)] = '\0'; after the malloc gets rid of the junk, thanks everyone for the ideas.

answered Dec 15 '22 at 19:59

Genata339

19
3

2

No it doesn't because you haven't allocated space for that array position. So you are accessing the array out of bounds, if it works it is by pure (un)luck. – Lundin Dec 16 '22 at 12:03

Malloc, dynamically allocating wrong size memory for a string

3 Answers3