Malloc, strlen, strcat

Question

The following is a piece of code from the begining of a program that I am writing (which contains an error).

    char *name;
    char *name2;

    if (argn != 2) {
        printf("You have to enter the name of the input file");
        return 1;
    }

    name = malloc(strlen(arg[1]) + 1);      
    name2 = malloc(strlen(arg[1]) + 1);

    strcpy(name, arg[1]); 
    strcpy(name2, arg[1]);

    strcat(name2, "-results.pdb");  

There is an error here which is with strcat, indeed name2 does not have enough size to carry out the operation above. Yet strcat executes without a problem. However later on in a completely unrelated part of the program an operation with another array that has been initialized after this strcat gives an error. It is an integer array for which I am assigning values to its elements and it gives an error before I can assign all the values. I am assuming that since there is not enough memory in name2 for the operation above this "somehow" affects the next arrays that are initialized. I would like to understand:

1- What is likely happening here so that the extra information that could not be written to name2 affects other arrays declared later?

2- I probably wouldn't be able to backtrace this problem so easily in a more complicated program since error occurs elsewhere rather than in strcat. How can I prevent this kind of sneaky errors like a memory problematic process affecting totally unrelated arrays elsewhere?

Answer 1

Yet strcat executes without a problem.

No it doesn't. It returns, but it has planted a time bomb. As you observe later on.

What happens is undefined behavior. You have written to memory you were not allowed to write. Whatever was stored there is now garbage and whatever code expects to find meaningful values there is now misbehaving. Especially if malloc internal data was corrupted the observation is random crashes when attempting to realloc or free memory later.

The correct way is to allocate the memory with

name2 = malloc(strlen(arg[1]) + sizeof "-results.pdb");

This takes care of the "+1" for the terminating NUL, since sizeof "-results.pdb" is 13.

Even easier is using asprintf (not ISO C but available on any contemporary Unix), which allocates the memory as needed:

asprintf(&name2, "%s-results.psb", arg[1]);

There! No strlen, no strcat, no sizeof, no malloc. Just an all-in-one call doing the Right Thing^TM.