CODEWITHSUNDEEP
c
please delete me
please delete me

Reputation: 137

Segmentaton fault for simple strcat() wrapper function

I have an exercise where I need to write a wrapper function for strcat. After it prints the string length (for debugging) it seg faults and I am not quite sure why. Any help would be greatly appreciated.

EDIT: I didn't specify that the wrapper function is supposed to guarantee that it never goes outside of the bounds of memory allocated for destination. This is why I allocated only enough memory for "string" in destination and reallocating more in the Strcat() wrapper.

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

    char* Strcat(char* destination, const char* source);

    int main(void)
    {
        (void)printf("This strcat cannot fail!\n");
        char* destination = (char*)malloc(sizeof(char)*7);
        destination = "string";
        (void)printf("%s\n", destination);
        Strcat(destination, " concatination");
        (void)printf("%s\n", destination);
        (void)printf("It works!\n");

        free(destination);

        return 0;
    }

    char* Strcat(char* destination, const char* source)
    {
        (void)printf("%d\n", (strlen(destination)+strlen(source))*sizeof(char));
        if((sizeof((strlen(destination)+strlen(source)))+1) > sizeof(destination))
            destination = (char*)realloc(destination, sizeof((strlen(destination)+strlen(source)))+1);
        return strcat(destination, source);
    }

Upvotes: 2

Views: 196

Answers (3)

jpalecek
jpalecek

Reputation: 47762

    if((sizeof((strlen(destination)+strlen(source)))+1) > sizeof(destination))

That's wrong, sizeof doesn't give you anything comparable to the length of the string. Don't use it here.

Moreover, you can't really get the number of allocated bytes of the string. So if you know destination has been allocated with malloc[*], you should realloc unconditionally:

        destination = (char*)realloc(destination, strlen(destination)+strlen(source)+1);

Again, no sizeof. And be prepared to handle allocation failures, which means saving the old value of destination and freeing it when realloc returns 0.

[*] Note that destination = "string" breaks this premise.

Upvotes: 1

Ed Heal
Ed Heal

Reputation: 60007

Try summat like

char *Strcat(char *d, const char *s) /* please put in the proper names - i am being lazy*
{
   int dLen = strlen(d); /* ditto above */
   int sLen = strlen(s);
   d = (char *)realloc(d, dLen + sLen +1);
   return strcat(d, s);
}

Upvotes: 0

Nikolai Fetissov
Nikolai Fetissov

Reputation: 84189

This line:

destination = "string";

overwrites the pointer returned from malloc(3) so you lose that memory forever. You probably meant to copy that string, so use strcpy(3) or something.

Upvotes: 2

Related Questions