Strange problem with linked list code on windows OS

[rkalyankumar77]

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;
}

 

[gw7rib]

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.

 

[Willem]

(e-mail address removed) wrote:
) On 24 Mar, 09:49, "(e-mail address removed)"
...
)> 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

 

[Flash Gordon]

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

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.