linked lists

Natasl · 08-04-2006, 06:01 AM

i've been playing around with linked lists, and came up with this for exercise...
it works, but i dont know if its all done correctly, are there any memory leaks or any other kind of errors. maybe you could check it out and tell me if its ok?

Code:

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

int main()
{
        struct item
        {
                int foo;
                struct item *next;
        };

        typedef struct item item;

        /* fill the list */

        item *current, *bar;
        bar = NULL;
        int baz;

        for(baz = 1; baz <= 10; baz++)
        {
                current = (item *) malloc(sizeof(item));
                current->foo = baz;
                current->next = bar;
                bar = current;
        }

        /* print list */

        current = bar;
        while(current)
        {
                printf("%d\n", current->foo);
                current = current->next;
        }

        /* clean up */

        current = bar;
        item *temporary;

        while(current)
        {
                temporary = current->next;
                free(current);
                current = temporary;
                puts("Cleaned one.");
        }

        return 0;
}

vharishankar · 08-04-2006, 06:09 AM

The code looks fairly ok, but I want to add some points.

Writing this is a good idea only if you're just learning, but for all practical purposes you should use an existing library which provides these functionalities. Particularly in C++ you should check out STL. It has all the classes for linked lists, stacks, queues and other data structures like dictionaries etc. The advantage of using existing standard libraries is that the code is practically guaranteed to be error free and memory leaks would be very rare.

Second thing, use functions (or classes if you're using C++) to do things like adding nodes, removing nodes and freeing a list and don't put everything in main ().

dmail · 08-04-2006, 06:57 AM

Code:

        for(baz = 1; baz <= 10; baz++)
        {
                current = (item *) malloc(sizeof(item));
                current->foo = baz;
                current->next = bar;
                bar = current;
        }

It works Natasl, but personally I do not like it and first thought it would not work until I realised its a bottom up linked list. ie the first node that is entered is actually the last in the list.

I would use a linked list with a head node, this gives you the entry point into the list, let me know if you would like an example.

edit:
Maybe I should say why I don't like it.
In my opinion a list should have the property of inserting/deleting a node at a given position, I can't see how you would accomplish this with your list.

Natasl · 08-04-2006, 08:29 AM

i wasn't going for any usability or anything... it's just an exercise, my only goal was for it to be technically correct

llmmix · 08-04-2006, 11:26 AM

compare your code with below library, it is written in c.
http://www.hyperrealm.com/main.php?s=cfl

Quote:

CFL is a C library of useful functions that simplify systems software development on System V UNIX. The library includes routines for memory management, string parsing, filesystem traversal, subprocess execution, I/O, as well as implementations of common data structures such as linked lists, hash tables, stacks, and queues. The library also includes a high-level interface to Berkeley sockets, and an implementation of a scheduler which has functionality very similar to that of the cron daemon.

Natasl · 08-04-2006, 12:02 PM

well, it's C_linklist_destroy() function is quite similar to my cleanup part of the code, so i guess i did that right. ^^