Returning (char *) from a function - memory problem

[Jason]

I don't know if anyone caught my previous post about a project I've
inherited, but I have another question about it.

(Quick summary:
I have old C code, converting to C++. Compiles and runs fine)

My problem is when the application exits, CodeGuard tells me that I have
memory leaks, and points to a function.

char *fooFunc(int Bar)
{
char *returnArr = new char[8];
int stuff;

// do some stuff with Bar

sprintf(returnArr,"%2d",stuff);
return(returnArr);
}


I see that I am allocating memory, but it's not being freed. The function
is called like so:

fprintf(filePtr,"%s and stuff",fooFunc(myInt));

How do I free (delete) the memory allocated by this? I know I could change
it to

char returnArr[8];

but that would prevent me from learning anything new today.

TIA for everyone's help.

- Jason

 

[Victor Bazarov]

I don't know if anyone caught my previous post about a project I've
inherited, but I have another question about it.

(Quick summary:
I have old C code, converting to C++. Compiles and runs fine)

My problem is when the application exits, CodeGuard tells me that I
have memory leaks, and points to a function.

char *fooFunc(int Bar)
{
char *returnArr = new char[8];
int stuff;

// do some stuff with Bar

sprintf(returnArr,"%2d",stuff);
return(returnArr);
}


I see that I am allocating memory, but it's not being freed. The
function is called like so:

fprintf(filePtr,"%s and stuff",fooFunc(myInt));

How do I free (delete) the memory allocated by this? I know I could
change it to

char returnArr[8];

You could, but then you'd be returning a pointer to a local variable
which is a VERY BAD THING(tm).

but that would prevent me from learning anything new today.

Not only that, it would have UB.

Let's leave 'new[]' in for now. There is no good way to free that
memory unless you actually store the pointer you obtain from the
function and then use 'delete[]' on that pointer.

You *could* declare 'returnArr' _static_ in that function, but then
you have a multithreading disaster.

It is much better to return a 'std::string':

std::string fooFunc(int Bar) {
std::string returnStr;
...
return returnStr;
}

of course the places where it's called need to be rewritten as well:

fprintf(filePtr, "%s and stuff", fooFunc(myInt).c_str());

V

 

[Jason]

How do I free (delete) the memory allocated by this? I know I could
change it to

char returnArr[8];

You could, but then you'd be returning a pointer to a local variable
which is a VERY BAD THING(tm).

Ugh.. see, it's a good thing that I asked.

Not only that, it would have UB.

Undefined Behaviour.. see, I remembered. ;-)

> You *could* declare 'returnArr' _static_ in that function, but then
you have a multithreading disaster.

Yeah, I really don't want to do that because this application eventually
will be multithreaded, and I know all about the agony of shared globals
between threads.

It is much better to return a 'std::string':

std::string fooFunc(int Bar) {
std::string returnStr;
...
return returnStr;
}

of course the places where it's called need to be rewritten as well:

fprintf(filePtr, "%s and stuff", fooFunc(myInt).c_str());

So for now, there's pretty much nothing I can do except leave it. If I
understand correctly, this function will be allocating 8 bytes on the heap
every time it runs? So if it runs for a long time, this could eventually be
disastrous? How was something like this solved in the days of C?

Could I do this?

myCharPtr = fooFunc(myInt);
fprintf(filePtr,"%s and stuff",myCharPtr);
delete[] myCharPtr;

Would that work, or would that give me issues? More UBness?

Thanks for your patience with my naivety.

- Jason

 

[Victor Bazarov]

[..returning an array allocated by 'new[]'..]

It is much better to return a 'std::string':

std::string fooFunc(int Bar) {
std::string returnStr;
...
return returnStr;
}

of course the places where it's called need to be rewritten as well:

fprintf(filePtr, "%s and stuff", fooFunc(myInt).c_str());

So for now, there's pretty much nothing I can do except leave it. If
I understand correctly, this function will be allocating 8 bytes on
the heap every time it runs? So if it runs for a long time, this
could eventually be disastrous? How was something like this solved
in the days of C?

Store a pointer, use the pointer, free it.

Could I do this?

myCharPtr = fooFunc(myInt);
fprintf(filePtr,"%s and stuff",myCharPtr);
delete[] myCharPtr;

That's what I would expect if you wanted to save the 8 bytes (with
the overhead) from becoming a leak.

Would that work, or would that give me issues? More UBness?
Nope.

Thanks for your patience with my naivety.

You're welcome.

V

 

[Javier Albiero]

I don't know if anyone caught my previous post about a project I've
inherited, but I have another question about it.

(Quick summary:
I have old C code, converting to C++. Compiles and runs fine)

My problem is when the application exits, CodeGuard tells me that I have
memory leaks, and points to a function.

char *fooFunc(int Bar)
{
char *returnArr = new char[8];
int stuff;

// do some stuff with Bar

sprintf(returnArr,"%2d",stuff);
return(returnArr);

}

I see that I am allocating memory, but it's not being freed. The function
is called like so:

fprintf(filePtr,"%s and stuff",fooFunc(myInt));

How do I free (delete) the memory allocated by this? I know I could change
it to

char returnArr[8];

but that would prevent me from learning anything new today.

TIA for everyone's help.

- Jason

Adding to Victor's comments: you can replace sprintf with
stringstream. Something like this:

std::string fooFunc(int Bar) {
std::stringstream ss;

ss << std::setw(2) << Bar;

return ss.str();
}


Regards

 

[LR]

Could I do this?

myCharPtr = fooFunc(myInt);
fprintf(filePtr,"%s and stuff",myCharPtr);
delete[] myCharPtr;

Yes. But if you're going to do that, then could you use std::auto_ptr?

BTW, may I ask what the cost is in modifying the code? IE how many
times do you have a line similar to your fprintf line above in your code?

If it's only once or twice or twenty times or a hundred times it might
be better to take the suggestion, I think it was made by Victor, about
std::string and c_str().

In the event that you can edit the code to put the call before the
fprintf line and the delete afterwards, the fprintf line doesn't have to
remain exactly as it is, so what would the objection be to using
std::string and c_str()?

Does the data have to persist beyond the call to fprintf?

LR

 

[=?ISO-8859-1?Q?Erik_Wikstr=F6m?=]

I don't know if anyone caught my previous post about a project I've
inherited, but I have another question about it.

(Quick summary:
I have old C code, converting to C++. Compiles and runs fine)

My problem is when the application exits, CodeGuard tells me that I have
memory leaks, and points to a function.

char *fooFunc(int Bar)
{
char *returnArr = new char[8];
int stuff;

// do some stuff with Bar

sprintf(returnArr,"%2d",stuff);
return(returnArr);
}


I see that I am allocating memory, but it's not being freed. The function
is called like so:

fprintf(filePtr,"%s and stuff",fooFunc(myInt));

How do I free (delete) the memory allocated by this? I know I could change
it to

char returnArr[8];

but that would prevent me from learning anything new today.

I think these kinds of situation are handled in two different ways in C
libraries, the first is to use a static string (char array) in the
function and the result is then not thread safe (as Victor explained).
You could make it thread safe by using a string in TLS in a
multithreaded program, but it's not a good solution to begin with.

Another way is to change the method so that it takes a char* as argument
and performs the operations into this string, just like sprintf does.
This requires you to change the way you use the function, but you could
then use local variables like this

char str[8];
fooFunct(myInt, str;
fprintf(filePtr,"%s and stuff",str);

 

[Andre Kostur]

Could I do this?

myCharPtr = fooFunc(myInt);
fprintf(filePtr,"%s and stuff",myCharPtr);
delete[] myCharPtr;

Yes. But if you're going to do that, then could you use std::auto_ptr?

No! std::auto_ptr will call delete on the pointer, not delete[]. Since
the memory was originally new[]'ed, this would lead to UB.

 

[LR]

Could I do this?

myCharPtr = fooFunc(myInt);
fprintf(filePtr,"%s and stuff",myCharPtr);
delete[] myCharPtr;

Yes. But if you're going to do that, then could you use std::auto_ptr?

No! std::auto_ptr will call delete on the pointer, not delete[]. Since
the memory was originally new[]'ed, this would lead to UB.

Oops. Thanks. Forgot all about that.

LR

 

[Old Wolf]

char *fooFunc(int Bar)
{
char *returnArr = new char[8];
int stuff;

// do some stuff with Bar

sprintf(returnArr,"%2d",stuff);
return(returnArr);
}

This code might cause a buffer overflow too - the %2d
does not restrict the output to 2 characters. If stuff
is greater than 9999999 or less than -999999 you will
be in trouble.

 

[Jason]

This code might cause a buffer overflow too - the %2d
does not restrict the output to 2 characters. If stuff
is greater than 9999999 or less than -999999 you will
be in trouble.

Funny, because I just got that exact problem. Thanks for the tip!

- Jason