[0/5] obstacks again

Message ID 5453E801.6050402@cs.ucla.edu
State Not applicable
Headers

Commit Message

Paul Eggert Oct. 31, 2014, 7:50 p.m. UTC
  On 10/29/2014 11:07 PM, Alan Modra wrote:
> So there are my reasons for leaving obstack_blank as is.

Thanks, they're persuasive.  With that in mind, 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.  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.

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

> Oh, and while I'm advocating breakage, the doc for obstack_next_free
> says it returns a void* but actually returns a char*.  Since it was
> deemed a good idea to similarly correct obstack_base, should we do the
> same for obstack_next_free?

Yes, that makes sense.  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.

> It's missing @safety markup for the
> previously undocumented obstack_begin, obstack_specify_allocation, and
> obstack_specify_allocation_with_arg macros.  I'm hoping someone else
> can do that as I'd just be guessing.  (Are they even appropriate for
> macros?)

Sorry, I don't know.

If I understand the recent set of proposed patches correctly, no further 
changes need to be made to gnulib, unless we decide to go ahead with the 
further "breakage" mentioned above.
  

Comments

Eric Blake Oct. 31, 2014, 10:13 p.m. UTC | #1
On 10/31/2014 01:50 PM, Paul Eggert wrote:
> On 10/29/2014 11:07 PM, Alan Modra wrote:
>> So there are my reasons for leaving obstack_blank as is.
> 
> Thanks, they're persuasive.  With that in mind, 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.  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.

Can we use ssize_t instead of size_t for obstack_blank_fast?
  
Paul Eggert Oct. 31, 2014, 10:24 p.m. UTC | #2
Eric Blake wrote:
> Can we use ssize_t instead of size_t for obstack_blank_fast?

Not portably, no.  ssize_t need not be the same width as size_t.

As I understand it, ssize_t was partly invented to support platforms where 
'read' took a 64-bit size_t argument and returned a 32-bit signed result -- and 
ssize_t is therefore best avoided except when dealing directly with system calls 
that return ssize_t.
  

Patch

From 049f337efd051bc4821173844fc9a5fdd38fb51a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 31 Oct 2014 12:45:22 -0700
Subject: [PATCH] obstack: add NEWS entry for recent incompatible changes

* NEWS: Describe recent changes.
---
 ChangeLog |  5 +++++
 NEWS      | 10 +++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 0d6c285..1f80f39 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2014-10-31  Paul Eggert  <eggert@cs.ucla.edu>
+
+	obstack: add NEWS entry for recent incompatible changes
+	* NEWS: Describe recent changes.
+
 2014-10-30  Pádraig Brady  <P@draigBrady.com>
 
 	mountlist: don't use libmount to decide on dummy/remote
diff --git a/NEWS b/NEWS
index 45d829b..23dd4f1 100644
--- a/NEWS
+++ b/NEWS
@@ -32,8 +32,16 @@  User visible incompatible changes
 
 Date        Modules         Changes
 
+2014-10-29  obstack         The obstack functions are no longer limited to
+                            int sizes; size values are now of type size_t.
+                            This changes both the ABI and the API.
+                            obstack_blank no longer accepts a negative size to
+                            shrink the current object; callers must now use
+                            obstack_blank_fast with a "negative" (actually,
+                            large positive) size for that.
+
 2014-02-23  diffseq         The members too_expensive, lo_minimal and hi_minimal
-                            were removed from public structureas, and the
+                            were removed from public structures, and the
                             find_minimal argument was removed from diag
                             and compareseq.
 
-- 
1.9.3