C: Concatenating Dynamically Allocated Strings

Question

I have searched high and low for an applicable answer, but I couldn't find one. I am well aware my inability to solve this issue is due to a unfamiliarity of C.

I am working on a function to concatenate one dynamically allocated string onto another, and it seems to work outside the realm of memory leaks and valgrind; however, when I run it with valgrind it is evident I have some glaring memory leaks I just cannot find. It seems to be a issue with how I am using realloc.

here is the code:

#include 
#include 
#include 

void dynamCat(char *dest, const char *src) {

    size_t len = strlen(dest)  + strlen(src) + 1;

    printf("strLen: %ld
", len);

    char *tmp = realloc(dest, len);

    if(tmp == NULL){
        fprintf(stderr, "strAppend: realloc messed up
");
        free(tmp);
        return;
    } else {
       dest = tmp;
    }

    strcat(dest, src);
}

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

        char *one = malloc(4);
        strcpy(one, "tee");

        char *two = malloc(4);
        strcpy(two, "hee");

        printf("one: %s
", one);
        printf("two: %s
", two);

        dynamCat(one, two);

        printf("one: %s
", one);
        printf("two: %s
", two);

        //freeee
        free(one);
        free(two);


        return 0;

}

the output on the command line:

$ ./dynamCat
one: tee
two: hee
strLen: 7
one: teehee
two: hee

here's where the bad stuff comes.

output with valgrind on the commandline:

==28804== Memcheck, a memory error detector
==28804== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==28804== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==28804== Command: ./dynamCat
==28804==
==28804== Invalid read of size 1
==28804==    at 0x4841C72: strlen (vg_replace_strmem.c:459)
==28804==    by 0x48D9407: __vfprintf_internal (in /usr/lib/libc-2.33.so)
==28804==    by 0x48C463E: printf (in /usr/lib/libc-2.33.so)
==28804==    by 0x1092EE: main (dynamCat.c:37)
==28804==  Address 0x4a3c040 is 0 bytes inside a block of size 4 free'd
==28804==    at 0x4840D7B: realloc (vg_replace_malloc.c:834)
==28804==    by 0x1091FA: dynamCat (dynamCat.c:11)
==28804==    by 0x1092D6: main (dynamCat.c:35)
==28804==  Block was alloc'd at
==28804==    at 0x483E77F: malloc (vg_replace_malloc.c:307)
==28804==    by 0x10926D: main (dynamCat.c:26)
==28804==
==28804== Invalid read of size 1
==28804==    at 0x4841C84: strlen (vg_replace_strmem.c:459)
==28804==    by 0x48D9407: __vfprintf_internal (in /usr/lib/libc-2.33.so)
==28804==    by 0x48C463E: printf (in /usr/lib/libc-2.33.so)
==28804==    by 0x1092EE: main (dynamCat.c:37)
==28804==  Address 0x4a3c041 is 1 bytes inside a block of size 4 free'd
==28804==    at 0x4840D7B: realloc (vg_replace_malloc.c:834)
==28804==    by 0x1091FA: dynamCat (dynamCat.c:11)
==28804==    by 0x1092D6: main (dynamCat.c:35)
==28804==  Block was alloc'd at
==28804==    at 0x483E77F: malloc (vg_replace_malloc.c:307)
==28804==    by 0x10926D: main (dynamCat.c:26)
==28804==
==28804== Invalid read of size 1
==28804==    at 0x48460D0: mempcpy (vg_replace_strmem.c:1536)
==28804==    by 0x48ED211: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib/libc-2.33.so)
==28804==    by 0x48D915A: __vfprintf_internal (in /usr/lib/libc-2.33.so)
==28804==    by 0x48C463E: printf (in /usr/lib/libc-2.33.so)
==28804==    by 0x1092EE: main (dynamCat.c:37)
==28804==  Address 0x4a3c042 is 2 bytes inside a block of size 4 free'd
==28804==    at 0x4840D7B: realloc (vg_replace_malloc.c:834)
==28804==    by 0x1091FA: dynamCat (dynamCat.c:11)
==28804==    by 0x1092D6: main (dynamCat.c:35)
==28804==  Block was alloc'd at
==28804==    at 0x483E77F: malloc (vg_replace_malloc.c:307)
==28804==    by 0x10926D: main (dynamCat.c:26)
==28804==
==28804== Invalid read of size 1
==28804==    at 0x48460DE: mempcpy (vg_replace_strmem.c:1536)
==28804==    by 0x48ED211: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib/libc-2.33.so)
==28804==    by 0x48D915A: __vfprintf_internal (in /usr/lib/libc-2.33.so)
==28804==    by 0x48C463E: printf (in /usr/lib/libc-2.33.so)
==28804==    by 0x1092EE: main (dynamCat.c:37)
==28804==  Address 0x4a3c040 is 0 bytes inside a block of size 4 free'd
==28804==    at 0x4840D7B: realloc (vg_replace_malloc.c:834)
==28804==    by 0x1091FA: dynamCat (dynamCat.c:11)
==28804==    by 0x1092D6: main (dynamCat.c:35)
==28804==  Block was alloc'd at
==28804==    at 0x483E77F: malloc (vg_replace_malloc.c:307)
==28804==    by 0x10926D: main (dynamCat.c:26)
==28804==
==28804== Invalid free() / delete / delete[] / realloc()
==28804==    at 0x483F9AB: free (vg_replace_malloc.c:538)
==28804==    by 0x109312: main (dynamCat.c:41)
==28804==  Address 0x4a3c040 is 0 bytes inside a block of size 4 free'd
==28804==    at 0x4840D7B: realloc (vg_replace_malloc.c:834)
==28804==    by 0x1091FA: dynamCat (dynamCat.c:11)
==28804==    by 0x1092D6: main (dynamCat.c:35)
==28804==  Block was alloc'd at
==28804==    at 0x483E77F: malloc (vg_replace_malloc.c:307)
==28804==    by 0x10926D: main (dynamCat.c:26)
==28804==
==28804==
==28804== HEAP SUMMARY:
==28804==     in use at exit: 7 bytes in 1 blocks
==28804==   total heap usage: 4 allocs, 4 frees, 4,111 bytes allocated
==28804==
==28804== 7 bytes in 1 blocks are definitely lost in loss record 1 of 1
==28804==    at 0x4840D7B: realloc (vg_replace_malloc.c:834)
==28804==    by 0x1091FA: dynamCat (dynamCat.c:11)
==28804==    by 0x1092D6: main (dynamCat.c:35)
==28804==
==28804== LEAK SUMMARY:
==28804==    definitely lost: 7 bytes in 1 blocks
==28804==    indirectly lost: 0 bytes in 0 blocks
==28804==      possibly lost: 0 bytes in 0 blocks
==28804==    still reachable: 0 bytes in 0 blocks
==28804==         suppressed: 0 bytes in 0 blocks
==28804==
==28804== For lists of detected and suppressed errors, rerun with: -s
==28804== ERROR SUMMARY: 9 errors from 6 contexts (suppressed: 0 from 0)

thank you so much for your help. I am still very new to C and really appreciate your help.

user_298225 · Answer

If realloc returns a null pointer, do not free its return value; there is no point in passing a null pointer to free:

    char *tmp = realloc(dest, len);

    if(tmp == NULL){
        fprintf(stderr, "strAppend: realloc messed up
");
        free(tmp); // This does nothing.

dest is a parameter, so assigning to it changes only the parameter. It does not change the original argument:

       dest = tmp; // This changes only the dest inside dynamCat.

To give the caller a new address, you must either return it as a pointer:

char *dynamCat(char *dest, const char *src)
{
    …
    dest = something;
    …
    return dest;
}

or you must put it into a pointer passed by the caller:

void dynamCat(char **dest, const char *src)
{
    …
    char *tmp = realloc(*dest, len);
    …
        *dest = tmp;
    }

    strcat(*dest, src);
}

In the latter case, when calling dynamCat, you must pass it the address of a pointer:

        dynamCat(&one, two);

In the former case, you must take the new pointer from the return value of dynamCat:

    one = dynamCat(one, two);

user_1250772 · Answer

The realloc function can change the address of the object. When he object is being made bigger, the need for this is obvious: there may not be space at the original location. It's potentially true when shrinking the object also.

In abstract terms, when we call realloc(x, newsize), the pointer x becomes indeterminate. We must capture the returned pointer y = realloc(x, newsize) and then use that in all places that previously referred to x.

In your program the catenating function returns void, and main keeps referring to the old string.

Your dynamCat should be like this:

/* dest is destroyed: use the return value
 * in place of dest!
 */
char *dynamCat(char *dest, const char *src) {
    size_t len = strlen(dest)  + strlen(src) + 1;

    printf("strLen: %ld
", len);

    char *tmp = realloc(dest, len);

    if(tmp == NULL){
        fprintf(stderr, "strAppend: realloc messed up
");
        /* better handling needed here, too */
    }

    strcat(tmp, src);
    return tmp;
}

C: Concatenating Dynamically Allocated Strings

2 Answers