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

Memory leaks with realloc()

P: n/a
The bigint struct defines a big integer and represents it as a string of
characters:

typedef struct bigint {
int sign;
int size;
int initflag;
char *number;
} bigint;

In the code below I am initializing a bigint to zero with malloc() then
realloc()-ing in bigAlloc() and the two assignment functions. Do I have
a memory leak anywhere in this code? In particular, once the bigint has
been initialized do I need to free the currently allocated memory before
reallocating in bigAlloc() and the bigAssign()s?

/* initialize a bigint to zero */
void bigInit(bigint *a)
{
if((a->number = malloc(2))==NULL)
{
fprintf(stderr, "malloc failed in bigInit()\n");
exit(EXIT_FAILURE);
}
a->sign = POS;
a->size = 1;
a->initflag = 1;
a->number[0] = '0';
a->number[1] = '\0';
return;
}

/* allocate memory for a bigint */
void bigAlloc(bigint *a, int bytes)
{
if(a->initflag != 1)
{
bigInit(a);
}

if((a->number = realloc(a->number, bytes * sizeof(*a->number)))==NULL)
{
fprintf(stderr, "bigAlloc failed\n");
exit(EXIT_FAILURE);
}
return;
}

/* bigint assignment: a = b */
void bigAssign(bigint *a, bigint b)
{
bigAlloc(a, b.size+1);
if(b.sign == POS) a->sign = POS;
if(b.sign == NEG) a->sign = NEG;
a->size = b.size;
strcpy(a->number, b.number);
return;
}

/* assignment */
void bigAssignByRef(bigint *a, bigint *b)
{
bigAlloc(a, b->size+1);
if(b->sign == POS) a->sign = POS;
if(b->sign == NEG) a->sign = NEG;
a->size = b->size;
strcpy(a->number, b->number);
return;
}
Nov 14 '05 #1
Share this Question
Share on Google+
7 Replies


P: n/a


Marlene Stebbins wrote:
[...] Do I have
a memory leak anywhere in this code?
You have a potential leak in bigAlloc() at
if((a->number = realloc(a->number, ...))==NULL)
.... because if realloc() fails you will overwrite the still-
valid pointer `a->number' with NULL. Unless you have another
pointer somewhere that retains the former value of `a->number',
you have lost all ability to do anything with the memory that
`a->number' used to point to; you have "leaked" that memory.

However, in your case the leak is not serious, because if
realloc() fails you call `exit(EXIT_FAILURE)'. The duration
of the leak will be short, and its effect on the subsequent
operation of the program practically nil ;-)
In particular, once the bigint has
been initialized do I need to free the currently allocated memory before
reallocating in bigAlloc() and the bigAssign()s?
You should *not* free() a memory area and then try to
realloc() it. The first argument to realloc() must be NULL
or must be a pointer to a currently-valid memory area as
allocated by malloc(), calloc(), or a previous realloc() --
if you hand realloc() a pointer to anything else, you get
undefined behavior.

In your case, though, it might make more sense to free()
and malloc() than to realloc() -- you do not need to preserve
the former contents of the memory `a->number' points to; you
are about to insert brand-new content. Thus, whatever extra
work realloc() undertakes to preserve the original bytes is
simply a waste. Indeed, free()/malloc() may succeed where a
realloc() would have failed; for your purposes realloc() is
not only less efficient, but less effective.

One additional remark: Why write
if(b.sign == POS) a->sign = POS;
if(b.sign == NEG) a->sign = NEG;


.... instead of `b.sign = a->sign;'?

--
Er*********@sun.com

Nov 14 '05 #2

P: n/a
Eric Sosman wrote:
[snip]

One additional remark: Why write
if(b.sign == POS) a->sign = POS;
if(b.sign == NEG) a->sign = NEG;


.... instead of `b.sign = a->sign;'?

You meant a->sign = b.sign, I bet.

Krishanu
Nov 14 '05 #3

P: n/a
Eric Sosman wrote:


One additional remark: Why write

if(b.sign == POS) a->sign = POS;
if(b.sign == NEG) a->sign = NEG;

... instead of `b.sign = a->sign;'?


Because I'm simple-minded. After I get everything working I'll try to
get rough edges like that cleaned up. Thanks for your help.

Marlene
Nov 14 '05 #4

P: n/a
On Wed, 23 Feb 2005 21:37:31 +0530,
Krishanu Debnath <kr******@cal.interrasystems.com> wrote:

Eric Sosman wrote:
[snip]

One additional remark: Why write
if(b.sign == POS) a->sign = POS;
if(b.sign == NEG) a->sign = NEG;


.... instead of `b.sign = a->sign;'?

You meant a->sign = b.sign, I bet.


Is it even remotely possible that b.sign could be neither POS nor NEG,
and what should be done in this case?

Villy
Nov 14 '05 #5

P: n/a
Villy Kruse wrote:
On Wed, 23 Feb 2005 21:37:31 +0530,
Krishanu Debnath <kr******@cal.interrasystems.com> wrote:
Eric Sosman wrote:
One additional remark: Why write
if(b.sign == POS) a->sign = POS;
if(b.sign == NEG) a->sign = NEG;

.... instead of `b.sign = a->sign;'?
You meant a->sign = b.sign, I bet.


Krishanu wins that bet ...
Is it even remotely possible that b.sign could be neither POS nor NEG,
and what should be done in this case?


I thought about that, but decided not to worry about
it for two reasons: First, the obvious intent of the function
was to make a new copy of the `b' struct and its contents,
and second, leaving `a->sign' uninitialized (freshly obtained
from malloc() and hence with indeterminate content) could not
possibly be "right."

--
Eric Sosman
es*****@acm-dot-org.invalid
Nov 14 '05 #6

P: n/a
On Thu, 24 Feb 2005 08:44:27 -0500,
Eric Sosman <es*****@acm-dot-org.invalid> wrote:

Is it even remotely possible that b.sign could be neither POS nor NEG,
and what should be done in this case?


I thought about that, but decided not to worry about
it for two reasons: First, the obvious intent of the function
was to make a new copy of the `b' struct and its contents,
and second, leaving `a->sign' uninitialized (freshly obtained
from malloc() and hence with indeterminate content) could not
possibly be "right."


So you could assert (b.sign == POS || b.sign == NET); Perhaps
that is already done earlier in the code. Defensive coding helps
find bugs.
Villy
Nov 14 '05 #7

P: n/a
Villy Kruse wrote:
On Thu, 24 Feb 2005 08:44:27 -0500,
Eric Sosman <es*****@acm-dot-org.invalid> wrote:
Is it even remotely possible that b.sign could be neither POS nor NEG,
and what should be done in this case?


I thought about that, but decided not to worry about
it for two reasons: First, the obvious intent of the function
was to make a new copy of the `b' struct and its contents,
and second, leaving `a->sign' uninitialized (freshly obtained
from malloc() and hence with indeterminate content) could not
possibly be "right."

So you could assert (b.sign == POS || b.sign == NET); Perhaps
that is already done earlier in the code. Defensive coding helps
find bugs.
Villy


Sign is set to POS by bigInit. If by some chance a bigint was not
initialized after it was declared, it would be initialialized by
bigAlloc which looks at initflag and calls bigInit if initflag != 1.
BTW, the assert statement you have written above wouldn't help much. :-)
Speaking of finding bugs, you can see the code for the entire big number
library at:

http://members.shaw.ca/bystander/bignum.html

If you have some time on your hands, take a look at it and see if you
can find the bug in division. Everything else works. If you would like
to email me, you can figure out my email address from the URL.

Marlene
Nov 14 '05 #8

This discussion thread is closed

Replies have been disabled for this discussion.