JOIN function in c language, valgrind problem

Question

I wrote a functions that tries to join two strings, using dynamic memory allocation. I can understand that it could be easer if I used a defined array of char, but I would to learn how manage memory in C.

The problem is simple ... Running the program with valgrind I found different error like these:

Invalid write of size 1
Invalid read of size 1
Address .... is 0 bytes [after|inside] a block of size 4 alloc'd.

I tried to find out where the problem could be, I did, but I didn't understand what's wrong.

It's very important to me to solve this problem, so I can go on with my homework and check the errors.

That's the code

void _join(char *s1, char *s2)
{
    int i = 0;
    int len = strlen(s1);
    while (i < strlen(s2)){
        s1 = realloc(s1, (len + i)*sizeof(char));
        s1[len + i]= s2[i];
        i++;
    }
    // I insert that after, but I don't know if it's necessary
    s1 = realloc(s1, (len + i + 1)*sizeof(char));
    s1[len + i] = '\0';
}

The code goes from line 48 to line 58.

I call the function from the main in this way

_join(s1, s2);

Where s1 and s2 are given by input. Then I call free function on s1 and s2, 'cause I created there with malloc.

A snippets of valgrind output

Before: hello
==11005== Invalid write of size 1
==11005==    at 0x109322: _join (changeJOIN.c:53)
==11005==    by 0x1091E9: main (changeJOIN.c:22)
==11005==  Address 0x4a41be4 is 0 bytes after a block of size 4 alloc'd
==11005==    at 0x4839D7B: realloc (vg_replace_malloc.c:826)
==11005==    by 0x1092FB: _join (changeJOIN.c:52)
==11005==    by 0x1091E9: main (changeJOIN.c:22)
==11005==

... Other error ...

==11005==  
==11005== HEAP SUMMARY:
==11005==     in use at exit: 9 bytes in 1 blocks
==11005==   total heap usage: 17 allocs, 17 frees, 2.109 bytes allocate
==11005==
==11005== 9 bytes in 1 blocks are definitely lost in loss record 1 of 1
==11005==    at 0x4839D7B: realloc (vg_replace_malloc.c:826)
==11005==    by 0x10935B: _join (changeJOIN.c:56)
==11005==    by 0x1091E9: main (changeJOIN.c:22)

Rather than multiple calls to realloc, you could realloc(s1, len + sizeof(s2) +1); and then loop (or memcpy) to copy s2 into s1. Save a lot of time. — user4581301
You're allocating N bytes and writing past that by one. What's the largest index you can use with an array that size? — Shawn
First, your function does not return a char, despite its prototype. Second, it is very inefficient. Third, and most important, realloc invalidates the original value of s1 but the change does not propagate to the caller. A simple solution: return s1 as char* and free it at the caller's. — DYZ
Worse, it might use the same storage allocated for s1 after the realloc, making you think your program works when it doesn't. — user4581301
I forgot about strcat. It's another simple solution to appending s2 to the resized s1 — user4581301

CoffeeTableEspresso CoffeeTableEspresso · Accepted Answer · 2019-05-23T21:48:59

cleaner implementation:

char *_join(char *s1, char *s2)
{
    size_t len1 = strlen(s1);
    size_t len2 = strlen(s2);
    s1 = realloc(s1, len1 + len2 + 1); // +1 for null terminator.
    memcpy(s1 + len1, s2, len2 + 1);

    return s1;
}

This should fix your valgrind issues, and is much cleaner.

Make sure you call it as s1 = _join(s1, s2), because the realloc will invalidate s1.

JOIN function in c language, valgrind problem

1 Answers