473,287 Members | 3,253 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,287 software developers and data experts.

freeing simple linked list

I am working on this example from a book "C Primer Plus" by Prata 4th
edition - p. 672. There is no erata on this problem at the publisher's
website.

1) Is it a violation of copyright laws to post example code from a
book to a newsgroup?

2) The program crashes as it tries to free memory and I would like to
know the best way to correct THIS section of the code. By assigning
current = head, current now points to the first structure in the
linked list. By freeing the first structure it frees all of the other
structures in the linked list below it. As a result, why isn't
current == NULL now ?

3) Is the author assigning current = current->next to make sure
everything gets freed? It seems unecessary.

Thanks,
disco

problem section:
/* Program is done, so free allocated memory */
current = head;
while (current != NULL)
{
free(current);
current = current->next;
}
printf("Bye!\n");

entire program:
/* Films2.c -- using a linked list of structures */
#include <stdio.h>
#include <stdlib.h> /* contains the malloc prototype */
#include <string.h> /* contains the strcpy prototype */
#define TSIZE 45 /* size of array to hold title */

struct film
{
char title[TSIZE];
int rating;
struct film *next; /* points to next struct in list */
};

int main (void)
{
struct film *head = NULL;
struct film *prev, *current;
char input[TSIZE];

printf("Enter first movie title:");
while (gets(input) !=NULL && input[0] != '\0')
{
current = (struct film*) malloc(sizeof(struct film));

if (head == NULL) /* first structure */
head = current;
else /* subsequent structures */
prev->next = current;

current->next = NULL;

strcpy(current->title, input);
printf("Enter your rating [0 to 10]: ");
scanf("%d", &current->rating);
while (getchar() != '\n')
continue;
printf("Please enter next movie title [enter to end]: ");
prev = current;
}

if (head == NULL)
printf("No data entered. ");
else
printf("Here is the movie list: \n");

current = head;

while (current != NULL)
{
printf("Movie: %s -- Rating: %d\n", current->title,
current->rating);
current = current->next;
}

/* Program is done, so free allocated memory */
current = head;
while (current != NULL)
{
free(current);
/* current = current->next; */
}
printf("Bye!\n");

return 0;
}
Nov 13 '05 #1
5 22940
jp********@hotmail.com (disco) writes:
I am working on this example from a book "C Primer Plus" by Prata 4th
edition - p. 672. There is no erata on this problem at the publisher's
website.

1) Is it a violation of copyright laws to post example code from a
book to a newsgroup?
Unless you have a license to post the material, you can only legally do so
under "fair use" provisions. What constitutes "fair use" varies between
different contries, and many contries don't have an exact definition in
the law, but leave it up to the courts to decide if some use is "fair".
2) The program crashes as it tries to free memory and I would like to
know the best way to correct THIS section of the code. By assigning
current = head, current now points to the first structure in the
linked list. By freeing the first structure it frees all of the other
structures in the linked list below it.
No, freeing the head does not automatically free all the other nodes.
As a result, why isn't current == NULL now ?
Passing a pointer to the `free' function frees the memory pointed to, but
doesn't change the value of the pointer itself. The latter would be
impossible as function arguments are always passed by value in C. Therefore,
`free' cannot modify the pointer value.

So you end up with an invalid pointer (it doesn't point to a valid memory
address any longer), but not a null pointer.
3) Is the author assigning current = current->next to make sure
everything gets freed? It seems unecessary.
It is necessary to free the whole list, which must be done node by
node. However, the way the author does this is incorrect.
/* Program is done, so free allocated memory */
current = head;
while (current != NULL)
{
free(current);
current = current->next;
}


It causes undefined behavior to dereference `current' (as in
`current->next') after it has been passed to `free'. The correct way
is to record a pointer to the next node in a temporary variable
/before/ the current node is freed:

current = head;
while (current != NULL)
{
struct node *const next_node = current->next;

free (current);
current = next_node;
}

Martin
Nov 13 '05 #2
disco wrote:
I am working on this example from a book "C Primer Plus" by Prata 4th
edition - p. 672. There is no erata on this problem at the publisher's
website.

1) Is it a violation of copyright laws to post example code from a
book to a newsgroup?
It probably comes under "fair use" if you don't overdo it.
2) The program crashes as it tries to free memory and I would like to
know the best way to correct THIS section of the code. By assigning
current = head, current now points to the first structure in the
linked list. By freeing the first structure it frees all of the other
structures in the linked list below it. As a result, why isn't
current == NULL now ?

3) Is the author assigning current = current->next to make sure
everything gets freed? It seems unecessary.

Thanks,
disco

problem section:
/* Program is done, so free allocated memory */
current = head;
while (current != NULL)
{
free(current);
current = current->next;


Undefined behaviour. Having freed current, you have no right to use its
value.

Here's how it should be done:

while(current != NULL)
{
tmp = current; /* tmp needs to be a pointer of the same type as current */
current = current->next;
free(tmp);
}

The error is a simple one. If there are many such errors, it calls the value
of the book into question.

--
Richard Heathfield : bi****@eton.powernet.co.uk
"Usenet is a strange place." - Dennis M Ritchie, 29 July 1999.
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html
K&R answers, C books, etc: http://users.powernet.co.uk/eton
Nov 13 '05 #3
Malcolm wrote:
.... snip ...
The way to do it is using a recursive function.

void freelist(struct film *ptr)
{
if(ptr->next)
freelist(ptr->next);
free(ptr);
}

If you don't want recursion you can write a somewhat faster
iterative routine with two pointers.


Which is probably wise, since lists can be arbitrarily long. At
any rate, your solution fails if called with a NULL ptr. For a
recursive solution better might be:

void freelist(void *ptr)
{
struct film *tmp = ptr;

if (tmp) {
freelist(tmp->next);
free(ptr);
}
}

which can work with various arrangements of the struct.

--
Chuck F (cb********@yahoo.com) (cb********@worldnet.att.net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net> USE worldnet address!
Nov 13 '05 #4
CBFalconer wrote:

Malcolm wrote:

... snip ...

The way to do it is using a recursive function.

void freelist(struct film *ptr)
{
if(ptr->next)
freelist(ptr->next);
free(ptr);
}

If you don't want recursion you can write a somewhat faster
iterative routine with two pointers.


Which is probably wise, since lists can be arbitrarily long. At
any rate, your solution fails if called with a NULL ptr. For a
recursive solution better might be:

void freelist(void *ptr)
{
struct film *tmp = ptr;

if (tmp) {
freelist(tmp->next);
free(ptr);
}
}

which can work with various arrangements of the struct.


Of course it is rather silly for simple lists. But a variant
handles binary trees nicely:

void freetree(void *ptr)
{
struct film *tmp = ptr;

if (tmp) {
freetree(tmp->left);
freetree(tmp->right);
free(ptr);
}
}

since the recursion depth is usually logarithmic in tree size.

--
Chuck F (cb********@yahoo.com) (cb********@worldnet.att.net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net> USE worldnet address!

Nov 13 '05 #5
Richard Heathfield <do******@address.co.uk.invalid> wrote in message news:<bi**********@hercules.btinternet.com>...
Undefined behaviour. Having freed current, you have no right to use its
value.

Here's how it should be done:

while(current != NULL)
{
tmp = current; /* tmp needs to be a pointer of the same type as current */
current = current->next;
free(tmp);
}

The error is a simple one. If there are many such errors, it calls the value
of the book into question.


Thank you everyone for your feedback. It was very informative and
helpful.

I have found this book very helpful, both as a reference and a
tutorial, given my particular experience and requirements to use C.
Has anyone else used it or had problems with it?

Thanks again,
disco
Nov 13 '05 #6

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

13
by: na1paj | last post by:
here's a simple linked list program. the DeleteNode function is producing an infinit loop i think, but i can't figure out where.. #include <stdio.h> typedef struct { char *str; //str is a...
10
by: Ben | last post by:
Hi, I am a newbie with C and am trying to get a simple linked list working for my program. The structure of each linked list stores the char *data and *next referencing to the next link. The...
1
by: Tim | last post by:
I can't seem to figure out why this very simple linked list wont build.. I mean, there is no intelligence, just add to end. Anyway, please let me know if something i can do will make head (the...
1
by: drewy2k12 | last post by:
Heres the story, I have to create a doubly linked list for class, and i have no clue on how to do it, i can barely create a single linked list. It has to have both a head and a tail pointer, and...
4
by: eksamor | last post by:
I have a simple linked list: struct element { struct element *next; int start; }; struct list { struct element *head;
1
by: theeverdead | last post by:
Ok I have a file in it is a record of a persons first and last name. Format is like: Trevor Johnson Kevin Smith Allan Harris I need to read that file into program and then turn it into a linked...
1
by: jyothiram | last post by:
write short notes on linked list resentation using arrays,pointers and cursors?
4
by: phe2003 | last post by:
Hi All, I've been testing some extremely simple code about a linked list. What I keep doing is actually writing some parts of a linked list and testing them. Below are my codes:...
0
by: babe20042004 | last post by:
My function round1(boolean qualify, int goals) is supposed to go through a list of nodes comparing every 2 side by side nodes for the highest goals and then has the boolean variable qualify changed...
2
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 7 Feb 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:30 (7.30PM). In this month's session, the creator of the excellent VBE...
0
by: MeoLessi9 | last post by:
I have VirtualBox installed on Windows 11 and now I would like to install Kali on a virtual machine. However, on the official website, I see two options: "Installer images" and "Virtual machines"....
0
by: Aftab Ahmad | last post by:
Hello Experts! I have written a code in MS Access for a cmd called "WhatsApp Message" to open WhatsApp using that very code but the problem is that it gives a popup message everytime I clicked on...
0
by: Aftab Ahmad | last post by:
So, I have written a code for a cmd called "Send WhatsApp Message" to open and send WhatsApp messaage. The code is given below. Dim IE As Object Set IE =...
0
by: ryjfgjl | last post by:
ExcelToDatabase: batch import excel into database automatically...
0
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: marcoviolo | last post by:
Dear all, I would like to implement on my worksheet an vlookup dynamic , that consider a change of pivot excel via win32com, from an external excel (without open it) and save the new file into a...
1
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: jfyes | last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.