[lnkForumImage]
TotalShareware - Download Free Software

Confronta i prezzi di migliaia di prodotti.
Asp Forum
 Home | Login | Register | Search 


 

Forums >

comp.lang.c

Linked-list problem - compiles but segfaults

Andy Elvey

7/5/2011 12:53:00 AM

Hi all -
I've been poking around with C for a while, but I have big problems
with linked lists. This seems to be mainly around the "declaration,
initialisation, iterating" area. Anyway, I have the code below, which
compiles but gives a segfault (and I have no idea why), so hoping that
someone may be able to help.
( I have three puts statements at the end because I'm unsure how to
iterate along a list. )
[code]
/* This code is released to the public domain */
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>

/* Linked list implementation */
typedef struct _List List;

struct _List {
void *data;
List *next;
};

static List *list_malloc (void)
{
return (List *) malloc (sizeof (List));
}

List *list_prepend (List *list, void *data)
{
List *ret = list_malloc ();
ret->data = data;
ret->next = list;
return ret;
}

List *list_append (List *list, void *data)
{
List *ret;
if (list == NULL) return list_prepend (list, data);
ret = list;
for (; list->next; list = list->next);
list->next = list_malloc ();
list->next->data = data;
list->next->next = NULL;
return ret;
}

void list_free (List *list)
{
List *next;
while (list)
{
next = list->next;
free (list);
list = next;
}
}

/* End linked list implementation */

int main()
{
/* Declare a list */
List *mylist = NULL;
list_append(mylist, "foo");
puts(mylist->data);
list_append(mylist, (int*) 42);
puts(mylist->data);
list_append(mylist, "baz");
puts(mylist->data);
return 0;
}
[/code]
Very many thanks if you can get this code working...... :)
- Andy
18 Answers

Morris Keesan

7/5/2011 2:18:00 AM

0

On Mon, 04 Jul 2011 20:53:29 -0400, Andy Elvey
<neptune.poseidon789@gmail.com> wrote:
....
> ( I have three puts statements at the end because I'm unsure how to
> iterate along a list. )
> [code]
> /* This code is released to the public domain */
> #include <ctype.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/stat.h>
>
> /* Linked list implementation */
> typedef struct _List List;

Don't use leading underscores in your type names. These identifiers
are reserved. If you're copying the style of names that you've seen
in standard headers, those names begin with underscores specifically
because they're reserved for the implementation, and the implementation
of the standard header would avoid conflicting with your names, if you
weren't going out of your way like this to remove that protection.

> struct _List {
> void *data;
> List *next;
> };
>
> static List *list_malloc (void)
> {
> return (List *) malloc (sizeof (List));
This cast is unnecessary. Because malloc returns a (void *), it is
automatically converted to the right kind of pointer by the return
statement, without the need of the cast operator. Also, either here
....

> }
>
> List *list_prepend (List *list, void *data)
> {
> List *ret = list_malloc ();
.... or here, you need to check to make sure the malloc() hasn't returned
NULL. This is almost certainly not the source of your current problem,
but will cause you trouble in larger programs in the future. You must
always check to make sure malloc succeeded, before you try using a
pointer which was returned by malloc.

> ret->data = data;
> ret->next = list;
> return ret;
> }
>
> List *list_append (List *list, void *data)
> {
> List *ret;
> if (list == NULL) return list_prepend (list, data);
> ret = list;
> for (; list->next; list = list->next);
This for loop is okay, but odd looking. In my opinion, it would be
more readable as
while (list->next != NULL) list = list->next;

> list->next = list_malloc ();
> list->next->data = data;
> list->next->next = NULL;
> return ret;
> }
>
> void list_free (List *list)
> {
> List *next;
> while (list)
> {
> next = list->next;
> free (list);
> list = next;
> }
> }
>
> /* End linked list implementation */

Except for the small issues I've mentioned, your linked list
implementation looks okay.

>
> int main()
> {
> /* Declare a list */
> List *mylist = NULL;
> list_append(mylist, "foo");

Okay. Right here, mylist is still null, because you haven't modified it.
list_append returns the newly-allocated list, but because you don't use
the return value of list_append, the allocation is lost. So

> puts(mylist->data);

is the same as "puts(NULL->data);". I hope you can now see where your
segfault is coming from. If you didn't have the "mylist->data" expression
here causing a crash, then the next call to list_append() would create a
new list, instead of appending a new node to an existing list, because
you're passing a null pointer to list_append for the second time.

Here is another problem:
> list_append(mylist, (int*) 42);
You try to convert the number 42 to an (int *), and in list_append it will
get converted to a (void *). Whether this is meaningful is implementation-
defined. Most likely, you'll get a pointer whose bit pattern matches the
number 42, i.e. something that appears to be pointing to the whatever is
at the address "42", whatever that address means. Depending on your
implementation, attempting to convert the number 42 to an (int *) may
be causing the segfault, since if your platform requires ints to be on
4-byte boundaries, 42 is not a valid (int *), even if it were within
the range of user-addressable memory.
If you want to store the number 42 in the linked list as you've
implemented it, the way to do this would be something like
int fortytwo = 42; list_append(mylist, &fortytwo);
but then you would have to have some way of knowing that the data in that
particular list node is an integer, and not a string.

Note that since nothing in your code modifies mylist, all three of these
puts calls should display the same data. In this case, that's a good
thing,
because if you were displaying each subsequent added data, this next
puts call would be equivalent to puts((char *)(void *)(int *)42), which is
meaningless, and which would probably cause a crash.
> puts(mylist->data);
> list_append(mylist, "baz");
> puts(mylist->data);
> return 0;
> }
> [/code]
> Very many thanks if you can get this code working...... :)
> - Andy

As far as iterating along the list: if you knew that the data in all
members
of the list, you could do it this way:

for (List *p = mylist; p != NULL; p = p->next) puts(p->data);


--
Morris Keesan -- mkeesan@post.harvard.edu

Andy Elvey

7/5/2011 2:32:00 AM

0

On Jul 5, 2:18 pm, "Morris Keesan" <mkee...@post.harvard.edu> wrote:
> On Mon, 04 Jul 2011 20:53:29 -0400, Andy Elvey  <neptune.poseidon...@gmail.com> wrote:
> (snip)
> As far as iterating along the list: if you knew that the data in all  
> members
> of the list, you could do it this way:
>
> for (List *p = mylist; p != NULL; p = p->next) puts(p->data);
>
> --
> Morris Keesan -- mkee...@post.harvard.edu
Hi Morris -
Thanks very much for your reply! I'll have a good look at what
you've suggested, and will do what you've said. Many thanks - bye for
now -
- Andy

gordonb.bxelr

7/5/2011 2:40:00 AM

0

> I've been poking around with C for a while, but I have big problems
> with linked lists. This seems to be mainly around the "declaration,
> initialisation, iterating" area. Anyway, I have the code below, which
> compiles but gives a segfault (and I have no idea why), so hoping that
> someone may be able to help.

Your functions list_prepend and list_append return the new list
head pointer. list_prepend *always* changes the list head pointer.
list_append changes the list head pointer if it is originally NULL.
You need to actually *USE* those return values in main(). As coded,
mylist in main() is always NULL, and puts(mylist->data) is begging
for a segfault.


> ( I have three puts statements at the end because I'm unsure how to
> iterate along a list. )

This kind of iteration is typically done by (list is the list head pointer):
List *foo;

for (foo = list; foo != NULL; foo = foo->next)
{
... do something with foo->data ...;
}

> [code]
> /* This code is released to the public domain */
> #include <ctype.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/stat.h>
>
> /* Linked list implementation */
> typedef struct _List List;
>
> struct _List {
> void *data;
> List *next;
> };
>
> static List *list_malloc (void)
> {
> return (List *) malloc (sizeof (List));
> }
>
> List *list_prepend (List *list, void *data)
> {
> List *ret = list_malloc ();
> ret->data = data;
> ret->next = list;
> return ret;
> }
>
> List *list_append (List *list, void *data)
> {
> List *ret;
> if (list == NULL) return list_prepend (list, data);
> ret = list;
> for (; list->next; list = list->next);
> list->next = list_malloc ();
> list->next->data = data;
> list->next->next = NULL;
> return ret;
> }
>
> void list_free (List *list)
> {
> List *next;
> while (list)
> {
> next = list->next;
> free (list);
> list = next;
> }
> }
>
> /* End linked list implementation */
>
> int main()
> {
> /* Declare a list */
> List *mylist = NULL;
> list_append(mylist, "foo");
> puts(mylist->data);

Casting 42 to an (int *), then using it in place of a char * in the
puts() below it, is also begging for a segmentation fault. You may use
various data types for the data pointer, but you need to figure out
some way of identifying what type it is before trying to print it.
puts((int *) 42) is trouble.

> list_append(mylist, (int*) 42);
> puts(mylist->data);
> list_append(mylist, "baz");
> puts(mylist->data);
> return 0;
> }
> [/code]
> Very many thanks if you can get this code working...... :)

Please send me the email address of your instructor, so I can
send the corrected code directly to him.

Andy Elvey

7/5/2011 3:00:00 AM

0

On Jul 5, 2:39 pm, gordonb.bx...@burditt.org (Gordon Burditt) wrote:
> >  I've been poking around with C for a while, but I have big problems
> > with linked lists. This seems to be mainly around the "declaration,
> > initialisation, iterating" area.  Anyway, I have the code below, which
> > compiles but gives a segfault (and I have no idea why), so hoping that
> > someone may be able to help.
>
> Your functions list_prepend and list_append return the new list
> head pointer.  list_prepend *always* changes the list head pointer.
> list_append changes the list head pointer if it is originally NULL.
> You need to actually *USE* those return values in main().  As coded,
> mylist in main() is always NULL, and puts(mylist->data) is begging
> for a segfault.
>
> > ( I have three puts statements at the end because I'm unsure how to
> > iterate along a list. )
>
> This kind of iteration is typically done by (list is the list head pointer):
>         List *foo;
>
>         for (foo = list; foo != NULL; foo = foo->next)
>         {
>                 ... do something with foo->data ...;
>         }
>
> > [code]
> > /* This code is released to the public domain  */
> > #include <ctype.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <sys/stat.h>
>
> > /* Linked list implementation */
> > typedef struct _List List;
>
> > struct _List {
> >        void *data;
> >        List *next;
> > };
>
> > static List *list_malloc (void)
> > {
> >   return (List *) malloc (sizeof (List));
> > }
>
> > List *list_prepend (List *list, void *data)
> > {
> >        List *ret = list_malloc ();
> >        ret->data = data;
> >        ret->next = list;
> >        return ret;
> > }
>
> > List *list_append (List *list, void *data)
> > {
> >        List *ret;
> >        if (list == NULL) return list_prepend (list, data);
> >        ret = list;
> >        for (; list->next; list = list->next);
> >        list->next = list_malloc ();
> >        list->next->data = data;
> >        list->next->next = NULL;
> >        return ret;
> > }
>
> > void list_free (List *list)
> > {
> >        List *next;
> >        while (list)
> >        {
> >                next = list->next;
> >                free (list);
> >                list = next;
> >        }
> > }
>
> > /* End linked list implementation */
>
> > int main()
> > {
> > /* Declare a list */
> > List *mylist = NULL;
> > list_append(mylist, "foo");
> > puts(mylist->data);
>
> Casting 42 to an (int *), then using it in place of a char * in the
> puts() below it, is also begging for a segmentation fault.  You may use
> various data types for the data pointer, but you need to figure out
> some way of identifying what type it is before trying to print it.
> puts((int *) 42) is trouble.
>
> > list_append(mylist, (int*) 42);
> > puts(mylist->data);
> > list_append(mylist, "baz");
> > puts(mylist->data);
> > return 0;
> > }
> > [/code]
> > Very many thanks if you can get this code working...... :)
>
> Please send me the email address of your instructor, so I can
> send the corrected code directly to him.

Hi Gordon - thanks for your help, much appreciated!
Um.... I don't have an instructor, I'm doing this on my own. I got the
code from here -
http://adamhooper.com/code?path=mcgill-se%2FCOMP206%2Fass2%2F...
Anyway.... I'd really appreciate it if you were able to either email
or post the corrected code. I've been stuck on linked-lists for ages,
and there are so many (confusing) implementations that make them more
confusing than they should be.... :)
Thanks again -
- Andy

pete

7/5/2011 4:13:00 AM

0

Andy Elvey wrote:
>
> Hi all -
> I've been poking around with C for a while, but I have big problems
> with linked lists. This seems to be mainly around the "declaration,
> initialisation, iterating" area. Anyway, I have the code below, which
> compiles but gives a segfault (and I have no idea why), so hoping that
> someone may be able to help.
> ( I have three puts statements at the end because I'm unsure how to
> iterate along a list. )
> [code]
> /* This code is released to the public domain */
> #include <ctype.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/stat.h>
>
> /* Linked list implementation */
> typedef struct _List List;
>
> struct _List {
> void *data;
> List *next;
> };
>
> static List *list_malloc (void)
> {
> return (List *) malloc (sizeof (List));
> }
>
> List *list_prepend (List *list, void *data)
> {
> List *ret = list_malloc ();
> ret->data = data;
> ret->next = list;
> return ret;
> }
>
> List *list_append (List *list, void *data)
> {
> List *ret;
> if (list == NULL) return list_prepend (list, data);
> ret = list;
> for (; list->next; list = list->next);
> list->next = list_malloc ();
> list->next->data = data;
> list->next->next = NULL;
> return ret;
> }
>
> void list_free (List *list)
> {
> List *next;
> while (list)
> {
> next = list->next;
> free (list);
> list = next;
> }
> }
>
> /* End linked list implementation */
>
> int main()
> {
> /* Declare a list */
> List *mylist = NULL;
> list_append(mylist, "foo");
> puts(mylist->data);
> list_append(mylist, (int*) 42);
> puts(mylist->data);
> list_append(mylist, "baz");
> puts(mylist->data);
> return 0;
> }
> [/code]
> Very many thanks if you can get this code working...... :)
> - Andy

/* BEGIN new.c */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* Linked list implementation */
typedef struct LIST {
struct LIST *next;
void *data;
} List;

List *
list_append(List **head,
List *tail,
const void *data,
size_t size)
{
List *node;

node = malloc(sizeof *node);
if (node != NULL) {
node -> next = NULL;
node -> data = malloc(size);
if (node -> data != NULL) {
memcpy(node -> data, data, size);
if (*head != NULL) {
tail -> next = node;
} else {
*head = node;
}
} else {
free(node);
node = NULL;
}
}
return node;
}

void
list_free(List *node, void (*free_data)(void *))
{
List *next_node;

while (node != NULL) {
next_node = node -> next;
free_data(node -> data);
free(node);
node = next_node;
}
}

int
list_fputs(List *node, FILE *stream)
{
int rc = 0;

while (node != NULL
&& (rc = fputs(node -> data, stream)) != EOF
&& (rc = putc( '\n', stream)) != EOF)
{
node = node -> next;
}
return rc;
}

/* End linked list implementation */

int
main(void)
{
/* Declare a list */
List *mylist = NULL;
List *tail = NULL;
char *w[] = {"foo", "bar", "bazooka"};
char **p = w;

do {
tail = list_append(&mylist, tail, *p, 1 + strlen(*p));
} while(tail != NULL && ++p != w + sizeof w / sizeof *w);
puts("/* BEGIN new.c output */\n");
list_fputs(mylist, stdout);
list_free(mylist, free);
puts("\n/* END new.c output */");
return 0;
}

/* END new.c */

--
pete

Andy Elvey

7/5/2011 4:20:00 AM

0

On Jul 5, 4:13 pm, pete <pfil...@mindspring.com> wrote:
> Andy Elvey wrote:
>
> > Hi all -
> >   I've been poking around with C for a while, but I have big problems
> > with linked lists. This seems to be mainly around the "declaration,
> > initialisation, iterating" area.  Anyway, I have the code below, which
> > compiles but gives a segfault (and I have no idea why), so hoping that
> > someone may be able to help.
> > ( I have three puts statements at the end because I'm unsure how to
> > iterate along a list. )
> > [code]
> > /* This code is released to the public domain  */
> > #include <ctype.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <sys/stat.h>
>
> > /* Linked list implementation */
> > typedef struct _List List;
>
> > struct _List {
> >         void *data;
> >         List *next;
> > };
>
> > static List *list_malloc (void)
> > {
> >    return (List *) malloc (sizeof (List));
> > }
>
> > List *list_prepend (List *list, void *data)
> > {
> >         List *ret = list_malloc ();
> >         ret->data = data;
> >         ret->next = list;
> >         return ret;
> > }
>
> > List *list_append (List *list, void *data)
> > {
> >         List *ret;
> >         if (list == NULL) return list_prepend (list, data);
> >         ret = list;
> >         for (; list->next; list = list->next);
> >         list->next = list_malloc ();
> >         list->next->data = data;
> >         list->next->next = NULL;
> >         return ret;
> > }
>
> > void list_free (List *list)
> > {
> >         List *next;
> >         while (list)
> >         {
> >                 next = list->next;
> >                 free (list);
> >                 list = next;
> >         }
> > }
>
> > /* End linked list implementation */
>
> > int main()
> > {
> > /* Declare a list */
> > List *mylist = NULL;
> > list_append(mylist, "foo");
> > puts(mylist->data);
> > list_append(mylist, (int*) 42);
> > puts(mylist->data);
> > list_append(mylist, "baz");
> > puts(mylist->data);
> > return 0;
> > }
> > [/code]
> > Very many thanks if you can get this code working...... :)
> > - Andy
>
> /* BEGIN new.c */
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> /* Linked list implementation */
> typedef struct LIST {
>     struct LIST *next;
>     void *data;
>
> } List;
>
> List *
> list_append(List **head,
>             List *tail,
>             const void *data,
>             size_t size)
> {
>     List *node;
>
>     node = malloc(sizeof *node);
>     if (node != NULL) {
>         node -> next = NULL;
>         node -> data = malloc(size);
>         if (node -> data != NULL) {
>             memcpy(node -> data, data, size);
>             if (*head != NULL) {
>                 tail -> next = node;  
>             } else {
>                 *head = node;
>             }
>         } else {
>             free(node);
>             node = NULL;
>         }
>     }
>     return node;
>
> }
>
> void
> list_free(List *node, void (*free_data)(void *))
> {
>     List *next_node;
>
>     while (node != NULL) {
>         next_node = node -> next;
>         free_data(node -> data);
>         free(node);
>         node = next_node;
>     }
>
> }
>
> int
> list_fputs(List *node, FILE *stream)
> {
>     int rc = 0;
>
>     while (node != NULL
>         && (rc = fputs(node -> data, stream)) != EOF
>         && (rc =  putc(        '\n', stream)) != EOF)
>     {
>         node = node -> next;
>     }
>     return rc;
>
> }
>
> /* End linked list implementation */
>
> int
> main(void)
> {
>     /* Declare a list */
>     List *mylist = NULL;
>     List *tail = NULL;
>     char *w[] = {"foo", "bar", "bazooka"};
>     char **p = w;
>
>     do {
>         tail = list_append(&mylist, tail, *p, 1 + strlen(*p));
>     } while(tail != NULL && ++p != w + sizeof w / sizeof *w);
>     puts("/* BEGIN new.c output */\n");
>     list_fputs(mylist, stdout);
>     list_free(mylist, free);
>     puts("\n/* END new.c output */");
>     return 0;
>
> }
>
> /* END new.c */
>
> --
> pete

Many thanks for that, Pete! That code is the best that I've seen for
a linked list!
It's good to see some code that's easy to understand.
Thanks again - bye for now -
- Andy

pete

7/5/2011 5:05:00 AM

0

Andy Elvey wrote:
>
> On Jul 5, 4:13 pm, pete <pfil...@mindspring.com> wrote:
> > Andy Elvey wrote:
> >
> > > Hi all -
> > > I've been poking around with C for a while, but I have big problems
> > > with linked lists. This seems to be mainly around the "declaration,
> > > initialisation, iterating" area. Anyway, I have the code below, which
> > > compiles but gives a segfault (and I have no idea why), so hoping that
> > > someone may be able to help.
> > > ( I have three puts statements at the end because I'm unsure how to
> > > iterate along a list. )
> > > [code]
> > > /* This code is released to the public domain */
> > > #include <ctype.h>
> > > #include <stdio.h>
> > > #include <stdlib.h>
> > > #include <string.h>
> > > #include <sys/stat.h>
> >
> > > /* Linked list implementation */
> > > typedef struct _List List;
> >
> > > struct _List {
> > > void *data;
> > > List *next;
> > > };
> >
> > > static List *list_malloc (void)
> > > {
> > > return (List *) malloc (sizeof (List));
> > > }
> >
> > > List *list_prepend (List *list, void *data)
> > > {
> > > List *ret = list_malloc ();
> > > ret->data = data;
> > > ret->next = list;
> > > return ret;
> > > }
> >
> > > List *list_append (List *list, void *data)
> > > {
> > > List *ret;
> > > if (list == NULL) return list_prepend (list, data);
> > > ret = list;
> > > for (; list->next; list = list->next);
> > > list->next = list_malloc ();
> > > list->next->data = data;
> > > list->next->next = NULL;
> > > return ret;
> > > }
> >
> > > void list_free (List *list)
> > > {
> > > List *next;
> > > while (list)
> > > {
> > > next = list->next;
> > > free (list);
> > > list = next;
> > > }
> > > }
> >
> > > /* End linked list implementation */
> >
> > > int main()
> > > {
> > > /* Declare a list */
> > > List *mylist = NULL;
> > > list_append(mylist, "foo");
> > > puts(mylist->data);
> > > list_append(mylist, (int*) 42);
> > > puts(mylist->data);
> > > list_append(mylist, "baz");
> > > puts(mylist->data);
> > > return 0;
> > > }
> > > [/code]
> > > Very many thanks if you can get this code working...... :)
> > > - Andy
> >
> > /* BEGIN new.c */
> >
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> >
> > /* Linked list implementation */
> > typedef struct LIST {
> > struct LIST *next;
> > void *data;
> >
> > } List;
> >
> > List *
> > list_append(List **head,
> > List *tail,
> > const void *data,
> > size_t size)
> > {
> > List *node;
> >
> > node = malloc(sizeof *node);
> > if (node != NULL) {
> > node -> next = NULL;
> > node -> data = malloc(size);
> > if (node -> data != NULL) {
> > memcpy(node -> data, data, size);
> > if (*head != NULL) {
> > tail -> next = node;
> > } else {
> > *head = node;
> > }
> > } else {
> > free(node);
> > node = NULL;
> > }
> > }
> > return node;
> >
> > }
> >
> > void
> > list_free(List *node, void (*free_data)(void *))
> > {
> > List *next_node;
> >
> > while (node != NULL) {
> > next_node = node -> next;
> > free_data(node -> data);
> > free(node);
> > node = next_node;
> > }
> >
> > }
> >
> > int
> > list_fputs(List *node, FILE *stream)
> > {
> > int rc = 0;
> >
> > while (node != NULL
> > && (rc = fputs(node -> data, stream)) != EOF
> > && (rc = putc( '\n', stream)) != EOF)
> > {
> > node = node -> next;
> > }
> > return rc;
> >
> > }
> >
> > /* End linked list implementation */
> >
> > int
> > main(void)
> > {
> > /* Declare a list */
> > List *mylist = NULL;
> > List *tail = NULL;
> > char *w[] = {"foo", "bar", "bazooka"};
> > char **p = w;
> >
> > do {
> > tail = list_append(&mylist, tail, *p, 1 + strlen(*p));
> > } while(tail != NULL && ++p != w + sizeof w / sizeof *w);
> > puts("/* BEGIN new.c output */\n");
> > list_fputs(mylist, stdout);
> > list_free(mylist, free);
> > puts("\n/* END new.c output */");
> > return 0;
> >
> > }
> >
> > /* END new.c */
> >
> > --
> > pete
>
> Many thanks for that, Pete! That code is the best that I've seen for
> a linked list!
> It's good to see some code that's easy to understand.
> Thanks again - bye for now -

You're welcome.
You can also use the same list_append() and list_free()
functions for other data types as well.

/* BEGIN new.c */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* Linked list implementation */
typedef struct LIST {
struct LIST *next;
void *data;
} List;

List *
list_append(List **head,
List *tail,
const void *data,
size_t size)
{
List *node;

node = malloc(sizeof *node);
if (node != NULL) {
node -> next = NULL;
node -> data = malloc(size);
if (node -> data != NULL) {
memcpy(node -> data, data, size);
if (*head != NULL) {
tail -> next = node;
} else {
*head = node;
}
} else {
free(node);
node = NULL;
}
}
return node;
}

void
list_free(List *node, void (*free_data)(void *))
{
List *next_node;

while (node != NULL) {
next_node = node -> next;
free_data(node -> data);
free(node);
node = next_node;
}
}

int
list_int_fprintf(List *node, FILE *stream)
{
int rc = 0;

while (node != NULL
&& (rc = fprintf(stream, "%d\n", *(int *)node->data)) > 0)
{
node = node -> next;
}
return rc;
}

/* End linked list implementation */

int
main(void)
{
/* Declare a list */
List *mylist = NULL;
List *tail = NULL;
int i[] = {1, 2, 42};
int *p = i;

do {
tail = list_append(&mylist, tail, p, sizeof *p);
} while(tail != NULL && ++p != i + sizeof i / sizeof *i);
puts("/* BEGIN new.c output */\n");
list_int_fprintf(mylist, stdout);
list_free(mylist, free);
puts("\n/* END new.c output */");
return 0;
}

/* END new.c */

--
pete

gordonb.tu82f

7/5/2011 5:12:00 AM

0

> http://adamhooper.com/code?path=mcgill-se%2FCOMP206%2Fass2%2F...
> Anyway.... I'd really appreciate it if you were able to either email
> or post the corrected code. I've been stuck on linked-lists for ages,
> and there are so many (confusing) implementations that make them more
> confusing than they should be.... :)

If you cannot figure out what the correct code should be based on
my post and others pointing out exactly where the error is, or at
least figure out some changes to get a little farther, you're not
going to get it, and you need an instructor, or a good textbook.

Do you know what the return value of a function is?
Do you know which lines of code are in main()?
Do you understand why "mylist in main() is always NULL"?
What function do you call in main() to add an element to the list?
Why should the variable mylist change from NULL after you add
an element to the list?
Do you understand why mylist being NULL is a problem when
you reach the puts(mylist->data); statement?
Do you understand which lines of code in main() call list_append()?
Do you understand what *use the return value* of list_append() means?
What variable might you use to hold the return value of list_append()?
(What variable in main() has the wrong value at times?)

You should at least be able to make some changes based on that
and try again. If it doesn't work, post the modified code to show
you're making some progress. If you can't figure out how to modify
the code to at least try to fix it, then you probably need a lot
more studying of the basics. Maybe you aren't ready for linked lists
yet.

christian.bau

7/5/2011 3:43:00 PM

0

On Jul 5, 1:53 am, Andy Elvey <neptune.poseidon...@gmail.com> wrote:

> int main()
> {
> /* Declare a list */
> List *mylist = NULL;
> list_append(mylist, "foo");
> puts(mylist->data);

Look at the documentation of your compiler and how to turn warnings
on. You should get a compiler warning on the last line and your
program will crash there; no need even to read the code for
list_append.

Keith Thompson

7/5/2011 4:05:00 PM

0

"christian.bau" <christian.bau@cbau.wanadoo.co.uk> writes:
> On Jul 5, 1:53 am, Andy Elvey <neptune.poseidon...@gmail.com> wrote:
>> int main()
>> {
>> /* Declare a list */
>> List *mylist = NULL;
>> list_append(mylist, "foo");
>> puts(mylist->data);
>
> Look at the documentation of your compiler and how to turn warnings
> on. You should get a compiler warning on the last line and your
> program will crash there; no need even to read the code for
> list_append.

You'd get a warning on the puts() call only if the compiler performs
enough dataflow analysis to infer that mylist->data will be a
null pointer at that point. Typically that's done only when you
enable optimization (because the same analysis that allows certain
optimizations to be performed safely can also reveal such errors).

And there's no guarantee that it would crash. There have been plenty
of systems on which dereferencing a null pointer doesn't cause any
obvious problems; for example, it might simply read whatever is at
address 0, which might be a zero byte. (Of course on many systems
it will crash.)

--
Keith Thompson (The_Other_Keith) kst-u@mib.org <http://www.ghoti.ne...
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"