4
void x( )
{
    strcpy(a, strdup(p) );
}

(error) Allocation with strdup, strcpy doesn't release it

Can anyone tell me what's wrong with above statement and why I am getting this error?

R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
lokesh
  • 109
  • 1
  • 1
  • 9
  • 5
    what the heck are you trying to do? – Karoly Horvath Sep 09 '11 at 11:16
  • nothing :D actually just started programming,while reading forms i saw this function tried some combinations...(just for fun) – lokesh Sep 09 '11 at 11:32
  • Depending on your needs, you should do either `a = strdup(p);` (`a` is now dynamically allocated) or `strcpy(a, p);` (assumes `a` already points to a buffer of sufficient size). Combining those calls is pointless. – visitor Sep 09 '11 at 11:32
  • actually i got this code from somewhere....i didnt know about strdup() function and also they said there are chances of memory leaks...i didnt know that strdup will use malloc and allocate memory i thought it will create an temporary variable ....so it will be removed from memory when scope of that function ends. – lokesh Sep 09 '11 at 11:45

3 Answers3

10

The problem is that you are leaking memory. The call to strdup allocates memory which is not freed. The pointer to the memory that is passed to strcpy is never saved anywhere and the compiler can therefore prove that it is leaked.

I'm not sure what you are trying to do since strdup performs both allocation and copying, the call to strcpy seems superfluous.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • hmm...actually i read from a post that strdup will make an string or copy it and will return its address,so strcpy should be able to catch that address and copy them. – lokesh Sep 09 '11 at 11:24
  • 1
    You don't understand. `strdup` allocates memory. That is then passed to `strcpy`, but `strcpy` does not deallocate that memory since that is not its job. Thus the memory allocated by `strdup` is never freed. – David Heffernan Sep 09 '11 at 11:26
  • oh got it thanx.....cause we havent stored adress of the new allocated array,we will never be able to delete that :D – lokesh Sep 09 '11 at 11:28
  • @lokesh That's fine. I was just trying to help you along since you are new. – David Heffernan Sep 09 '11 at 11:48
3

strdup doing something similar to this (Taken from paxdiablo's answer here) :-

char *strdup (const char *s) {
char *d = malloc (strlen (s) + 1);   // Allocate memory
if (d != NULL)
    strcpy (d,s);                    // Copy string 
return d;                            // Return new memory

}

SO leaking the memory which has been allocated inside strdup leads to that error .

Community
  • 1
  • 1
Sandeep Pathak
  • 10,567
  • 8
  • 45
  • 57
  • Mate, if you're going to "lift" my code, either credit me (http://stackoverflow.com/questions/252782/strdup-what-does-it-do-in-c/252802#252802) or disguise the fact with a little editing :-) Just joking, but I've only just gone back and edited that answer recently so I recognised it instantly. – paxdiablo Sep 09 '11 at 15:19
2

The problem is strdup calls malloc() inside and passes the resulting pointer to your code. Your code doesn't take ownership of that allocated memory and it is leaked. You code is roughly doing this:

char* duplicate = malloc(strlen(p)+1); //first part of strdup
memcpy(dupliate,p); //second part of strdup
memcpy(a, duplicate);//memcpy in your code

the above code doesn't free() memory pointed to by duplicate.

sharptooth
  • 167,383
  • 100
  • 513
  • 979