memory leak problem

[coandrei]

That's the task (should use copy by value semantics):

typedef struct {
  // street address
  char street[30];
  // postal code
  int postalCode;
} Address;

/*
 * This function creates a new address with a certain street address
 * and a certain postal code.
 */
Address addressConstruct(const char *street, int postalCode);
// Accessor for the street address. The caller
// of this function must free() the return value when
// he/she is done with it!
char *addressGetStreet(Address addr);

Here is my implementation

Address addressConstruct(const char *street, int postalCode)
{
	Address add;

	strcpy(add.street, street);
	add.postalCode = postalCode;

	return add;
}
char *addressGetStreet(Address addr)
{
	char* ch;
	
	ch = (char*)malloc((strlen(addr.street)+1)*sizeof(char));
	strcpy(ch, addr.street);
	
	return ch;
}

Could someone tell me how to use DevC++ to detect memory leaks?
Thanks in advance.

[anantwakode]

Hi,

You can use memory validator software tool for detecting memory leaks....
http://www.softwareverify.com/cpp/memory/index.html

also you can create wrapper for Malloc() and free and by keeping track of memory allocation. you can detect memory leaks.

-Anant

[coandrei]

http://www.softwareverify.com/cpp/memory/index.html[/url]
-Anant

the link does not work.

Actually function "addressGetStreet" creates memory leaking. I guess I didn't understand correctly the following text:

// Accessor for the street address. The caller
// of this function must free() the return value when
// he/she is done with it!

Could someone explain what does it mean?

[Paul McKenzie]

the link does not work.

Actually function "addressGetStreet" creates memory leaking. I guess I didn't understand correctly the following text:

// Accessor for the street address. The caller
// of this function must free() the return value when
// he/she is done with it!

Could someone explain what does it mean?

It means what it says -- you called malloc() and returned the pointer. So who will call free()?

Regards,

Paul McKenzie

[anantwakode]

the link does not work.

Actually function "addressGetStreet" creates memory leaking. I guess I didn't understand correctly the following text:

// Accessor for the street address. The caller
// of this function must free() the return value when
// he/she is done with it!

Could someone explain what does it mean?

Link is working now......there must be some server problem I cheked it.

in your shown code actually there is no memory leak....
addressGetStreet() function allocates memory and return pointer...it means its the responcibility of the caller to deallocate the memory. or you can do is return the ref to address string array.....


-Anant

[coandrei]

Link is working now......there must be some server problem I cheked it.

in your shown code actually there is no memory leak....
addressGetStreet() function allocates memory and return pointer...it means its the responcibility of the caller to deallocate the memory. or you can do is return the ref to address string array.....


-Anant

I solved it now. The problem was that I was using directly the function in a printf() statement similar to:

printf("%s\n", addressGetStreet(a1));

instead of

char *street;
street = addressGetStreet(a1);
printf("%s\n", street);
free(street);

Thank you for explanations

[Paul McKenzie]

I solved it now. The problem was that I was using directly the function in a printf() statement similar to:

printf("%s\n", addressGetStreet(a1));

instead of

char *street;
street = addressGetStreet(a1);
printf("%s\n", street);
free(street);

Thank you for explanations

Sorry, but you didn't fix the problem.

If the function returns a char*, it is perfectly acceptable in a printf() function that accepts a "%s" format specifier. All you really did was move the code around a little bit, thereby moving the bug to another area of your program.

So as an analogy, instead of adding 2 + 2, you added 1 + 3. They both still equal 4. Different numbers, same results.

Regards,

Paul McKenzie

[Paul McKenzie]

So you need to change your code back to where it was crashing, and really fix the problem. The other way is just masking the bug, and you don't know when the bug will appear again.

Regards,

Paul McKenzie

[ZuK]

If the function returns a char*, it is perfectly acceptable in a printf() function that accepts a "%s" format specifier.

I have to disagree. Using such a function ( one that allocates memory that the caller has to deallocate ) in a printf() statement will always result in a memory leak.
coandrei's solution is the only way to use such a function in a printf() statement.
Kurt

[vlongsoft]

Why don't you change your routine to avoid "malloc" ?

char *addressGetStreet(Address addr)
{
	return (char*)addr.street;;
}

[SuperKoko]

Why don't you change your routine to avoid "malloc" ?

• It introduces a life time issue: The string becomes invalid as soon as the Address object dies... Actually, your code is buggy because Address is passed by value, so the pointer is invalid as soon as the addressGetStreet function is exited.
• It breaks encapsulation, for several reasons.
  
  • If, for some reason, the street string has to be computed on demand, it won't work properly unless a cache (which may use too much memory) is used.
  • It isn't clear whether modifying (if it's possible) the street of an address, can modify existing references to this street, or not.
    If you require that the street reference returned by addressGetStreet becomes a "mirror" of the street string inside the structure, then, encapsulation is obviously very flawed, as even a simple implementation change, such as a resizable malloc'ed buffer, would be impossible.
  • Giving a non-const access to an internal buffer, means that the user can modify the string inside your structure without any control. So, for example, you won't be able to keep track of the length of the string.
    This last one can easily be corrected. Replacing "char*" with "const char*".
  
  Conclusion: Encapsulation is broken to such an extent, that it's merely equivalent to a public array member!