Question on free() ing a hash table

[Chad]

Excercise 6-5

Write a functon undef that will remove a name and defintion from the
table maintained by lookup and install.

Code:

unsigned hash(char *s);

void undef(char *s)
{
int h;
struct nlist *prev, *np;

prev = NULL;
h = hash(s);
for(np = hashtab[h]; np!=NULL; np = np->next){
if(strcmp(s, np->name) == 0)
break;
prev = np;
}

if(np !=NULL) {
if(prev == NULL)
hashtab[h] = np->next;
else
prev->next = np->next;
free((void *) np->name);
free((void *) np->defn);
free((void *) np);
}
}

The question is, how come I have to free() both np->name and np->defn?
Ie, how come I can't just free() np?

[santosh]

Chad wrote:

Excercise 6-5

Write a functon undef that will remove a name and defintion from the
table maintained by lookup and install.

Code:

unsigned hash(char *s);

void undef(char *s)
{
int h;
struct nlist *prev, *np;

prev = NULL;
h = hash(s);
for(np = hashtab[h]; np!=NULL; np = np->next){
if(strcmp(s, np->name) == 0)
break;
prev = np;
}

if(np !=NULL) {
if(prev == NULL)
hashtab[h] = np->next;
else
prev->next = np->next;
free((void *) np->name);
free((void *) np->defn);
free((void *) np);
}
}

The question is, how come I have to free() both np->name and np->defn?
Ie, how come I can't just free() np?

Because memory for them was allocated separately.
You need a call to free() for every call to malloc(). You construct the
struct np with three calls to malloc - one for the struct itself and
two more to initialise it's members 'name' and 'defn' to point to
usable storage. Thus when destroying the struct you need to deallocate
storage pointed to by these members too.

In particular if 'name' and 'defn' are the only pointers that point to
their storage, then you must free() them _before_ calling free() on np,
or else you'll create a memory leak.

Note that the casts to void * within the calls to free() are not needed.

[Chad]

On Oct 25, 7:02 pm, santosh <santosh....@gm ail.comwrote:

Chad wrote:

Excercise 6-5

Write a functon undef that will remove a name and defintion from the
table maintained by lookup and install.

Code:

unsigned hash(char *s);

void undef(char *s)
{
int h;
struct nlist *prev, *np;

prev = NULL;
h = hash(s);
for(np = hashtab[h]; np!=NULL; np = np->next){
if(strcmp(s, np->name) == 0)
break;
prev = np;
}

if(np !=NULL) {
if(prev == NULL)
hashtab[h] = np->next;
else
prev->next = np->next;
free((void *) np->name);
free((void *) np->defn);
free((void *) np);
}
}

The question is, how come I have to free() both np->name and np->defn?
Ie, how come I can't just free() np?

Because memory for them was allocated separately.
You need a call to free() for every call to malloc(). You construct the
struct np with three calls to malloc - one for the struct itself and
two more to initialise it's members 'name' and 'defn' to point to
usable storage. Thus when destroying the struct you need to deallocate
storage pointed to by these members too.

In particular if 'name' and 'defn' are the only pointers that point to
their storage, then you must free() them _before_ calling free() on np,
or else you'll create a memory leak.

Note that the casts to void * within the calls to free() are not needed.

The question was taken from page 145 in the "The C Programming
Langauge" by K & R. I think what is tripping me up when trying to free
the structure is the following line of code on page 145.

In install(), they have the following line of code:

if((np->defn = strdup(defn)) == NULL)
return NULL.

For reasons that still elude me, every time I see this line of code, I
keep thinking that memory ISN'T allocated for np->defn.

[Chad]

On Oct 25, 7:02 pm, santosh <santosh....@gm ail.comwrote:

Chad wrote:

Excercise 6-5

Write a functon undef that will remove a name and defintion from the
table maintained by lookup and install.

Code:

unsigned hash(char *s);

void undef(char *s)
{
int h;
struct nlist *prev, *np;

prev = NULL;
h = hash(s);
for(np = hashtab[h]; np!=NULL; np = np->next){
if(strcmp(s, np->name) == 0)
break;
prev = np;
}

if(np !=NULL) {
if(prev == NULL)
hashtab[h] = np->next;
else
prev->next = np->next;
free((void *) np->name);
free((void *) np->defn);
free((void *) np);
}
}

The question is, how come I have to free() both np->name and np->defn?
Ie, how come I can't just free() np?

Because memory for them was allocated separately.
You need a call to free() for every call to malloc(). You construct the
struct np with three calls to malloc - one for the struct itself and
two more to initialise it's members 'name' and 'defn' to point to
usable storage. Thus when destroying the struct you need to deallocate
storage pointed to by these members too.

In particular if 'name' and 'defn' are the only pointers that point to
their storage, then you must free() them _before_ calling free() on np,
or else you'll create a memory leak.

Note that the casts to void * within the calls to free() are not needed.

The question was taken from page 145 in the "The C Programming
Langauge" by K & R (Second Edition). I think what is tripping me up
when trying to free the structure is the following line of code on
page 145.

In install(), they have the following line of code:

if((np->defn = strdup(defn)) == NULL)
return NULL;

For reasons that still elude me, every time I see this line of code, I
keep thinking that memory ISN'T allocated for np->defn.

[santosh]

Chad wrote:

On Oct 25, 7:02 pm, santosh <santosh....@gm ail.comwrote:

>Chad wrote:

Excercise 6-5

Write a functon undef that will remove a name and defintion from
the table maintained by lookup and install.

Code:

unsigned hash(char *s);

void undef(char *s)
{
int h;
struct nlist *prev, *np;

prev = NULL;
h = hash(s);
for(np = hashtab[h]; np!=NULL; np = np->next){
if(strcmp(s, np->name) == 0)
break;
prev = np;
}

if(np !=NULL) {
if(prev == NULL)
hashtab[h] = np->next;
else
prev->next = np->next;
free((void *) np->name);
free((void *) np->defn);
free((void *) np);
}
}

The question is, how come I have to free() both np->name and
np->defn? Ie, how come I can't just free() np?

Because memory for them was allocated separately.
You need a call to free() for every call to malloc(). You construct
the struct np with three calls to malloc - one for the struct itself
and two more to initialise it's members 'name' and 'defn' to point to
usable storage. Thus when destroying the struct you need to
deallocate storage pointed to by these members too.

In particular if 'name' and 'defn' are the only pointers that point
to their storage, then you must free() them _before_ calling free()
on np, or else you'll create a memory leak.

Note that the casts to void * within the calls to free() are not
needed.

The question was taken from page 145 in the "The C Programming
Langauge" by K & R (Second Edition). I think what is tripping me up
when trying to free the structure is the following line of code on
page 145.

In install(), they have the following line of code:

if((np->defn = strdup(defn)) == NULL)
return NULL;

For reasons that still elude me, every time I see this line of code, I
keep thinking that memory ISN'T allocated for np->defn.

This line attempts to duplicate the string pointed to by 'defn' passed
to strdup() at 'np-defn.' If strdup() fails it returns NULL. Otherwise
you have to explicitly free() memory allocated by it.

Note that strdup() is not defined by ISO C, though it's a common
extension.

See man strdup on UNIX systems.

[pete]

santosh wrote:

>
Chad wrote:

On Oct 25, 7:02 pm, santosh <santosh....@gm ail.comwrote:

Chad wrote:
Excercise 6-5

Write a functon undef that will remove a name and defintion from
the table maintained by lookup and install.

Code:

unsigned hash(char *s);

void undef(char *s)
{
int h;
struct nlist *prev, *np;

prev = NULL;
h = hash(s);
for(np = hashtab[h]; np!=NULL; np = np->next){
if(strcmp(s, np->name) == 0)
break;
prev = np;
}

if(np !=NULL) {
if(prev == NULL)
hashtab[h] = np->next;
else
prev->next = np->next;
free((void *) np->name);
free((void *) np->defn);
free((void *) np);
}
}

The question is, how come I have to free() both np->name and
np->defn? Ie, how come I can't just free() np?

Because memory for them was allocated separately.
You need a call to free() for every call to malloc(). You construct
the struct np with three calls to malloc
- one for the struct itself
and two more to initialise it's members 'name'
and 'defn' to point to
usable storage. Thus when destroying the struct you need to
deallocate storage pointed to by these members too.

In particular if 'name' and 'defn' are the only pointers that point
to their storage, then you must free() them _before_ calling free()
on np, or else you'll create a memory leak.

Note that the casts to void * within the calls to free() are not
needed.

The question was taken from page 145 in the "The C Programming
Langauge" by K & R (Second Edition). I think what is tripping me up
when trying to free the structure is the following line of code on
page 145.

In install(), they have the following line of code:

if((np->defn = strdup(defn)) == NULL)
return NULL;

For reasons that still elude me,
every time I see this line of code, I
keep thinking that memory ISN'T allocated for np->defn.

This line attempts to duplicate the string pointed to by 'defn' passed
to strdup() at 'np-defn.' If strdup() fails it returns NULL. Otherwise
you have to explicitly free() memory allocated by it.

Note that strdup() is not defined by ISO C, though it's a common
extension.

See man strdup on UNIX systems.

strdup is defined on page 143 of K&R2.

--
pete

[vipvipvipvipvip.ru]

On Oct 26, 8:50 am, pete <pfil...@mindsp ring.comwrote:

>
strdup is defined on page 143 of K&R2.

K&R2 is old.

[Richard Bos]

vi************* ***@gmail.com wrote:

On Oct 26, 8:50 am, pete <pfil...@mindsp ring.comwrote:

strdup is defined on page 143 of K&R2.

K&R2 is old.

True, but irrelevant. strdup() is normally considered a Unixoid function
here (and with reason), but in the context of the OP's problem, it is
allowed, simply because that problem itself is from K&R, and presumes
the existence of K&R's, just a little earlier defined, strdup(), not
POSIX's one.

Richard

[santosh]

vi************* ***@gmail.com wrote:

On Oct 26, 8:50 am, pete <pfil...@mindsp ring.comwrote:

>>
strdup is defined on page 143 of K&R2.

K&R2 is old.

It's age is irrelevant to the problem at hand. As pete says strdup is
defined elsewhere in the book.

I think that Chad's confusion stems from the fact that he believes 'np'
may be uninitialised. It is however initialised by the function lookup.

[gw7rib]

On 26 Oct, 03:29, Chad <cdal...@gmail. comwrote:

In install(), they have the following line of code:

if((np->defn = strdup(defn)) == NULL)
return NULL;

For reasons that still elude me, every time I see this line of code, I
keep thinking that memory ISN'T allocated for np->defn.

Perhaps you could try rewriting it as:

np->defn = strdup(defn);
if(np->defn == NULL) return NULL;

Readability is important, and this version is only a smidgeon less
efficient.

Paul.