Variable Arguments in C creating error in Valgrind

Question

I was trying to run a program which uses a function concat_str. It can take in multiple arguments as strings and the end of arguments is denoted by "quit". The code for my function is given below:

char *concat_str(char *str1, ...)
{
    va_list pstr;
    char *minion = NULL, *temp = NULL;
    minion = (char*) malloc (sizeof(str1));
    strcpy (minion,str1);
    va_start (pstr, str1);
    if ( strcmp ("quit",str1) == 0)
    {
        va_end (pstr);
        return minion;
    }
    while (1)
    {
        temp = va_arg (pstr, char *);
        if ( strcmp ("quit", temp) == 0)
        {
            break;
        }
        minion = (char*) realloc (minion, sizeof(temp));
        strncat (minion,temp,sizeof(temp));
    }
    va_end (pstr);
    return minion;
}

A calling statement for the same would be:

char *result;
result = concat_str("hello", "hai", "how", "are", "you", "quit");

I do get the correct output. But when I ran it on valgrind with memcheck tool, I got many errors. The error is as below:

==2635== Invalid write of size 1
==2635==    at 0x4A065D3: strncat (mc_replace_strmem.c:218)
==2635==    by 0x400A7E: concat_str (var_fun.c:23)
==2635==    by 0x400757: main (var_main.c:15)
==2635==  Address 0x4C33038 is 0 bytes after a block of size 8 alloc'd
==2635==    at 0x4A0590B: realloc (vg_replace_malloc.c:306)
==2635==    by 0x400A5F: concat_str (var_fun.c:22)
==2635==    by 0x400757: main (var_main.c:15)
==2635==
==2635== Invalid read of size 1
==2635==    at 0x4A06594: strncat (mc_replace_strmem.c:218)
==2635==    by 0x400A7E: concat_str (var_fun.c:23)
==2635==    by 0x400757: main (var_main.c:15)
==2635==  Address 0x4C33038 is 0 bytes after a block of size 8 alloc'd
==2635==    at 0x4A0590B: realloc (vg_replace_malloc.c:306)
==2635==    by 0x400A5F: concat_str (var_fun.c:22)
==2635==    by 0x400757: main (var_main.c:15)
==2635==
==2635== Invalid write of size 1
==2635==    at 0x4A065BF: strncat (mc_replace_strmem.c:218)
==2635==    by 0x400A7E: concat_str (var_fun.c:23)
==2635==    by 0x400757: main (var_main.c:15)
==2635==  Address 0x4C33038 is 0 bytes after a block of size 8 alloc'd
==2635==    at 0x4A0590B: realloc (vg_replace_malloc.c:306)
==2635==    by 0x400A5F: concat_str (var_fun.c:22)
==2635==    by 0x400757: main (var_main.c:15)

==2635==
==2635== Invalid read of size 1
==2635==    at 0x4A066D4: strlen (mc_replace_strmem.c:246)
==2635==    by 0x32DAC46B18: vfprintf (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC4D3A9: printf (in /lib64/libc-2.5.so)
==2635==    by 0x40076E: main (var_main.c:16)
==2635==  Address 0x4C33038 is 0 bytes after a block of size 8 alloc'd
==2635==    at 0x4A0590B: realloc (vg_replace_malloc.c:306)
==2635==    by 0x400A5F: concat_str (var_fun.c:22)
==2635==    by 0x400757: main (var_main.c:15)
==2635==
==2635== Invalid read of size 1
==2635==    at 0x32DAC6CE09: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC464B2: vfprintf (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC4D3A9: printf (in /lib64/libc-2.5.so)
==2635==    by 0x40076E: main (var_main.c:16)
==2635==  Address 0x4C33040 is 8 bytes after a block of size 8 alloc'd
==2635==    at 0x4A0590B: realloc (vg_replace_malloc.c:306)
==2635==    by 0x400A5F: concat_str (var_fun.c:22)
==2635==    by 0x400757: main (var_main.c:15)
==2635==
==2635== Invalid read of size 1
==2635==    at 0x32DAC6CE1C: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC464B2: vfprintf (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC4D3A9: printf (in /lib64/libc-2.5.so)
==2635==    by 0x40076E: main (var_main.c:16)
==2635==  Address 0x4C3303F is 7 bytes after a block of size 8 alloc'd
==2635==    at 0x4A0590B: realloc (vg_replace_malloc.c:306)
==2635==    by 0x400A5F: concat_str (var_fun.c:22)
==2635==    by 0x400757: main (var_main.c:15)
==2635==
==2635== Invalid read of size 1
==2635==    at 0x32DAC6CD66: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC464B2: vfprintf (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC4D3A9: printf (in /lib64/libc-2.5.so)
==2635==    by 0x40076E: main (var_main.c:16)
==2635==  Address 0x4C33038 is 0 bytes after a block of size 8 alloc'd
==2635==    at 0x4A0590B: realloc (vg_replace_malloc.c:306)
==2635==    by 0x400A5F: concat_str (var_fun.c:22)
==2635==    by 0x400757: main (var_main.c:15)
==2635==
==2635== Invalid read of size 1
==2635==    at 0x32DAC6CD7A: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC464B2: vfprintf (in /lib64/libc-2.5.so)
==2635==    by 0x32DAC4D3A9: printf (in /lib64/libc-2.5.so)
==2635==    by 0x40076E: main (var_main.c:16)
==2635==  Address 0x4C33039 is 1 bytes after a block of size 8 alloc'd
==2635==    at 0x4A0590B: realloc (vg_replace_malloc.c:306)
==2635==    by 0x400A5F: concat_str (var_fun.c:22)
==2635==    by 0x400757: main (var_main.c:15)

Source of error (my own deduction):

Line 22 : minion = (char*) realloc (minion, sizeof(temp));

Realloc returns the address of a pointer of a new block to minion. But the old block creates problems.

Things I tried:

1. I changed strncat (minion,temp,sizeof(temp)); to strncat (minion,temp,sizeof(temp) + 10); . This reduced some errors. But if the string arguments were lengthy, I again got the same errors. By the by, I didn't understand how this solved the issue.

2. I changed minion = (char*) realloc (minion, sizeof(temp)); to

   char t = NULL; t = minion; minion = (char) realloc (t, sizeof(temp)); free (t);

Please tell me, if my source of error is correct and suggest what I should do to solve this issue.

Answer 1

The expression

sizeof(str1)

will yield the size of a pointer, not the string length. You should allocate

minion = malloc(strlen(str1) + 1);

For your reallocations, you must provide the size of the whole array, not only the newly allocated storage, so you should keep track of your string length.

And finally, a stylistic hint: It is common to use NULL as a sentinel value to terminate string list instead of using an arbitrary literal like your "quit".