My char pointer returns weird string

Question

As trying to concatenate two strings (char arrays), the code below returns the correct result via return but not the expected result through the arguments passed by reference (namely, char *dst in the StrAdd function).

Concerning the "after" results; printf prints the correct concatenated string for st. But the variable "s1" is supposed to contain the concatenated string as well. However, s1 prints something weird.

Can someone figure out what's wrong with it?

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

char *StrAdd (char *dst, const char *src) {
/*  add source string "src" at the end of destination string "dst"
    i.e. concatenate
*/
    size_t lenDst = strlen (dst);
    size_t lenSrc = strlen (src);
    size_t lenTot = lenDst + lenSrc + 1;
    char *sTmp = (char *) malloc (lenTot);
    memcpy (&sTmp [0], &dst [0], lenDst);
    memcpy (&sTmp [lenDst], &src [0], lenSrc);
    sTmp [lenTot - 1] = '\0';
    free (dst);
    dst = (char *) malloc (lenTot);
    memcpy (&dst [0], &sTmp [0], lenTot);
    free (sTmp);
    return (dst);
}

int main () {
    char *s1 = strdup ("Xxxxx");
    char *s2 = strdup ("Yyyyy");
    char *st = strdup ("Qqqqq");

    printf ("s1 before: \"%s\"\n", s1);
    printf ("s2 before: \"%s\"\n", s2);
    printf ("st before: \"%s\"\n", st);
    printf ("\n");

    st = StrAdd (s1, s2);

    printf ("s1 after : \"%s\"\n", s1); // weird???
    printf ("s2 after : \"%s\"\n", s2); // ok
    printf ("st after : \"%s\"\n", st); // ok
    printf ("\n");

    return (0);
}

2501 · Accepted Answer · 2014-10-12 15:15:55Z

8

You passed s1 to the function and then called free on it. After you do that the pointer is no longer valid, and you cannot use it. If you do, like you did in printf(), you get undefined behavour.

answered Oct 12, 2014 at 15:15

2501

25.8k4 gold badges51 silver badges93 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

ssd Over a year ago

Ok, I free the dst in the function once, but then allocate another memory space for that variable. Doesn't that second allocation return back the pointer via the *dst argument in the header? As printed in the main, it returns the correct pointer via return (dst); why not via *dst in the header?

user3920237 Over a year ago

@merkez3110 No, you're freeing s1 and assigning the newly allocated memory to st. Please see a proper implementation of strcat here.

ssd Over a year ago

Excuse me for using wrong terminology. By the term "header", I did not mean a header file; I meant the char *StrAdd (char *dst, const char *src) part at the top of the function.

Spikatrix Over a year ago

@merkez3110 ,FYI, that is called a function definition

weston Over a year ago

@merkez3110 Yes you still need the space, it just replaces memcpy. I've put strcat example in my second case.

|

weston · Accepted Answer · 2014-10-12 15:58:32Z

4

If you want to create and return a new string, just return the sTmp you alloced and do not free the dst.

char *StrAdd (const char *dst, const char *src) {
    size_t lenDst = strlen (dst);
    size_t lenSrc = strlen (src);
    size_t lenTot = lenDst + lenSrc + 1;
    char *sTmp = (char *) malloc (lenTot);
    memcpy (&sTmp [0], &dst [0], lenDst);
    memcpy (&sTmp [lenDst], &src [0], lenSrc + 1); //+1 copies the existing nil char from src
    return sTmp;
}

On the other hand if you do want the destination pointer passed in to be updated, you will need to pass it by reference, a pointer to a pointer:

char *StrAdd (char** dstPtr, const char *src) {
    char* dst = *dstPtr;
    size_t lenDst = strlen (dst);
    size_t lenSrc = strlen (src);
    size_t lenTot = lenDst + lenSrc + 1;
    char *sTmp = (char *) realloc (dst, lenTot); //realloc will also copy the old data for us
    strcat(sTmp, src); //same as the second memcpy
    *dstPtr = sTmp; //update dest pointer
    return sTmp;
}

But you will need to call it like this:

st = StrAdd (&s1, s2);

After this, s1 and st point to the same string, try this to see that:

printf ("Addresses after: %p %p\n", (void*)s1, (void*)st);

edited Oct 12, 2014 at 15:58

answered Oct 12, 2014 at 15:29

weston

55k23 gold badges151 silver badges209 bronze badges

5 Comments

ssd Over a year ago

Maybe @2501's explanation is correct but your solutions seems better. This way, I think, the pointer to *s1 will not get lost.

weston Over a year ago

I've added a way of updating s1. Just make sure you have no other references to that string as it gets freed.

weston Over a year ago

That is it could get freed by the realloc. realloc may not change the pointer.

ssd Over a year ago

Still, your first code (returning the newly defined char *, ie. sTmp) seems better. This way, if I want to use the old s1 after the function, I would use sNew = StrAdd (s1, s2); and if I allow the old s1 to be changed, I would use s1 = StrAdd (s1, s2);

weston Over a year ago

Problem with this: s1 = StrAdd (s1, s2); Is that nothing frees the old s1 and you will get a memory leak. char *newS1 = StrAdd (s1, s2); free(s1); s1 = newS1; would be the way to do that.

Collectives™ on Stack Overflow

My char pointer returns weird string

2 Answers 2

6 Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related