Re: [PATCH 0/5] obstacks again

[Alan Modra <amodra at gmail dot com>]

On Fri, Oct 31, 2014 at 12:50:25PM -0700, Paul Eggert wrote:
> I have a minor complaint
> about obstack_blank_fast's revised documentation.  It says "You can use
> @code{obstack_blank_fast} with a negative size argument to make the current
> object smaller."  Technically, though, the argument is of type size_t so it
> cannot be negative.

Really, it's a fiction that the size argument of obstack_blank_fast
has a type.  (This is what happens when you start being pedantic in
documentation. :-)  A worse pedant comes along and says you aren't
completely correct.)  Please don't take this as a challenge to make the
manual completely correct!  I think we'd make it unreadable it we did.
At least, I wasn't clever enough to write something that was simple
yet correct..

>  So, how about if we change this wording to "If @var{S}
> is a positive size, you can give @code{obstack_blank_fast} a ``negated''
> (actually, large positive) size @code{-@var{S}} to shrink the current object
> by @var{S} bytes."  Also, it may be worth noting explicitly that this trick
> does not work for object_blank.

I've added: "Earlier
versions of obstacks allowed you to use @code{obstack_blank} to shrink
objects.  This will no longer work."

> Also, doesn't libc need a NEWS item for these changes?  At least, gnulib
> needs one; I installed the attached.

Yes, thanks!  I've added the bug number to the list fixed with 2.21
and stolen your wording for a NEWS item, minus the "(actually large
positive)" phrase.

> Don't we have similar problems with lots of other
> macros and/or functions: obstack_1grow_fast, obstack_blank_fast,
> obstack_int_grow, etc.?  They all seem to return types that don't match the
> documentation.  As long as we're fixing things, this might all be done in a
> separate patch.

There another gnulib issue too.  AC_FUNC_OBSTACK is no longer correct.
The system libc may support obstacks, but not the current version.  It
seems to me the easiest solution is to always include obstack.o in
libgnu.a.  If the system libc is up to date, then obstack.o will be
empty.

And another small tweak: Don't use alignof.h if __IBM__ALIGNOF__ is
defined (alignof.h just uses __alignof__ in that case), or when
__alignof__ is defined (for oddball systems, or testing).

-- 
Alan Modra
Australia Development Lab, IBM

>From 39a719f31f871143a011f368b5f74f03a9c33244 Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Thu, 30 Oct 2014 15:04:07 +1030
Subject: [PATCH] obstack macro return values, and always add obstack.o

Many of the obstack macros are not supposed to return a value, and
obstack_next_free is supposed to return a void* not a char*.

	* lib/obstack.h (obstack_next_free): Return void *.
	(obstack_1grow_fast, obstack_blank_fast): Return void.
	For __GNUC__ macros:
	(obstack_free): Return void.
	(obstack_1grow, obstack_blank): Remove now unnecessary (void)0.
	For !__GNUC__ macros:
	(obstack_make_room, obstack_grow, obstack_grow0,
	obstack_ptr_grow_fast, obstack_int_grow_fast): Return void.
	* lib/obstack.c: Don't include alignof.h if __alignof__ is defined,
	nor when __IBM_ALIGNOF__ is defined.
	* modules/obstack (configure.ac): Always add obstack.o to libgnu.a.
---
 ChangeLog       | 15 +++++++++++++++
 lib/obstack.c   |  3 ++-
 lib/obstack.h   | 30 +++++++++++++++++-------------
 modules/obstack |  3 +--
 4 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0d6c285..5574df9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2014-11-03  Alan Modra  <amodra@gmail.com>
+
+	obstack: macro return values, and always add obstack.o.
+	* lib/obstack.h (obstack_next_free): Return void *.
+	(obstack_1grow_fast, obstack_blank_fast): Return void.
+	For __GNUC__ macros:
+	(obstack_free): Return void.
+	(obstack_1grow, obstack_blank): Remove now unnecessary (void)0.
+	For !__GNUC__ macros:
+	(obstack_make_room, obstack_grow, obstack_grow0,
+	obstack_ptr_grow_fast, obstack_int_grow_fast): Return void.
+	* lib/obstack.c: Don't include alignof.h if __alignof__ is defined,
+	nor when __IBM_ALIGNOF__ is defined.
+	* modules/obstack (configure.ac): Always add obstack.o to libgnu.a.
+
 2014-10-30  Pádraig Brady  <P@draigBrady.com>
 
 	mountlist: don't use libmount to decide on dummy/remote
diff --git a/lib/obstack.c b/lib/obstack.c
index ba7dff3..e1707b9 100644
--- a/lib/obstack.c
+++ b/lib/obstack.c
@@ -48,7 +48,8 @@
 #endif
 
 #ifndef _OBSTACK_ELIDE_CODE
-# if !defined _LIBC && !defined __GNUC__
+# if (!defined _LIBC && !defined __GNUC__ && !defined __IBM__ALIGNOF__	      \
+      && !defined __alignof__)
 #  include <alignof.h>
 #  define __alignof__(type) alignof_type (type)
 # endif
diff --git a/lib/obstack.h b/lib/obstack.h
index ba4de1d..79edffb 100644
--- a/lib/obstack.h
+++ b/lib/obstack.h
@@ -219,7 +219,7 @@ extern int obstack_exit_failure;
 
 /* Pointer to next byte not yet allocated in current chunk.  */
 
-#define obstack_next_free(h)    ((h)->next_free)
+#define obstack_next_free(h) ((void *) (h)->next_free)
 
 /* Mask specifying low bits that should be clear in address of an object.  */
 
@@ -252,9 +252,9 @@ 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), (void) 0)
 
-#define obstack_blank_fast(h, n) ((h)->next_free += (n))
+#define obstack_blank_fast(h, n) ((h)->next_free += (n), (void) 0)
 
 #define obstack_memory_used(h) _obstack_memory_used (h)
 
@@ -322,8 +322,7 @@ extern int obstack_exit_failure;
     ({ struct obstack *__o = (OBSTACK);					      \
        if (obstack_room (__o) < 1)					      \
          _obstack_newchunk (__o, 1);					      \
-       obstack_1grow_fast (__o, datum);					      \
-       (void) 0; })
+       obstack_1grow_fast (__o, datum); })
 
 /* These assume that the obstack alignment is good enough for pointers
    or ints, and that the data added so far to the current object
@@ -365,8 +364,7 @@ extern int obstack_exit_failure;
        _OBSTACK_SIZE_T __len = (length);				      \
        if (obstack_room (__o) < __len)					      \
          _obstack_newchunk (__o, __len);				      \
-       obstack_blank_fast (__o, __len);					      \
-       (void) 0; })
+       obstack_blank_fast (__o, __len); })
 
 # define obstack_alloc(OBSTACK, length)					      \
   __extension__								      \
@@ -410,7 +408,8 @@ extern int obstack_exit_failure;
        if (__obj > (void *) __o->chunk && __obj < (void *) __o->chunk_limit)  \
          __o->next_free = __o->object_base = (char *) __obj;		      \
        else								      \
-         _obstack_free (__o, __obj); })
+         _obstack_free (__o, __obj);					      \
+       (void) 0; })
 
 #else /* not __GNUC__ */
 
@@ -435,14 +434,16 @@ extern int obstack_exit_failure;
 # define obstack_make_room(h, length)					      \
   ((h)->temp.i = (length),						      \
    ((obstack_room (h) < (h)->temp.i)					      \
-   ? (_obstack_newchunk ((h), (h)->temp.i), 0) : 0))
+    ? (_obstack_newchunk ((h), (h)->temp.i), 0) : 0),			      \
+   (void) 0)
 
 # define obstack_grow(h, where, length)					      \
   ((h)->temp.i = (length),						      \
    ((obstack_room (h) < (h)->temp.i)					      \
    ? (_obstack_newchunk ((h), (h)->temp.i), 0) : 0),			      \
    memcpy ((h)->next_free, where, (h)->temp.i),				      \
-   (h)->next_free += (h)->temp.i)
+   (h)->next_free += (h)->temp.i,					      \
+   (void) 0)
 
 # define obstack_grow0(h, where, length)				      \
   ((h)->temp.i = (length),						      \
@@ -450,7 +451,8 @@ extern int obstack_exit_failure;
    ? (_obstack_newchunk ((h), (h)->temp.i + 1), 0) : 0),		      \
    memcpy ((h)->next_free, where, (h)->temp.i),				      \
    (h)->next_free += (h)->temp.i,					      \
-   *((h)->next_free)++ = 0)
+   *((h)->next_free)++ = 0,						      \
+   (void) 0)
 
 # define obstack_1grow(h, datum)					      \
   (((obstack_room (h) < 1)						      \
@@ -468,10 +470,12 @@ extern int obstack_exit_failure;
    obstack_int_grow_fast (h, datum))
 
 # define obstack_ptr_grow_fast(h, aptr)					      \
-  (((const void **) ((h)->next_free += sizeof (void *)))[-1] = (aptr))
+  (((const void **) ((h)->next_free += sizeof (void *)))[-1] = (aptr),	      \
+   (void) 0)
 
 # define obstack_int_grow_fast(h, aint)					      \
-  (((int *) ((h)->next_free += sizeof (int)))[-1] = (aint))
+  (((int *) ((h)->next_free += sizeof (int)))[-1] = (aint),		      \
+   (void) 0)
 
 # define obstack_blank(h, length)					      \
   ((h)->temp.i = (length),						      \
diff --git a/modules/obstack b/modules/obstack
index 36dc444..bbcb974 100644
--- a/modules/obstack
+++ b/modules/obstack
@@ -13,8 +13,7 @@ stdint
 stdlib
 
 configure.ac:
-AC_FUNC_OBSTACK
-dnl Note: AC_FUNC_OBSTACK does AC_LIBSOURCES([obstack.h, obstack.c]).
+AC_LIBOBJ([obstack])
 
 Makefile.am: