CODEWITHSUNDEEP
c++cfreestrdup
Ros
Ros

Reputation: 37

strdup and free usage

I'm trying this code:

imei=Found_imei(pClient->GetBuffer());
printf("6. CODICE IMEI %s \n",imei); 
pClient->SetImei(imei);
printf("6.1 CODICE IMEI %s \n",pClient->GetImei());
free(imei); <<<<<<<<<<<<<<
printf("6.2 CODICE IMEI %s \n",pClient->GetImei());

where 

char *Found_imei(char *string) 
{
char *start;
char *end;
char str[40];
int l;

start=strstr(string,"imei:");

strstr(start,",");

l=end-start-5;
strncpy(str,start+5,l);
str[l]='\0';
return strdup(str); <<<<<<<<<<<<<<<<< 
}

After free command, an error occured in printf.

I'm doing some error in using strdup and free?

Thanks

Upvotes: 0

Views: 290

Answers (3)

Adam Rosenfield
Adam Rosenfield

Reputation: 400454

Several obvious problems I see:

  1. The return value of strstr(start,","); is ignored, making that function call useless
  2. end is never initialized inside of the Found_imei function, so the pointer arithmetic l=end-start-5; results in Undefined Behavior
  3. strncpy doesn't always null-terminate its output. That can easily lead to walking off the end of a string and into Undefined Behavior.
  4. You're passing in l as the length parameter to strncpy when you should actually be passing the buffer size minus 1. If l is 40 or more, you're going to be copying too many characters into str and smash your stack.

I suspect that problems 1 and 2 are the result of a failure to copy+paste your exact code, since it looks like you intended to write end = strstr(start,",");, but I can't be sure. Nevertheless, problems 3 and 4 are still serious problems.

Upvotes: 1

Charlie Burns
Charlie Burns

Reputation: 7044

char *end; is used l=end-start-5; before it is initialized.

Thus

l=end-start-5;
strncpy(str,start+5,l); // god knows how far this goes. more than 40 easy
str[l]='\0'; // and here
return strdup(str); <<<<<<<<<<<<<<<<<

str can be overwritten ( out of bounds ), after that all bets are off.

Upvotes: 1

JaredPar
JaredPar

Reputation: 755131

I'm going to guess htat GetImei and SetImei are thin wrappers over a field of type char*. If that is the case then the free call is freeing the memory which backs the char* value. Hence the call to GetImei after the free is accessing freed memory and hence has undefined behavior

Upvotes: 1

Related Questions