Array returning more characters than allocated with malloc

Question

I am writing a function in C which takes two strings of the same length,

for example:-

S1 = "Hello" and S2 = "12345"

and returns a result that mixes the characters from the input strings so that the first char in the result string is the first char from S1, the second char is the first char from S2, the third char in r is the second char from S1 and the fourth char in r is the second char from S2, and so on....

So if:-

S1 = "Hello"
S2 = "12345"
Result = "H1e2l3l4o5"

My function looks like this:

#include <stdio.h>
#include <string.h>

char* mix(char *s1, char *s2)
{
    int length = strlen(s1);
    char* result = malloc(sizeof(char) * length * 2);
    int i, j;
    for(i = 0, j = 0; i < (length); i++, j = j + 2)
    {
        result[j] = s1[i];
        result[j + 1] = s2[i];
    }
    return result;
}

For the values of S1 and S2 above it produces:

  `H1e2l3l4o5:=C:\Uºt6▄╘º`

Which confuses me as I didn't allocate "result" enough memory for that many chars and wasn't warned of a segmentation fault or anything of that nature. Have I obviously done something wrong?

don't forget to nul terminate the string. and add 1 to malloc — Jean-François Fabre, Oct 31 '17 at 10:24
The string delimiter `\0` is missing. Take a look at https://stackoverflow.com/questions/4711449/what-does-the-0-symbol-mean-in-a-c-string — Roberto Trani, Oct 31 '17 at 10:24
`strlen` would not give the end terminated null byte, so you need ad +1 byte in your `malloc()`. — danglingpointer, Oct 31 '17 at 10:26
I'm voting to close this question as off-topic because questions about the nul termination and size of the malloc-ed memory was answered here hundreds of time. OP did not even try to use the search — 0___________, Oct 31 '17 at 10:26

score 5 · Accepted Answer · answered Oct 31 '17 at 10:24

This happens because you did not null-terminated the string in the result. Add this line to fix this problem:

result[j] = '\0';

Another issue you need to fix is allocating the space for null terminator:

char* result = malloc(length * 2 + 1);

C standard requires sizeof(char) to be 1, so multiplying by it is not necessary.

Finally, you need to make sure that s2 has enough length to be copied into the result, i.e. strlen(s2) >= strlen(1).

score 0 · Answer 2 · answered Oct 31 '17 at 10:30

You didn't allocate memory for the null character required to terminate a C string and, of course, you didn't put it after the string. This is why the code prints garbage from memory until it reaches the first null character.

char* mix(char *s1, char *s2)
{
    int length = strlen(s1);
    char* result = malloc(sizeof(char) * (length * 2 + 1));
    int i, j;
    for(i = 0, j = 0; i < length; i ++, j = j + 2)
    {
        result[j] = s1[i];
        result[j + 1] = s2[i];
    }
    // Put the null character to end the the string
    result[j] = 0;

    return result;
}

Apparently the code should also check the length of s2 but, by chance, this is not needed.

If s1 is longer than s2, the null-terminating character of s2 is copied during the loop and the length of the resulting string is 2*strlen(s2). If s2 is longer than s1, the code copies only its first length characters.

Array returning more characters than allocated with malloc

2 Answers2