Strange problem with linked list code on windows OS

Discussion in 'C Programming' started by rkalyankumar77@gmail.com, Mar 24, 2008.

  1. Guest

    Hi,
    I see strange problem while I run the below code on Windows using gcc
    (mingw 3.4.5) & MS-visual studio compiler 2003 (cl compiler) -
    compiles ok. Both are failing for some referenced memory being passed
    to the routines add_head/add_tail and the macro add_list which
    couldn't be read. However this code works well with the gcc compiler
    under cygwin on windows and gcc compiler on any Linux system.

    Are there any issues with portability of code to windows? I am unable
    to guess where this is going wrong & why.

    Thanks for your suggestions/solution in advance.

    PS: I haven't coded for freeing memory that's malloc'd.

    list.h
    ----------
    #ifndef _K_LIST_H_
    #define _K_LIST_H_


    struct list_head
    {
    struct list_head *prev,*next;
    };


    #define LIST_INIT(name) {&(name),&(name)}
    #define LIST(name) \
    struct list_head name = LIST_INIT(name)

    #ifndef __GNUC__
    #define list_entry(PTR,TYPE,FIELD) ((TYPE*)((char*) PTR -
    offsetof(TYPE,FIELD)))
    #else /*GCC specific implementation*/
    #define list_entry(PTR,TYPE,FIELD) ({\
    const typeof( ((TYPE *) 0)->FIELD) *__mptr = (PTR);\
    (TYPE*) ((char*) __mptr - offsetof(TYPE,FIELD));})
    #endif

    void init_list_head(struct list_head *list);

    void
    init_list_head(struct list_head *list)
    {
    list->next = list;
    list->prev = list;
    }

    extern void add_tail(struct list_head *tail, struct list_head
    *node);
    extern void add_head(struct list_head *head, struct list_head
    *node);

    #define list_for_each(head,node) \
    for((node) = (head)->next;(node) != (head) && (node);(node) = (node)-
    >next)

    #endif


    list.c
    ------------
    #include "list.h"
    #define add_list(N1,N2,N3) { \
    do { \
    struct list_head *__new = (struct list_head*) (N1); \
    struct list_head *__prev = (struct list_head*) (N2); \
    struct list_head *__next = (struct list_head*) (N3); \
    __next->prev = (struct list_head*)__new;\
    __new->next =(struct list_head*) __next;\
    __new->prev = (struct list_head*)__prev;\
    __prev->next = (struct list_head*)__new;\
    } while(0); \
    }


    void
    add_head(struct list_head *head,struct list_head *node)
    {
    add_list(node,head,head->next);
    }

    void
    add_tail(struct list_head *head,struct list_head *node)
    {
    add_list(node,head->prev,head);
    }


    struct int_list
    {
    int data;
    struct list_head head;
    };

    int
    main(void)
    {
    struct list_head *val;
    struct int_list *ilist,*inode;
    int i;
    ilist = malloc(sizeof(struct list_head));
    init_list_head(&ilist->head);
    for(i = 0; i < 10; i++) {
    inode = malloc(sizeof(struct list_head));
    inode->data = i + 1;
    printf("Adding %d to list\n",inode->data);
    add_head(&ilist->head,&inode->head);
    }

    list_for_each(&ilist->head,val) {
    inode = list_entry(val,struct int_list,head);
    printf("%d ",inode->data);
    }

    printf("\n");
    system("pause");
    return 0;
    }
     
    , Mar 24, 2008
    #1
    1. Advertising

  2. Guest

    On 24 Mar, 09:49, ""
    <> wrote:
    > Hi,
    > I see strange problem ...


    > struct list_head
    > {
    >         struct list_head *prev,*next;
    >
    > };


    > struct int_list
    > {
    >         int data;
    >         struct list_head head;
    >
    > };


    > int
    > main(void)
    > {
    >         struct list_head *val;
    >         struct int_list *ilist,*inode;
    >         int i;
    >         ilist = malloc(sizeof(struct list_head));
    >         init_list_head(&ilist->head);
    >         for(i = 0; i < 10; i++) {
    >                 inode = malloc(sizeof(struct list_head));


    Your code looks very confused, and you might do well to try to tidy it
    up. However, there seems to be an error here - you're only allocating
    enough space for a list_head but inode is an int_list, which is
    bigger.

    You ought to try to distinguish what variables represent the list as a
    whole, and which represent nodes in the list. For instance, it seems
    to me that list_head and int_list are both to do with individual
    nodes, whereas their names suggest that they are to do with the whole
    list.

    Hope this helps.
    Paul.
     
    , Mar 24, 2008
    #2
    1. Advertising

  3. Willem Guest

    wrote:
    ) On 24 Mar, 09:49, ""
    )<> wrote:
    ...
    )> ilist = malloc(sizeof(struct list_head));
    )> init_list_head(&ilist->head);
    )> for(i = 0; i < 10; i++) {
    )> inode = malloc(sizeof(struct list_head));
    )
    ) Your code looks very confused, and you might do well to try to tidy it
    ) up. However, there seems to be an error here - you're only allocating
    ) enough space for a list_head but inode is an int_list, which is
    ) bigger.

    That's why the reccomended malloc idiom is:

    inode = malloc(sizeof *inode);

    thus avoiding any size mismatches.


    SaSW, Willem
    --
    Disclaimer: I am in no way responsible for any of the statements
    made in the above text. For all I know I might be
    drugged or something..
    No I'm not paranoid. You all think I'm paranoid, don't you !
    #EOT
     
    Willem, Mar 24, 2008
    #3
  4. Flash Gordon Guest

    wrote, On 24/03/08 09:49:
    > Hi,
    > I see strange problem while I run the below code on Windows using gcc
    > (mingw 3.4.5) & MS-visual studio compiler 2003 (cl compiler) -
    > compiles ok.


    In that case the code posted is *not* the same as what you are
    compiling. I therefore no was of knowing whether the errors I spot are
    transcription errors or errors in your code.

    > Both are failing for some referenced memory being passed
    > to the routines add_head/add_tail and the macro add_list which
    > couldn't be read. However this code works well with the gcc compiler
    > under cygwin on windows and gcc compiler on any Linux system.
    >
    > Are there any issues with portability of code to windows? I am unable
    > to guess where this is going wrong & why.


    Whether there are issues depends on what you want to do. Linked list
    code can be done completely in standard C which is portable to any
    hosted environment and many embedded systems (non-hosted implementations
    are not required to provide malloc).

    > Thanks for your suggestions/solution in advance.
    >
    > PS: I haven't coded for freeing memory that's malloc'd.
    >
    > list.h
    > ----------
    > #ifndef _K_LIST_H_
    > #define _K_LIST_H_


    Symbols starting with an underscore are reserved in many situations and,
    in general, it is not worth remembering when it is safe to use them. In
    this case, if someone include *any* standard header as well as yours
    then the above is "illegal", i.e. something you should not do.

    #ifndef K_LIST_H_
    'define K_LIST_H_

    > struct list_head
    > {
    > struct list_head *prev,*next;
    > };
    >
    >
    > #define LIST_INIT(name) {&(name),&(name)}
    > #define LIST(name) \
    > struct list_head name = LIST_INIT(name)
    >
    > #ifndef __GNUC__
    > #define list_entry(PTR,TYPE,FIELD) ((TYPE*)((char*) PTR -
    > offsetof(TYPE,FIELD)))
    > #else /*GCC specific implementation*/
    > #define list_entry(PTR,TYPE,FIELD) ({\
    > const typeof( ((TYPE *) 0)->FIELD) *__mptr = (PTR);\
    > (TYPE*) ((char*) __mptr - offsetof(TYPE,FIELD));})
    > #endif


    If the non-gcc specific version is correct, why provide a gcc specific
    version as well? After all, gcc will (modulo bugs) compile correct C
    code. If the non-gcc specific version does not work, then fix it.

    > void init_list_head(struct list_head *list);


    Having a declaration immediately followed by the definition is
    pointless. Having a definition of a function in a header is almost
    always the wrong thing to do, and in this case it definitely is wrong.

    > void
    > init_list_head(struct list_head *list)
    > {
    > list->next = list;
    > list->prev = list;
    > }
    >
    > extern void add_tail(struct list_head *tail, struct list_head
    > *node);
    > extern void add_head(struct list_head *head, struct list_head
    > *node);


    Using extern in the above is pointless but harmless. Some people like it
    some dislike it. Purely a matter of style.

    > #define list_for_each(head,node) \
    > for((node) = (head)->next;(node) != (head) && (node);(node) = (node)-> next)


    What if head is a null pointer? Also this for loop means you won't act
    on the head of the list which would be surprising behaviour for a lot of
    people.

    > #endif
    >
    >
    > list.c
    > ------------
    > #include "list.h"
    > #define add_list(N1,N2,N3) { \
    > do { \
    > struct list_head *__new = (struct list_head*) (N1); \
    > struct list_head *__prev = (struct list_head*) (N2); \
    > struct list_head *__next = (struct list_head*) (N3); \
    > __next->prev = (struct list_head*)__new;\
    > __new->next =(struct list_head*) __next;\
    > __new->prev = (struct list_head*)__prev;\
    > __prev->next = (struct list_head*)__new;\
    > } while(0); \
    > }


    Why the enclosing braces on the above? You seem to have tried to use the
    standard trick for using a block without understanding it. You would
    normally do
    #define add_list(N1,N2,N3) \
    do { \
    struct list_head *new__ = (N1); \
    struct list_head *prev__ = (N2); \
    struct list_head *next__ = (N3); \
    next__->prev = new__; \
    new__->next = next__; \
    new__->prev = prev__; \
    prev__->next = new__; \
    } while(0)

    Note I have also removed all of the casts since *none* of them were
    required and they just make the code more dangerous (prevents the
    compiler from complaining about miss-use of the macro) and removed the
    leading underscores.

    To see why I've removed the outer braces and last semicolon try
    expanding the use of it in your functions below by hand.

    > void
    > add_head(struct list_head *head,struct list_head *node)
    > {
    > add_list(node,head,head->next);
    > }
    >
    > void
    > add_tail(struct list_head *head,struct list_head *node)
    > {
    > add_list(node,head->prev,head);
    > }
    >
    >
    > struct int_list
    > {
    > int data;
    > struct list_head head;
    > };
    >
    > int
    > main(void)
    > {
    > struct list_head *val;
    > struct int_list *ilist,*inode;
    > int i;
    > ilist = malloc(sizeof(struct list_head));


    Less error prone (as someone else pointed out) is
    ilist = malloc(sizeof *ilist);

    Also, you need to include stdlib.h to provide a prototype for malloc.
    Both gcc and Visual Studio *do* complain about this code for this reason
    as it was presented. Not including stdlib.h is a *serious* error and
    *does* cause crashes on modern implementations.

    > init_list_head(&ilist->head);
    > for(i = 0; i < 10; i++) {
    > inode = malloc(sizeof(struct list_head));


    inode = malloc(sizeof *inode);
    Again, someone else pointed this out.

    > inode->data = i + 1;
    > printf("Adding %d to list\n",inode->data);


    You need to include stdio.h to provide a prototype for printf.

    > add_head(&ilist->head,&inode->head);
    > }
    >
    > list_for_each(&ilist->head,val) {
    > inode = list_entry(val,struct int_list,head);


    The above line does not compile on my gcc install. I'm guessing this is
    because you have not posted your real code and your real code includes
    stddef.h

    > printf("%d ",inode->data);
    > }
    >
    > printf("\n");
    > system("pause");
    > return 0;
    > }


    As you say it compiles but it does not I'm not going to bother trying to
    work out what is else is wrong. After all, the error you are looking for
    could be in any of the 93674 lines of code you have not posted. Please
    post your *actual* code that show the problem, copy and paste it, don't
    retype it and don't miss bits because you know they are correct.
    --
    Flash Gordon
     
    Flash Gordon, Mar 24, 2008
    #4
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Chris Ritchey
    Replies:
    7
    Views:
    484
    emerth
    Jul 10, 2003
  2. Chris Ritchey

    Generating a char* from a linked list of linked lists

    Chris Ritchey, Jul 9, 2003, in forum: C Programming
    Replies:
    7
    Views:
    473
    emerth
    Jul 10, 2003
  3. fool
    Replies:
    14
    Views:
    514
    Barry Schwarz
    Jul 3, 2006
  4. joshd
    Replies:
    12
    Views:
    675
    John Carson
    Oct 2, 2006
  5. jawdoc
    Replies:
    9
    Views:
    764
    Chris Thomasson
    Mar 10, 2008
Loading...

Share This Page