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
Similar Threads
