[Bug-gnulib] Re: obstack default alignment

[Paul Eggert]

Benno <address@hidden> writes:

> This originally went to the glibc list. It was suggested
> I copy here as more people might actually the obstack functions.

obstack seems to be somewhat of an orphan in glibc.  It's got lots of
bugs on 64-bit platforms, fixing them will require changing the API
and ABI, and nobody has had the time or inclination to do this.  Some
(but not all!) of its bugs have been fixed in the gnulib copy.  It's
not a good situation.

> By constrast obstack_alloc is defined to return memory aligned to 4 bytes.

That's an error in the glibc documentation.  I just filed a bug report
with a suggested patch
<http://sources.redhat.com/bugzilla/show_bug.cgi?id=315>.

> I would have thought it made sense to set the default alignment in the same
> way as malloc().

True, and that's what the code tries to do.  I should warn you, though
that there are some bugs in that area.  I've just checked in the
following change to the gnulib version to fix some more of them that I
noticed when looking into your bug report.

2004-08-09  Paul Eggert  <address@hidden>

        * obstack.h (obstack_empty_p):
        Don't assume that chunk->contents is suitably aligned.
        * obstack.c (_obstack_begin, _obstack_begin_1, _obstack_newchunk):
        Likewise. Problem reported by Benno in
        <http://sources.redhat.com/ml/libc-alpha/2004-08/msg00055.html>.

Index: lib/obstack.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/obstack.h,v
retrieving revision 1.29
diff -p -u -r1.29 obstack.h
--- lib/obstack.h       24 Jun 2004 06:04:10 -0000      1.29
+++ lib/obstack.h       10 Aug 2004 05:13:06 -0000
@@ -287,7 +287,10 @@ __extension__                                              
                \
 # define obstack_empty_p(OBSTACK)                                      \
   __extension__                                                                
\
   ({ struct obstack const *__o = (OBSTACK);                            \
-     (__o->chunk->prev == 0 && __o->next_free - __o->chunk->contents == 0); })
+     (__o->chunk->prev == 0                                            \
+      && __o->next_free == __PTR_ALIGN ((char *) __o->chunk,           \
+                                       __o->chunk->contents,           \
+                                       __o->alignment_mask)); })
 
 # define obstack_grow(OBSTACK,where,length)                            \
 __extension__                                                          \
@@ -411,7 +414,10 @@ __extension__                                              
                \
  (unsigned) ((h)->chunk_limit - (h)->next_free)
 
 # define obstack_empty_p(h) \
- ((h)->chunk->prev == 0 && (h)->next_free - (h)->chunk->contents == 0)
+ ((h)->chunk->prev == 0                                                        
\
+  && (h)->next_free == __PTR_ALIGN ((char *) (h)->chunk,               \
+                                   (h)->chunk->contents,               \
+                                   (h)->alignment_mask))
 
 /* Note that the call to _obstack_newchunk is enclosed in (..., 0)
    so that we can avoid having void expressions
Index: lib/obstack.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/obstack.c,v
retrieving revision 1.27
diff -p -u -r1.27 obstack.c
--- lib/obstack.c       21 May 2004 04:51:32 -0000      1.27
+++ lib/obstack.c       10 Aug 2004 05:13:06 -0000
@@ -173,7 +173,8 @@ _obstack_begin (struct obstack *h,
   chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
   if (!chunk)
     (*obstack_alloc_failed_handler) ();
-  h->next_free = h->object_base = chunk->contents;
+  h->next_free = h->object_base = __PTR_ALIGN ((char *) chunk, chunk->contents,
+                                              alignment - 1);
   h->chunk_limit = chunk->limit
     = (char *) chunk + h->chunk_size;
   chunk->prev = 0;
@@ -220,7 +221,8 @@ _obstack_begin_1 (struct obstack *h, int
   chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
   if (!chunk)
     (*obstack_alloc_failed_handler) ();
-  h->next_free = h->object_base = chunk->contents;
+  h->next_free = h->object_base = __PTR_ALIGN ((char *) chunk, chunk->contents,
+                                              alignment - 1);
   h->chunk_limit = chunk->limit
     = (char *) chunk + h->chunk_size;
   chunk->prev = 0;
@@ -287,7 +289,10 @@ _obstack_newchunk (struct obstack *h, in
   /* If the object just copied was the only data in OLD_CHUNK,
      free that chunk and remove it from the chain.
      But not if that chunk might contain an empty object.  */
-  if (h->object_base == old_chunk->contents && ! h->maybe_empty_object)
+  if (! h->maybe_empty_object
+      && (h->object_base
+         == __PTR_ALIGN ((char *) old_chunk, old_chunk->contents,
+                         h->alignment_mask)))
     {
       new_chunk->prev = old_chunk->prev;
       CALL_FREEFUN (h, old_chunk);