Memory leaks with realloc()

M

Marlene Stebbins

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;
}
 
E

Eric Sosman

Marlene said:
[...] 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;'?
 
M

Marlene Stebbins

Eric said:
One additional remark: Why write




... 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
 
V

Villy Kruse

Eric said:
[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
 
E

Eric Sosman

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."
 
V

Villy Kruse

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
 
M

Marlene Stebbins

Villy said:
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
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,483
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top