SLIST_FOREACH_PREVPTR() Macro Fix

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

SLIST_FOREACH_PREVPTR() Macro Fix

Ray Lai
Hi,

This diff fixes the SLIST_FOREACH_PREVPTR() macro in queue(3).

The included code demonstrates the problem.  Currently the code
outputs:

        one
        two
        three
        one -> one
        two -> two
        three -> three

With the patch:

        one
        two
        three
        NULL -> one
        one -> two
        two -> three

Comments?

-Ray-

#define QUEUE_MACRO_DEBUG

#include <sys/queue.h>

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

int
main()
{
        SLIST_HEAD(listhead, entry) head;
        struct entry {
                SLIST_ENTRY(entry) entries; /* Simple list. */
                char *name;
        } *n1, *np, **npp;

        SLIST_INIT(&head); /* Initialize simple list. */

        n1 = malloc(sizeof(struct entry)); /* Insert at the head. */
        n1->name = "three";
        SLIST_INSERT_HEAD(&head, n1, entries);

        n1 = malloc(sizeof(struct entry)); /* Insert at the head. */
        n1->name = "two";
        SLIST_INSERT_HEAD(&head, n1, entries);

        n1 = malloc(sizeof(struct entry)); /* Insert at the head. */
        n1->name = "one";
        SLIST_INSERT_HEAD(&head, n1, entries);

        npp = &n1;
                                                /* Forward traversal. */
        SLIST_FOREACH(np, &head, entries)
                printf("%s\n", np->name);

                                                /* Traversal with previous element. */
        SLIST_FOREACH_PREVPTR(np, npp, &head, entries) {
                const char *name, *prev;

                name = np ? np->name : "NULL";
                prev = *npp ? (*npp)->name : "NULL";
                printf("%s -> %s\n", prev, name);
        }

        return (0);
}

Index: queue.h
===================================================================
RCS file: /cvs/src/sys/sys/queue.h,v
retrieving revision 1.31
diff -u -r1.31 queue.h
--- queue.h 25 Nov 2005 08:06:25 -0000 1.31
+++ queue.h 10 Jan 2006 05:40:35 -0000
@@ -118,9 +118,9 @@
     (var) = SLIST_NEXT(var, field))
 
 #define SLIST_FOREACH_PREVPTR(var, varp, head, field) \
- for ((varp) = &SLIST_FIRST((head)); \
-    ((var) = *(varp)) != SLIST_END(head); \
-    (varp) = &SLIST_NEXT((var), field))
+ for (*(varp) = NULL, (var) = SLIST_FIRST(head); \
+    (var) != SLIST_END(head); \
+    *(varp) = (var), (var) = SLIST_NEXT((var), field))
 
 /*
  * Singly-linked List functions.

Reply | Threaded
Open this post in threaded view
|

Re: SLIST_FOREACH_PREVPTR() Macro Fix

Otto Moerbeek
On Tue, 10 Jan 2006, Ray Lai wrote:

> Hi,
>
> This diff fixes the SLIST_FOREACH_PREVPTR() macro in queue(3).
>
> The included code demonstrates the problem.  Currently the code
> outputs:
>
> one
> two
> three
> one -> one
> two -> two
> three -> three
>
> With the patch:
>
> one
> two
> three
> NULL -> one
> one -> two
> two -> three
>
> Comments?

At first sight your diff looks correct; currently
SLIST_FOREACH_PREVPTR is (only) used in sys/kern/sysv_sem.c:semexit();
we'll have to double check that function to see if it expects the
current behaviour. From a quick view it looks like it can bomb with
your diff.

        -Otto

>
> -Ray-
>
> #define QUEUE_MACRO_DEBUG
>
> #include <sys/queue.h>
>
> #include <stdio.h>
> #include <stdlib.h>
>
> int
> main()
> {
> SLIST_HEAD(listhead, entry) head;
> struct entry {
> SLIST_ENTRY(entry) entries; /* Simple list. */
> char *name;
> } *n1, *np, **npp;
>
> SLIST_INIT(&head); /* Initialize simple list. */
>
> n1 = malloc(sizeof(struct entry)); /* Insert at the head. */
> n1->name = "three";
> SLIST_INSERT_HEAD(&head, n1, entries);
>
> n1 = malloc(sizeof(struct entry)); /* Insert at the head. */
> n1->name = "two";
> SLIST_INSERT_HEAD(&head, n1, entries);
>
> n1 = malloc(sizeof(struct entry)); /* Insert at the head. */
> n1->name = "one";
> SLIST_INSERT_HEAD(&head, n1, entries);
>
> npp = &n1;
> /* Forward traversal. */
> SLIST_FOREACH(np, &head, entries)
> printf("%s\n", np->name);
>
> /* Traversal with previous element. */
> SLIST_FOREACH_PREVPTR(np, npp, &head, entries) {
> const char *name, *prev;
>
> name = np ? np->name : "NULL";
> prev = *npp ? (*npp)->name : "NULL";
> printf("%s -> %s\n", prev, name);
> }
>
> return (0);
> }
>
> Index: queue.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/queue.h,v
> retrieving revision 1.31
> diff -u -r1.31 queue.h
> --- queue.h 25 Nov 2005 08:06:25 -0000 1.31
> +++ queue.h 10 Jan 2006 05:40:35 -0000
> @@ -118,9 +118,9 @@
>      (var) = SLIST_NEXT(var, field))
>  
>  #define SLIST_FOREACH_PREVPTR(var, varp, head, field) \
> - for ((varp) = &SLIST_FIRST((head)); \
> -    ((var) = *(varp)) != SLIST_END(head); \
> -    (varp) = &SLIST_NEXT((var), field))
> + for (*(varp) = NULL, (var) = SLIST_FIRST(head); \
> +    (var) != SLIST_END(head); \
> +    *(varp) = (var), (var) = SLIST_NEXT((var), field))
>  
>  /*
>   * Singly-linked List functions.