By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
445,797 Members | 1,794 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 445,797 IT Pros & Developers. It's quick & easy.

yet another memory leak

P: n/a
Hello

I was expecting the method below to break ...but it seems to behave at
this point.
It's 'posed to reverse a string ...

char* server::reverse(char* fromClient,int length)
{
char* reverse=(char*) malloc(length+1);

for(int pos=0;pos<length;pos++)
{
reverse[pos] = fromClient[abs(pos-length+1)];
}
return reverse;
}
'reverse' is local variable so it should not be pointing to a valid
memory location after method exits.

Why does it (seem to) work ...?

(How can I break it? ...I'm sure it's wrong cause it's too simple to be
any good )

these are my compiler flags -g -O2 -D_REENTRANT -Wall -Werror
-D__EXTENSIONS__
Thank you

A.

Nov 3 '06 #1
Share this Question
Share on Google+
5 Replies


P: n/a

Amchi wrote:
Hello

I was expecting the method below to break ...but it seems to behave at
this point.
It's 'posed to reverse a string ...

char* server::reverse(char* fromClient,int length)
{
char* reverse=(char*) malloc(length+1);

for(int pos=0;pos<length;pos++)
{
reverse[pos] = fromClient[abs(pos-length+1)];
}
return reverse;
}
'reverse' is local variable so it should not be pointing to a valid
memory location after method exits.

Why does it (seem to) work ...?

(How can I break it? ...I'm sure it's wrong cause it's too simple to be
any good )

these are my compiler flags -g -O2 -D_REENTRANT -Wall -Werror
-D__EXTENSIONS__
Thank you

A.
Just because the memory isn't valid, doesn't mean you haven't cleared
it yet. To clear it use memset(reverse, 0, length+1);

Nov 4 '06 #2

P: n/a

Amchi wrote:
Hello

I was expecting the method below to break ...but it seems to behave at
this point.
It's 'posed to reverse a string ...

char* server::reverse(char* fromClient,int length)
{
char* reverse=(char*) malloc(length+1);

for(int pos=0;pos<length;pos++)
{
reverse[pos] = fromClient[abs(pos-length+1)];
}
You forgot to null-terminate the reversed string. That's why you
allocated length + 1 bytes, right? This is needed:

reverse[length] = 0;

Why are you using abs() there? Since the loop body executes only if the
guard condition pos < length is true, it must be that pos - length is
negative, and that pos - length + 1 is negative or zero.

So all that the abs function does is change the sign, which you could
achieve more clearly and directly by taking the additive inverse of the
expression:

length - pos - 1

This is easier to verify. If pos is zero, then it takes length - 1,
which is indeed the last character of the string. So the last character
is taken into position 0, and everything else follows.
return reverse;
}
'reverse' is local variable so it should not be pointing to a valid
memory location after method exits.
Because reverse is a local variable, it simply doesn't exist after the
method exits.

However, the memory location which it pointed to still does exist,
because it came from a dynamic allocator.

The reversed string, and the varaible pointing to it, are separate
objects with different lifetimes.

Nov 4 '06 #3

P: n/a

"Amchi" <am*********@gmail.comwrote in message
news:11**********************@m73g2000cwd.googlegr oups.com...
Hello

I was expecting the method below to break ...but it seems to behave at
this point.
It's 'posed to reverse a string ...

char* server::reverse(char* fromClient,int length)
{
char* reverse=(char*) malloc(length+1);
reverse is a local variable, that holds the address of a character pointer.
Memory is allocated, and the address of that memory is stored in reverse.
for(int pos=0;pos<length;pos++)
{
reverse[pos] = fromClient[abs(pos-length+1)];
}
return reverse;
Here you are storing the address of the memory that was allocated, that was
stored in reverse. Since this address is returned (pushed onto the stack in
most applications) after this returns reverse is no longer needed, it was
just used to store the address.

Just as if this function was returning an int, and a variable was declared
int ReturnVal;
After you do a
return ReturnVal;
it doesn't matter that ReturnVal goes out of scope, it's contents were
already returned.
}
'reverse' is local variable so it should not be pointing to a valid
memory location after method exits.

Why does it (seem to) work ...?

(How can I break it? ...I'm sure it's wrong cause it's too simple to be
any good )

these are my compiler flags -g -O2 -D_REENTRANT -Wall -Werror
-D__EXTENSIONS__
Thank you

A.

Nov 4 '06 #4

P: n/a

Amchi wrote in message
<11**********************@m73g2000cwd.googlegroups .com>...
>Hello
I was expecting the method below to break ...but it seems to behave at
this point.
It's 'posed to reverse a string ...

char* server::reverse(char* fromClient, int length){
char* reverse=(char*) malloc(length+1);
for(int pos=0;pos<length;pos++){
reverse[pos] = fromClient[abs(pos-length+1)];
}
return reverse;
}

'reverse' is local variable so it should not be pointing to a valid
memory location after method exits.
But you returned the address of a piece of 'malloc'ed memory. Be sure to
'free' it.
Why don't you use 'new/delete'? ...and 'std::string'?

(How can I break it? ...I'm sure it's wrong cause it's too simple to be any
good )

// It's already broken!
char bust[] = "...I'm sure it's wrong cause it's too simple to be any
good";
char *busted = bust;
server serv;
while( busted = serv.reverse( bust, 68) ){
std::cout<<busted<<std::endl;
}
// How is your memory about here?
// Be sure to check your CPU temperature every few minutes!
#include <iostream>
#include <ostream>
#include <string>
#include <algorithm>
int main(){
std::string Rev( "I want to reverse this string!" );
std::cout<<Rev<<std::endl;
std::reverse( Rev.begin(), Rev.end() );
std::cout<<Rev<<std::endl;
return 0;
} // main()

--
Bob R
POVrookie
Nov 4 '06 #5

P: n/a
BobR wrote:
Amchi wrote in message
<11**********************@m73g2000cwd.googlegroups .com>...
Hello
I was expecting the method below to break ...but it seems to behave at
this point.
It's 'posed to reverse a string ...

char* server::reverse(char* fromClient, int length){
char* reverse=(char*) malloc(length+1);
for(int pos=0;pos<length;pos++){
reverse[pos] = fromClient[abs(pos-length+1)];
}
return reverse;
}

'reverse' is local variable so it should not be pointing to a valid
memory location after method exits.

But you returned the address of a piece of 'malloc'ed memory. Be sure to
'free' it.
Why don't you use 'new/delete'? ...and 'std::string'?

(How can I break it? ...I'm sure it's wrong cause it's too simple to be any
good )

// It's already broken!
char bust[] = "...I'm sure it's wrong cause it's too simple to be any
good";
char *busted = bust;
server serv;
while( busted = serv.reverse( bust, 68) ){
std::cout<<busted<<std::endl;
}
// How is your memory about here?
My CPU cried for help.

// Be sure to check your CPU temperature every few minutes!
....did not dare .....(CPU is being abused daily and expected to melt
soon)

>

#include <iostream>
#include <ostream>
#include <string>
#include <algorithm>
int main(){
std::string Rev( "I want to reverse this string!" );
std::cout<<Rev<<std::endl;
std::reverse( Rev.begin(), Rev.end() );
std::cout<<Rev<<std::endl;
return 0;
} // main()

--
Bob R
POVrookie
Thanks ....I appreciate your help.

Nov 4 '06 #6

This discussion thread is closed

Replies have been disabled for this discussion.