Marco Costa
Marco Costa

Reputation: 37

String concatenate error with malloc dynamic memory allocation

String *char concatenate error with malloc dynamic memory allocation

I want to make a function to concatenate strings, it works but it gives an error and the processor restarts, I think there's something wrong with pointers, but I do not know what it is, a problem of memory allocation.

Thanks in advance!

char *buf;

int main(void) {
    // ...

    WriteString("#INIT.\r\n"); //serial output

    buf = "";

    while(1)
    {
        char *str1 = "qwe";
        char *str2 = "asd";
        char *str3 = "zxc";
        char *str4 = "123";

        buf = my_strcat(buf,str1);
        buf = my_strcat(buf,str2);
        buf = my_strcat(buf,str3);
        buf = my_strcat(buf,str4);

        WriteString(buf); //serial output

        free(buf);
    }
}

char *my_strcat(const char *str1, const char *str2) {
    char *new_str;
    new_str = malloc(strlen(str1)+strlen(str2)+1);
    new_str[0] = '\0';
    strcat(new_str,str1);
    strcat(new_str,str2);
    return new_str;
}

Serial output...

#INIT.
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
#INIT.
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
#INIT.
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
#INIT.
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
#INIT.
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
#INIT.
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
#INIT.
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
#INIT.
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123
qweasdzxc123

Upvotes: 0

Views: 1099

Answers (3)

Weather Vane
Weather Vane

Reputation: 34585

Your use of buf is illogical. You haven't shown how it was allocated but even if you did malloc() memory for buf, you overwrote it with

    buf = "";

Then, you have an infinite loop with no exit condition

while(1) {
    ...    
}

which will continue to attempt concatenation until the computer catches fire. Worse, at the end of the while() loop you

free(buf);

So on the subsequent, infinite, loops, you don't even have buf to concatenate to.

Upvotes: 1

R Sahu
R Sahu

Reputation: 206577

You have memory leaks in the while loop and are running out of memory.

    buf = my_strcat(buf,str1); // Got some new memory
    buf = my_strcat(buf,str2); // Got some more new memory without freeing the previous memory
                               // The previous memory is lost. You don't even have a pointer 
                               // to it any more.

    buf = my_strcat(buf,str3); // Ditto
    buf = my_strcat(buf,str4); // Ditto

What you need:

    char* temp = NULL
    buf = my_strcat(buf,str1);
    temp = buf;

    buf = my_strcat(temp,str2);
    free(temp);
    temp = buf;

    buf = my_strcat(temp,str3);
    free(temp);
    temp = buf;

    buf = my_strcat(temp,str4);
    free(temp);

Upvotes: 2

Iharob Al Asimi
Iharob Al Asimi

Reputation: 53006

Your first call to my_strcat has undefined behavior because buf was not initialized before. Precisely this line is the problem

new_str = malloc(strlen(str1)+strlen(str2)+1);

strlen(str1) where str1 is uninitialized.

Suggestion, use realloc

char *my_strcat(char *str1, const char *str2)
{
    char  *new_str;
    size_t length;
    size_t str1length;

    if (str2 == NULL)
        return str1;
    str1length = 0;
    if (str1 != NULL)
        str1length = strlen(str1);
    length  = strlen(str2) + str1length;
    new_str = realloc(str1, 1 + length);
    if (new_str == NULL)
        return str1;
    new_str[str1length] = '\0';

    strcat(new_str, str2);

    return new_str;
}

and

char *buf;
char *str1 = "qwe";
char *str2 = "asd";
char *str3 = "zxc";
char *str4 = "123";

buf = NULL;
buf = my_strcat(buf, str1);
buf = my_strcat(buf, str2);
buf = my_strcat(buf, str3);
buf = my_strcat(buf, str4);

Upvotes: 4

Related Questions