[0/5] obstacks again
diff mbox

Message ID 54589199.10006@cs.ucla.edu
State New
Headers show

Commit Message

Paul Eggert Nov. 4, 2014, 8:43 a.m. UTC
On 11/02/2014 11:07 PM, Alan Modra wrote:
> Really, it's a fiction that the size argument of obstack_blank_fast has a type.

Yes and no.  It's a fiction in the sense that obstack_blank_fast is a macro, so 
its argument doesn't actually have a C type.  But it's not a fiction in the 
sense that obstack_blank_fast can be documented as if it were a function, and if 
a program calls it with valid arguments that a function would have, it behaves 
like that function.

In this sense, obstack_blank_fast is like putchar.  Although putchar is a macro 
and strictly speaking its argument doesn't have a C type, it is documented to 
take a value of type int, and if you call it according to its specification it 
behaves properly.

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

But this doesn't suffice for obstack_blank_stack, which *can* shrink objects and 
where user code relies on this feature.  E.g., the following code (taken from 
GCC's finish_regexp_representation) should work if the current object size fits 
into 'int', and should continue to work if we change its 'int' to 'size_t' and 
allocate objects larger than what 'int' can represent:

    int length = obstack_object_size (&irp);
    obstack_blank_fast (&irp, -length);


> 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.

Thanks, I wrote a "harder" solution, which builds obstack.o only if the system 
version doesn't work with arbitrary size_t values.  This is always true now but 
won't be once these fixes make their way into glibc.

> Ianother 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). 

I'd rather insulate obstack from that internal detail of alignof.  I gave a shot 
at doing this in a different way, which preserves the insulation better.

A couple more things.  obstack_chunkfun and obstack_freefun should also return 
void.  And the code should avoid casting function values from one calling 
convention to another: this sort-of-works with the old API on glibc's current 
platforms but is likely to cause problems with the transition to the new one.

I installed the attached patches to gnulib to try to implement the above; please 
let me know of any issues.
From 3d99d9fb66ceea0f93a6cfeb496f0471efa212d3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 3 Nov 2014 16:34:52 -0800
Subject: [PATCH 1/4] obstack: port to platforms that #define __alignof__

* lib/obstack.c: Include <alignof.h> if !defined __alignof__,
not if !_LIBC.  We don't know of any platforms that #define
__alignof__, but it might be useful in tests.  Conversely,
glibc assumes GCC.
---
 ChangeLog     | 8 ++++++++
 lib/obstack.c | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Alan Modra Nov. 5, 2014, 11:21 a.m. UTC | #1
On Tue, Nov 04, 2014 at 12:43:05AM -0800, Paul Eggert wrote:
> >I've added: "Earlier versions of obstacks allowed you to use
> >@code{obstack_blank} to shrink objects. This will no longer work."
> 
> But this doesn't suffice for obstack_blank_stack, which *can* shrink objects
> and where user code relies on this feature.

Right, the entire para on shrinking object is now:

@cindex shrinking objects
You can use @code{obstack_blank_fast} with a ``negative'' size
argument to make the current object smaller.  Just don't try to shrink
it beyond zero length---there's no telling what will happen if you do
that.  Earlier versions of obstacks allowed you to use
@code{obstack_blank} to shrink objects.  This will no longer work.

> I installed the attached patches to gnulib to try to implement the above;
> please let me know of any issues.

Thanks for doing all this.  I particularly like the removal of
obstack_chunk_alloc and obstack_chunk_free casts for version 2, except
here:

#define obstack_chunkfun(h, newchunkfun)				      \
  ((void) ((h)->chunkfun.extra = _OBSTACK_CAST (void *(*) (void *, size_t),   \
                                                newchunkfun)))

and in obstack_freefun.  I think you need to keep the casts, otherwise
you can only specify new extra arg alloc and free functions.

Patch
diff mbox

diff --git a/ChangeLog b/ChangeLog
index 6728893..274b794 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@ 
+2014-11-03  Paul Eggert  <eggert@cs.ucla.edu>
+
+	obstack: port to platforms that #define __alignof__
+	* lib/obstack.c: Include <alignof.h> if !defined __alignof__,
+	not if !_LIBC.  We don't know of any platforms that #define
+	__alignof__, but it might be useful in tests.  Conversely,
+	glibc assumes GCC.
+
 2014-11-03  Pádraig Brady  <P@draigBrady.com>
 
 	linkat: don't unconditionally replace on GNU/Linux
diff --git a/lib/obstack.c b/lib/obstack.c
index ba7dff3..d763c57 100644
--- a/lib/obstack.c
+++ b/lib/obstack.c
@@ -48,7 +48,10 @@ 
 #endif
 
 #ifndef _OBSTACK_ELIDE_CODE
-# if !defined _LIBC && !defined __GNUC__
+/* If GCC, or if an oddball (testing?) host that #defines __alignof__,
+   use the already-supplied __alignof__.  Otherwise, this must be Gnulib
+   (as glibc assumes GCC); defer to Gnulib's alignof_type.  */
+# if !defined __GNUC__ && !defined __alignof__
 #  include <alignof.h>
 #  define __alignof__(type) alignof_type (type)
 # endif