Re: [Bug-gnulib] obstack uses casts as lvalues

[Paul Eggert]

"Joseph S. Myers" <address@hidden> writes:

> If they are used, they can't ever have worked, but maybe they need to be
> removed.

They're documented in the glibc manual, so it may be a hassle to
remove them.  For now let's keep them.

> I think that reinterpreting the bit pattern of p in this way as another
> type of pointer would break the type-based aliasing rules.

OK.  I installed the following more-conservative change.  It's more
elaborate than what you proposed, since it attacks the non-GCC side as
well, and it revamps the code to make the "unused and never-tested
macro" bug less likely in the future.

2003-10-21  Paul Eggert  <address@hidden>

        * obstack.h (obstack_1grow_fast): Properly parenthesize arg.
        (obstack_ptr_grow_fast, obstack_int_grow_fast):
        Don't use lvalue casts, as GCC plans to remove support for them
        in GCC 3.5.  Reported by Joseph S. Meyers.  This bug
        was also present in the non-GCC version, indicating that this
        code had always been buggy and had never been widely used.
        (obstack_1grow, obstack_ptr_grow, obstack_int_grow, obstack_blank):
        Use the fast variant of each macro, rather than copying the
        definiens of the fast variant; that way, we'll be more likely to
        catch future bugs in the fast variants.

Index: obstack.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/obstack.h,v
retrieving revision 1.23
diff -p -u -r1.23 obstack.h
--- obstack.h   9 Sep 2003 23:00:43 -0000       1.23
+++ obstack.h   22 Oct 2003 00:59:13 -0000
@@ -274,7 +274,7 @@ extern int obstack_exit_failure;
 # define obstack_freefun(h, newfreefun) \
   ((h) -> freefun = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
 
-#define obstack_1grow_fast(h,achar) (*((h)->next_free)++ = achar)
+#define obstack_1grow_fast(h,achar) (*((h)->next_free)++ = (achar))
 
 #define obstack_blank_fast(h,n) ((h)->next_free += (n))
 
@@ -342,7 +342,7 @@ __extension__                                               
                \
 ({ struct obstack *__o = (OBSTACK);                                    \
    if (__o->next_free + 1 > __o->chunk_limit)                          \
      _obstack_newchunk (__o, 1);                                       \
-   *(__o->next_free)++ = (datum);                                      \
+   obstack_1grow_fast (__o, datum);                                    \
    (void) 0; })
 
 /* These assume that the obstack alignment is good enough for pointers
@@ -354,22 +354,28 @@ __extension__                                             
                \
 ({ struct obstack *__o = (OBSTACK);                                    \
    if (__o->next_free + sizeof (void *) > __o->chunk_limit)            \
      _obstack_newchunk (__o, sizeof (void *));                         \
-   *((void **)__o->next_free)++ = (datum);                             \
-   (void) 0; })
+   obstack_ptr_grow_fast (__o, datum); })                              \
 
 # define obstack_int_grow(OBSTACK,datum)                               \
 __extension__                                                          \
 ({ struct obstack *__o = (OBSTACK);                                    \
    if (__o->next_free + sizeof (int) > __o->chunk_limit)               \
      _obstack_newchunk (__o, sizeof (int));                            \
-   *((int *)__o->next_free)++ = (datum);                               \
-   (void) 0; })
+   obstack_int_grow_fast (__o, datum); })
 
-# define obstack_ptr_grow_fast(h,aptr)                                 \
-  (*((void **) (h)->next_free)++ = (aptr))
+# define obstack_ptr_grow_fast(OBSTACK,aptr)                           \
+__extension__                                                          \
+({ struct obstack *__o1 = (OBSTACK);                                   \
+   *(const void **) __o1->next_free = (aptr);                          \
+   __o1->next_free += sizeof (const void *);                           \
+   (void) 0; })
 
-# define obstack_int_grow_fast(h,aint)                                 \
-  (*((int *) (h)->next_free)++ = (aint))
+# define obstack_int_grow_fast(OBSTACK,aint)                           \
+__extension__                                                          \
+({ struct obstack *__o1 = (OBSTACK);                                   \
+   *(int *) __o1->next_free = (aint);                                  \
+   __o1->next_free += sizeof (int);                                    \
+   (void) 0; })
 
 # define obstack_blank(OBSTACK,length)                                 \
 __extension__                                                          \
@@ -377,7 +383,7 @@ __extension__                                               
                \
    int __len = (length);                                               \
    if (__o->chunk_limit - __o->next_free < __len)                      \
      _obstack_newchunk (__o, __len);                                   \
-   __o->next_free += __len;                                            \
+   obstack_blank_fast (__o, __len);                                    \
    (void) 0; })
 
 # define obstack_alloc(OBSTACK,length)                                 \
@@ -464,29 +470,29 @@ __extension__                                             
                \
 # define obstack_1grow(h,datum)                                                
\
 ( (((h)->next_free + 1 > (h)->chunk_limit)                             \
    ? (_obstack_newchunk ((h), 1), 0) : 0),                             \
-  (*((h)->next_free)++ = (datum)))
+  obstack_1grow_fast (h, datum))
 
 # define obstack_ptr_grow(h,datum)                                     \
 ( (((h)->next_free + sizeof (char *) > (h)->chunk_limit)               \
    ? (_obstack_newchunk ((h), sizeof (char *)), 0) : 0),               \
-  (*((const char **) (((h)->next_free+=sizeof(char *))-sizeof(char *))) = 
(datum)))
+  obstack_ptr_grow_fast (h, datum))
 
 # define obstack_int_grow(h,datum)                                     \
 ( (((h)->next_free + sizeof (int) > (h)->chunk_limit)                  \
    ? (_obstack_newchunk ((h), sizeof (int)), 0) : 0),                  \
-  (*((int *) (((h)->next_free+=sizeof(int))-sizeof(int))) = (datum)))
+  obstack_int_grow_fast (h, datum))
 
 # define obstack_ptr_grow_fast(h,aptr)                                 \
-  (*((const char **) (h)->next_free)++ = (aptr))
+  (((const void **) ((h)->next_free += sizeof (void *)))[-1] = (aptr))
 
 # define obstack_int_grow_fast(h,aint)                                 \
-  (*((int *) (h)->next_free)++ = (aint))
+  (((int *) ((h)->next_free += sizeof (int)))[-1] = (aptr))
 
 # define obstack_blank(h,length)                                       \
 ( (h)->temp = (length),                                                        
\
   (((h)->chunk_limit - (h)->next_free < (h)->temp)                     \
    ? (_obstack_newchunk ((h), (h)->temp), 0) : 0),                     \
-  ((h)->next_free += (h)->temp))
+  obstack_blank_fast (h, (h)->temp))
 
 # define obstack_alloc(h,length)                                       \
  (obstack_blank ((h), (length)), obstack_finish ((h)))