-3

Dint find the exact link to my answer, so asking this simple question in terms where the function returns the char *

I wrote the code in this way so that the concat function itself should allocate & free the memory.

Can anyone plz help with the questions:

1) Is the freeing of memory placed correctly? (I ran Valgrind & found no issues as well)

2) How come pointer "ptr" which is allocated holding value, as per my understanding, if a memory to a ptr is freed, then value also should be lost right? Kindly correct me if am wrong.

char * concat(char *str1, char *str2)
{
    char *ptr;
    ptr=(char*)malloc(strlen(str1)+strlen(str2)+1); //allocated+1 for NULL character as well
    int i=0,j=0;
    while(str1[i]!='\0')
    {
        ptr[j]=str1[i];
        printf("letter from src is %c\n",str1[i]);
        i++; j++;
    }
    i=0;
    while(str2[i]!='\0')
    {
        ptr[j]=str2[i];
        j++;i++;
    }
    ptr[j]='\0';
    cout<<ptr;
    free(ptr);
    return ptr;
}

int main()
{
    char *str1="hello";
    char *str2="world";
    cout<<str1<<endl;
    cout<<str2<<endl;
    char *res=concat(str1,str2);
    return 0;
}
Azeez
  • 25
  • 5
  • 1
    `cout` is C++. Why are you using `malloc` and `free` in C++? – Vittorio Romeo Sep 07 '17 at 11:55
  • 1
    The `free` function is not required to overwrite the freed memory. – Jabberwocky Sep 07 '17 at 11:56
  • `free` move to `main`. – BLUEPIXY Sep 07 '17 at 11:56
  • Read the first answer to this SO question: https://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope. It's not quite the same question, and the answer is for C++, but it applies perfectly to your question. – Jabberwocky Sep 07 '17 at 11:57
  • @MichaelWalz the code **is** C++. – too honest for this site Sep 07 '17 at 12:07
  • @Olaf right, I didn't read the whole code and the question was tagged C it has been edited, but the link is still worth beeing read. – Jabberwocky Sep 07 '17 at 12:09
  • I was expecting the #downvoter to share if any link available which answers my question. @MichaelWalz can u plz elaborate it? – Azeez Sep 07 '17 at 12:10
  • @BLUEPIXY moving free to main i would doubt if any dumps occur later in bigger codes... – Azeez Sep 07 '17 at 12:12
  • @Azeez downvote was not mine. Did you read the link I posted (I know it's long, but really worth reading). You allocate memory, then put some information there and then free it. Now you wonder why the information is still there when you access the freed memory. The `free` function puts the freed memory back into the pool of free memory, that's all. Just as the hotel room is put back into the pool of free rooms when you check out. This is only about point 2) of your question. – Jabberwocky Sep 07 '17 at 12:15
  • @MichaelWalz: From his posts, I'm not sure if OP can see the similarities. Didn't find one, but I'm sure there are a lot of dupes of that one. That's in fact C basics, taught by every C book. But youtube videos are so much more entertaining … – too honest for this site Sep 07 '17 at 12:25
  • 1
    I'm voting to close this question as off-topic because this is really much too broad for Stack Overflow, which deals with more focused coding problems than this. For advice on improving working code, consider [codereview.se] instead - but do read [A guide to Code Review for Stack Overflow users](//codereview.meta.stackexchange.com/a/5778) first, as some things are done differently over there!. – Toby Speight Sep 07 '17 at 14:03

2 Answers2

1

Valgrind isn't complaining because you don't access res in main. If you do read res then I expect that you will get Valgrind errors, and quite likely crashes in a bigger application.

You need to change the end of concat to remove the call to free, i.e.,

    cout<<ptr;
    return ptr;
}

Also rather than hand written loops, you could use strcat, i.e.,

ptr=(char*)malloc(strlen(str1)+strlen(str2)+1);
strcat(ptr, str1);
strcat(ptr, str2);

Then in main you can use your result. For instance

int main()
{
    char *str1="hello";
    char *str2="world";
    cout<<str1<<endl;
    cout<<str2<<endl;
    char *res=concat(str1,str2);

    cout << "res from main: " << res << "\n";
    free(res);

    return 0;
}
Paul Floyd
  • 5,530
  • 5
  • 29
  • 43
  • My goal was to avoid using built in function, I was looking out to write a user defined function called concat(..).. any ways thanks for your reply – Azeez Sep 07 '17 at 12:28
  • The two lines of `strcat` that I suggest would still be in the body of your user concat function, just that they would replace the 14 lines it takes for your hand written loops, – Paul Floyd Sep 07 '17 at 12:35
1

When you call ptr = malloc(/*size*/) a block of free memory of at least the size you requested is allocated to you by the system; and the memory address of the start of this block is returned to you.

When you call free(ptr), you're telling the system you no longer want that block of memory. However your pointer will still point to the same memory address after the call.

The implementation of free is system dependant. Accessing memory at this address after you've called free(ptr) is undefined behaviour.

MikeySans
  • 11
  • 1
  • Thanks, so moving it to main is correct is it? or what do u suggest is better.? – Azeez Sep 07 '17 at 12:54
  • yes, I'd also suggest changing `char * concat (char *str1, char *str2)` to `char * concat (const char *str1, const char *str2)` so it's clear you aren't changing or returning your input strings – MikeySans Sep 07 '17 at 13:28