Memory Management Command Line Arguments in C

Question

I am writing a simple program that takes the command line arguments and stores them into a char **. I am trying to learn more about memory management but cannot get past this simple stumbling block. My program is supposed to copy the command line argumetns into a dynamicly allocated char **. However the first position in my array is always corrupter. Below is the code and what it prints:

if (strcmp(argv[1], "test") ==0)
{
    test();
}
else
{
    char ** file_names = malloc(10);

    for(int i =0; i < argc-1; ++i)
    {
        file_names[i] = malloc(10);
        strcpy(file_names[i], argv[i+1]);

        printf("%s, %s\n", argv[i+1], file_names[i]);
    }

    printf("____________\n");

    for(int i =0; i < argc-1; ++i)
    {
        printf("%s\n", file_names[i]);
    }
}

and the out come is:

what, what
test, test
again, again
wow, wow
____________
pK@??
test
again
wow

can someone please explain why this is happening? Thanks

You most likely have *undefined behavior*, as for `file_names` you only allocate *ten bytes*, not space for ten pointers. — Some programmer dude, Mar 05 '15 at 11:41
Can you explain how to allocate in bytes. I follow what you are saying, just dont understand the solution. — user2455869, Mar 05 '15 at 11:43

unwind · Accepted Answer · 2015-03-05T11:49:07.290

This:

char ** file_names = malloc(10);

is a bug. It attempts to allocate 10 bytes, which has nothing at all to do with how many bytes you need. Under-allocating and then overwriting gives you undefined behavior.

It should be something like:

char **file_names = malloc(argc * sizeof *file_names);

This computes the size of the allocation by multiplying the number of arguments (argc, if you don't want to store argv[0] then this should be (argc - 1), of course) by the size of a character pointer, which is expressed as sizeof *file_names. Since file_names is of type char * *, the type of *file_names is char *, which is what you want. This is a general pattern, it can be applied very often and it lets you stop repeating the type name. It can protect you from errors.

For instance compare:

double *floats = malloc(1024 * sizeof(float));  /* BAD CODE */

and:

double *floats = malloc(1024 * sizeof *floats);

If you imagine that originally it was float *floats (as the naming suggests) then the first variant contains another under-allocation bug, while the second "survived" the change of types without error.

Then you need to check that it succeeded, before assuming it did.

Thank you. Never knew about under allocating and overwriting. Can you explain what you are doing with your second line. what does "malloc(argc * sizeof *file_names)" do? — user2455869, Mar 05 '15 at 11:45
@user2455869: It allocates memory for argc elements of the size that `*file_names` has, which is `sizeof(char*)`. — datenwolf, Mar 05 '15 at 11:47
@user2455869 What did you think `malloc(10)` meant? Surely it stands to reason that if you use more than you allocate, that can be a problem? — unwind, Mar 05 '15 at 11:56

score 1 · Answer 2 · answered Mar 05 '15 at 11:43

1

You want to allocate the right amount of memory for file_names, probably more like:

char ** file_names = malloc(sizeof(char*) * (argc - 1));

answered Mar 05 '15 at 11:43

Paul Evans

27,315
3
37
54

But `(sizeof(char*) * argc)` if you want to copy all the arguments. – Weather Vane Mar 05 '15 at 11:47
@WeatherVane This is asumming the executable file name wasn't needed as per the `for` loop. But if so, then yes use `argc` not `argc - 1` – Paul Evans Mar 05 '15 at 11:58
Ah yes, I missed his `argv[i+1]`. – Weather Vane Mar 05 '15 at 12:00

Memory Management Command Line Arguments in C

2 Answers2