diff mbox

[2/5] obstack tidy part 2

Message ID 20141029033222.GK4267@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Oct. 29, 2014, 3:32 a.m. UTC 
a) Don't be concerned about "not polluting the namespace with stddef.h
   symbols" in obstack.h, since gnulib string.h includes stddef.h
   anyway, and it seems unlikely that anyone would care.
b) Don't roll our own slow memcpy in _obstack_newchunk.
c) Rename obstack_free to _obstack_free.  This makes the naming
   consistent with other obstack functions and obviates the need for
   __obstack_free.  Ancient obstack.c defined both obstack_free and
   _obstack_free.  We continue to do that for _LIBC via an alias.
d) Miscellaneous macro fixes.  The expression used to test for gcc-2.8
   is clever, but nowadays gcc warns on undefined macros.  You'll get
   an undefined macro warning if simulating an old gcc with -U__GNUC__
   -U__GNUC_MINOR__ -D__GNUC__=1.

	* lib/obstack.h: Include stddef.h unconditionally.  Formatting fixes.
	(PTR_INT_TYPE): Delete, replace with ptrdiff_t.
	(__obstack_free): Delete, update refs.
	(_obstack_free): Rename from obstack_free.
	(__extension__): Avoid undefined macro warning for __GNUC_MINOR__.
	(obstack_object_size, obstack_room): Parenthesise !__GNUC__ versions.
	* lib/obstack.c: Don't include stddef.h.
	(COPYING_UNIT): Delete.
	(_obstack_begin): Formatting fix.
	(_obstack_newchunk): Use memcpy to move existing object to new chunk.
	(_obstack_free): Rename from __obstack_free, update alias.  Move
	undef of obstack_free to where it is needed.
---
 lib/obstack.c | 47 +++++++----------------------------------------
 lib/obstack.h | 53 +++++++++++++++++++----------------------------------
 2 files changed, 26 insertions(+), 74 deletions(-)

Comments

Roland McGrath Oct. 30, 2014, 7:51 p.m. UTC | #1 
> a) Don't be concerned about "not polluting the namespace with stddef.h
>    symbols" in obstack.h, since gnulib string.h includes stddef.h
>    anyway, and it seems unlikely that anyone would care.

libc is where this sort of constraint is most likely to be important.  I
doubt gnulib users care.  Since obstack.h is not standard, we are free to
decide we don't care for libc either.  But it is not something to be done
lightly.  If this change doesn't actually enable anything else you're
doing, please leave it for later as a separate individual item.

> b) Don't roll our own slow memcpy in _obstack_newchunk.

Good change.

> c) Rename obstack_free to _obstack_free.  This makes the naming
>    consistent with other obstack functions and obviates the need for
>    __obstack_free.  Ancient obstack.c defined both obstack_free and
>    _obstack_free.  We continue to do that for _LIBC via an alias.
> d) Miscellaneous macro fixes.  The expression used to test for gcc-2.8
>    is clever, but nowadays gcc warns on undefined macros.  You'll get
>    an undefined macro warning if simulating an old gcc with -U__GNUC__
>    -U__GNUC_MINOR__ -D__GNUC__=1.

These seem OK to me.

You didn't report what testing you did for the libc build, which is
mandatory for approval.  In particular, we have 'make check-abi' to be
concerned with, which is not something relevant to gnulib.
diff mbox

Patch

diff --git a/lib/obstack.c b/lib/obstack.c
index 66573df..2d5dfbc 100644
--- a/lib/obstack.c
+++ b/lib/obstack.c
@@ -47,8 +47,6 @@ 
 # endif
 #endif
 
-#include <stddef.h>
-
 #ifndef ELIDE_CODE
 
 
@@ -76,14 +74,6 @@  enum
   DEFAULT_ROUNDING = sizeof (union fooround)
 };
 
-/* When we copy a long block of data, this is the unit to do it with.
-   On some machines, copying successive ints does not work;
-   in such a case, redefine COPYING_UNIT to 'long' (if that works)
-   or 'char' as a last resort.  */
-# ifndef COPYING_UNIT
-#  define COPYING_UNIT int
-# endif
-
 
 # ifdef _LIBC
 #  if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_3_4)
@@ -161,8 +151,7 @@  _obstack_begin (struct obstack *h,
     (*obstack_alloc_failed_handler) ();
   h->next_free = h->object_base = __PTR_ALIGN ((char *) chunk, chunk->contents,
                                                alignment - 1);
-  h->chunk_limit = chunk->limit
-                     = (char *) chunk + h->chunk_size;
+  h->chunk_limit = chunk->limit = (char *) chunk + h->chunk_size;
   chunk->prev = 0;
   /* The initial chunk now contains no empty object.  */
   h->maybe_empty_object = 0;
@@ -231,8 +220,6 @@  _obstack_newchunk (struct obstack *h, int length)
   struct _obstack_chunk *new_chunk;
   long new_size;
   long obj_size = h->next_free - h->object_base;
-  long i;
-  long already;
   char *object_base;
 
   /* Compute size for new chunk.  */
@@ -252,25 +239,8 @@  _obstack_newchunk (struct obstack *h, int length)
   object_base =
     __PTR_ALIGN ((char *) new_chunk, new_chunk->contents, h->alignment_mask);
 
-  /* Move the existing object to the new chunk.
-     Word at a time is fast and is safe if the object
-     is sufficiently aligned.  */
-  if (h->alignment_mask + 1 >= DEFAULT_ALIGNMENT)
-    {
-      for (i = obj_size / sizeof (COPYING_UNIT) - 1;
-           i >= 0; i--)
-        ((COPYING_UNIT *) object_base)[i]
-          = ((COPYING_UNIT *) h->object_base)[i];
-      /* We used to copy the odd few remaining bytes as one extra COPYING_UNIT,
-         but that can cross a page boundary on a machine
-         which does not do strict alignment for COPYING_UNITS.  */
-      already = obj_size / sizeof (COPYING_UNIT) * sizeof (COPYING_UNIT);
-    }
-  else
-    already = 0;
-  /* Copy remaining bytes one by one.  */
-  for (i = already; i < obj_size; i++)
-    object_base[i] = h->object_base[i];
+  /* Move the existing object to the new chunk.  */
+  memcpy (object_base, h->object_base, obj_size);
 
   /* If the object just copied was the only data in OLD_CHUNK,
      free that chunk and remove it from the chain.
@@ -322,10 +292,8 @@  _obstack_allocated_p (struct obstack *h, void *obj)
 /* Free objects in obstack H, including OBJ and everything allocate
    more recently than OBJ.  If OBJ is zero, free everything in H.  */
 
-# undef obstack_free
-
 void
-__obstack_free (struct obstack *h, void *obj)
+_obstack_free (struct obstack *h, void *obj)
 {
   struct _obstack_chunk *lp;    /* below addr of any objects in this chunk */
   struct _obstack_chunk *plp;   /* point to previous chunk if any */
@@ -353,11 +321,10 @@  __obstack_free (struct obstack *h, void *obj)
     /* obj is not in any of the chunks! */
     abort ();
 }
-
 # ifdef _LIBC
-/* Older versions of libc used a function _obstack_free intended to be
-   called by non-GCC compilers.  */
-strong_alias (obstack_free, _obstack_free)
+/* Older versions of libc defined both _obstack_free and obstack_free.  */
+#  undef obstack_free
+strong_alias (_obstack_free, obstack_free)
 # endif
 
 int
diff --git a/lib/obstack.h b/lib/obstack.h
index cd90e4e..f3a7c77 100644
--- a/lib/obstack.h
+++ b/lib/obstack.h
@@ -104,17 +104,7 @@ 
 #ifndef _OBSTACK_H
 #define _OBSTACK_H 1
 
-/* We need the type of a pointer subtraction.  If __PTRDIFF_TYPE__ is
-   defined, as with GNU C, use that; that way we don't pollute the
-   namespace with <stddef.h>'s symbols.  Otherwise, include <stddef.h>
-   and use ptrdiff_t.  */
-
-#ifdef __PTRDIFF_TYPE__
-# define PTR_INT_TYPE __PTRDIFF_TYPE__
-#else
-# include <stddef.h>
-# define PTR_INT_TYPE ptrdiff_t
-#endif
+#include <stddef.h>
 
 /* If B is the base of an object addressed by P, return the result of
    aligning P to the next multiple of A + 1.  B and P must be of type
@@ -122,15 +112,15 @@ 
 
 #define __BPTR_ALIGN(B, P, A) ((B) + (((P) - (B) + (A)) & ~(A)))
 
-/* Similar to _BPTR_ALIGN (B, P, A), except optimize the common case
+/* Similar to __BPTR_ALIGN (B, P, A), except optimize the common case
    where pointers can be converted to integers, aligned as integers,
-   and converted back again.  If PTR_INT_TYPE is narrower than a
+   and converted back again.  If ptrdiff_t is narrower than a
    pointer (e.g., the AS/400), play it safe and compute the alignment
    relative to B.  Otherwise, use the faster strategy of computing the
    alignment relative to 0.  */
 
 #define __PTR_ALIGN(B, P, A)						      \
-  __BPTR_ALIGN (sizeof (PTR_INT_TYPE) < sizeof (void *) ? (B) : (char *) 0, \
+  __BPTR_ALIGN (sizeof (ptrdiff_t) < sizeof (void *) ? (B) : (char *) 0,      \
                 P, A)
 
 #include <string.h>
@@ -159,7 +149,7 @@  struct obstack          /* control current object in current chunk */
   char *chunk_limit;            /* address of char after current chunk */
   union
   {
-    PTR_INT_TYPE i;
+    ptrdiff_t i;
     void *p;
   } temp;                       /* Temporary for some macros.  */
   int alignment_mask;           /* Mask of alignment for each object. */
@@ -182,6 +172,7 @@  struct obstack          /* control current object in current chunk */
 /* Declare the external functions we use; they are in obstack.c.  */
 
 extern void _obstack_newchunk (struct obstack *, int);
+extern void _obstack_free (struct obstack *, void *);
 extern int _obstack_begin (struct obstack *, int, int,
                            void *(*)(long), void (*)(void *));
 extern int _obstack_begin_1 (struct obstack *, int, int,
@@ -189,13 +180,6 @@  extern int _obstack_begin_1 (struct obstack *, int, int,
                              void (*)(void *, void *), void *);
 extern int _obstack_memory_used (struct obstack *) __attribute_pure__;
 
-/* The default name of the function for freeing a chunk is 'obstack_free',
-   but gnulib users can override this by defining '__obstack_free'.  */
-#ifndef __obstack_free
-# define __obstack_free obstack_free
-#endif
-extern void __obstack_free (struct obstack *, void *);
-
 
 /* Error handler called when 'obstack_chunk_alloc' failed to allocate
    more memory.  This can be set to a user defined function which
@@ -235,7 +219,7 @@  extern int obstack_exit_failure;
                   (void *(*)(long))obstack_chunk_alloc,			      \
                   (void (*)(void *))obstack_chunk_free)
 
-#define obstack_specify_allocation(h, size, alignment, chunkfun, freefun)  \
+#define obstack_specify_allocation(h, size, alignment, chunkfun, freefun)     \
   _obstack_begin ((h), (size), (alignment),				      \
                   (void *(*)(long))(chunkfun),				      \
                   (void (*)(void *))(freefun))
@@ -245,10 +229,10 @@  extern int obstack_exit_failure;
                     (void *(*)(void *, long))(chunkfun),		      \
                     (void (*)(void *, void *))(freefun), (arg))
 
-#define obstack_chunkfun(h, newchunkfun) \
+#define obstack_chunkfun(h, newchunkfun)				      \
   ((h)->chunkfun = (struct _obstack_chunk *(*)(void *, long))(newchunkfun))
 
-#define obstack_freefun(h, newfreefun) \
+#define obstack_freefun(h, newfreefun)					      \
   ((h)->freefun = (void (*)(void *, struct _obstack_chunk *))(newfreefun))
 
 #define obstack_1grow_fast(h, achar) (*((h)->next_free)++ = (achar))
@@ -258,7 +242,7 @@  extern int obstack_exit_failure;
 #define obstack_memory_used(h) _obstack_memory_used (h)
 
 #if defined __GNUC__
-# if ! (2 < __GNUC__ + (8 <= __GNUC_MINOR__))
+# if !defined __GNUC_MINOR__ || __GNUC__ * 1000 + __GNUC_MINOR__ < 2008
 #  define __extension__
 # endif
 
@@ -331,7 +315,7 @@  extern int obstack_exit_failure;
     ({ struct obstack *__o = (OBSTACK);					      \
        if (__o->next_free + sizeof (void *) > __o->chunk_limit)		      \
          _obstack_newchunk (__o, sizeof (void *));			      \
-       obstack_ptr_grow_fast (__o, datum); })				      \
+       obstack_ptr_grow_fast (__o, datum); })
 
 # define obstack_int_grow(OBSTACK, datum)				      \
   __extension__								      \
@@ -406,17 +390,18 @@  extern int obstack_exit_failure;
        void *__obj = (OBJ);						      \
        if (__obj > (void *) __o->chunk && __obj < (void *) __o->chunk_limit)  \
          __o->next_free = __o->object_base = (char *) __obj;		      \
-       else (__obstack_free) (__o, __obj); })
+       else								      \
+         _obstack_free (__o, __obj); })
 
 #else /* not __GNUC__ */
 
-# define obstack_object_size(h) \
-  (unsigned) ((h)->next_free - (h)->object_base)
+# define obstack_object_size(h)						      \
+  ((unsigned) ((h)->next_free - (h)->object_base))
 
 # define obstack_room(h)						      \
-  (unsigned) ((h)->chunk_limit - (h)->next_free)
+  ((unsigned) ((h)->chunk_limit - (h)->next_free))
 
-# define obstack_empty_p(h) \
+# define obstack_empty_p(h)						      \
   ((h)->chunk->prev == 0						      \
    && (h)->next_free == __PTR_ALIGN ((char *) (h)->chunk,		      \
                                      (h)->chunk->contents,		      \
@@ -504,7 +489,7 @@  extern int obstack_exit_failure;
       && (h)->temp.i < (h)->chunk_limit - (char *) (h)->chunk))		      \
     ? (void) ((h)->next_free = (h)->object_base				      \
                           = (h)->temp.i + (char *) (h)->chunk)		      \
-    : (__obstack_free) (h, (h)->temp.i + (char *) (h)->chunk)))
+    : _obstack_free (h, (h)->temp.i + (char *) (h)->chunk)))
 
 #endif /* not __GNUC__ */
 
@@ -512,4 +497,4 @@  extern int obstack_exit_failure;
 }       /* C++ */
 #endif
 
-#endif /* obstack.h */
+#endif /* _OBSTACK_H */