Re: new gnulib/maint-tools git repository

2023-05-31 Thread Ben Pfaff
On Wed, May 31, 2023 at 4:39 AM Bruno Haible  wrote:
> We now have a separate git repository for gnulib maintainer tools.
> I asked the savannah admins to create the repository and filled it
> with the following tools:

That is super! I did not know that Savannah supported more than one
repository per project.

It might be hard even for gnulib maintainers to notice it. I suggest
updating https://www.gnu.org/software/gnulib/ to mention it, and to
point to it from an appropriate place inside Gnulib also.



Re: checking whether a double is in the range of long

2023-04-23 Thread Ben Pfaff
On Sat, Apr 22, 2023 at 10:40 PM Paul Eggert  wrote:
> On 2023-04-22 16:34, Ben Pfaff wrote:
> > determine whether converting 'd' to 'long' would
> > yield a 'long' with the same value as 'd'
>
> LONG_MIN - 1.0 < d && d < LONG_MAX + 1.0 && d == (long) d
>
> On all practical platforms this should avoid undefined behavior and
> works correctly even if rounding occurs. You can replace "d == (long) d"
> with "d == floor (d)" if you prefer.

That's much simpler. Thank you.



Re: checking whether a double is in the range of long

2023-04-22 Thread Ben Pfaff
On Sat, Apr 22, 2023 at 5:52 PM Bruno Haible  wrote:
>
> Ben Pfaff wrote:
> > determine whether converting 'd' to 'long' would
> > yield a 'long' with the same value as 'd'.
>
> Maybe
>   d == (double) (long) d
> ?
>
> Just a wild guess. I haven't tested it.

I don't trust the undefined behavior in conversions that go outside the
valid range. The program I showed gave me different output with and
without GCC optimization turned on, for example.



Re: checking whether a double is in the range of long

2023-04-22 Thread Ben Pfaff
On Sat, Apr 22, 2023 at 4:34 PM Ben Pfaff  wrote:
> Before this afternoon, I thought that a check like this for a double 'd':
> d == floor (d) && d >= LONG_MIN && d <= LONG_MAX
> was sufficient to determine whether converting 'd' to 'long' would
> yield a 'long' with the same value as 'd'.
>
> Now I realize that this is wrong. In particular, take a look at the
> following program:
>
> #include 
> #include 
>
> int main (void)
> {
>   long i = LONG_MAX;
>   double d = i;
>   long j = d;
>   printf ("%ld, %f, %ld\n", i, d, j);
>   return 0;
> }
>
> On my system, this prints:
>
> 9223372036854775807, 9223372036854775808.00, -9223372036854775808
>
> In other words, LONG_MAX gets rounded up to 2**63 when it's converted to
> 'double', which makes sense because 'double' only has 53 bits of precision,
> but this also means that 'd <= LONG_MAX' and still doesn't fit in 'long', as 
> one
> can see from it getting converted to a wrong answer (-2**63 instead of
> 2**63) when converted back to 'long'. And of course any answer is OK there,
> since this out-of-range conversion yields undefined behavior.
>
> Can anyone suggest a correct way to check whether a 'double' is in the
> range of 'long'?

I figured out a solution to the problem I wanted to solve. After
thinking about it for
a while longer, I realized that what I really wanted was the range of
integers that
'double' represents without loss of precision, that is, most
commonly,-2**53...2**53.
And then I wanted the intersection of this range with the range of
'long'. Typically
that's going to be -2**53...2**53 also, of course.

I came up with the following. Feedback welcomed!

#include 
#include 
#include 
#include 

/* Maximum positive integer 'double' represented with no loss of precision
   (that is, with unit precision).

   The maximum negative integer with this property is -DBL_UNIT_MAX. */
#if DBL_MANT_DIG == 53  /* 64-bit double */
#define DBL_UNIT_MAX 9007199254740992.0
#elif DBL_MANT_DIG == 64/* 80-bit double */
#define DBL_UNIT_MAX 18446744073709551616.0
#elif DBL_MANT_DIG == 113   /* 128-bit double */
#define DBL_UNIT_MAX 10384593717069655257060992658440192.0
#else
#error "Please define DBL_UNIT_MAX for your system (as 2**DBL_MANT_DIG)."
#endif

/* Intersection of ranges [LONG_MIN,LONG_MAX] and [-DBL_UNIT_MAX,DBL_UNIT_MAX],
   as a range of 'long's.  This range is the (largest contiguous) set of
   integer values that can be safely converted between 'long' and 'double'
   without loss of precision. */
#if DBL_MANT_DIG < LONG_WIDTH - 1
#define DBL_UNIT_LONG_MIN ((long) -DBL_UNIT_MAX)
#define DBL_UNIT_LONG_MAX ((long) DBL_UNIT_MAX)
#else
#define DBL_UNIT_LONG_MIN LONG_MIN
#define DBL_UNIT_LONG_MAX LONG_MAX
#endif

int main (void)
{
  long i = DBL_UNIT_LONG_MAX;
  double d = i;
  long j = d;
  printf ("%ld, %f, %ld\n", i, d, j);

  printf ("%f\n", DBL_UNIT_MAX);
  printf ("%d, %d\n", DBL_MANT_DIG, LONG_WIDTH);
  printf ("%ld, %ld\n", DBL_UNIT_LONG_MIN, DBL_UNIT_LONG_MAX);
  printf ("%ld, %ld\n", LONG_MIN, LONG_MAX);

  return 0;
}



checking whether a double is in the range of long

2023-04-22 Thread Ben Pfaff
Before this afternoon, I thought that a check like this for a double 'd':
d == floor (d) && d >= LONG_MIN && d <= LONG_MAX
was sufficient to determine whether converting 'd' to 'long' would
yield a 'long' with the same value as 'd'.

Now I realize that this is wrong. In particular, take a look at the
following program:

#include 
#include 

int main (void)
{
  long i = LONG_MAX;
  double d = i;
  long j = d;
  printf ("%ld, %f, %ld\n", i, d, j);
  return 0;
}

On my system, this prints:

9223372036854775807, 9223372036854775808.00, -9223372036854775808

In other words, LONG_MAX gets rounded up to 2**63 when it's converted to
'double', which makes sense because 'double' only has 53 bits of precision,
but this also means that 'd <= LONG_MAX' and still doesn't fit in 'long', as one
can see from it getting converted to a wrong answer (-2**63 instead of
2**63) when converted back to 'long'. And of course any answer is OK there,
since this out-of-range conversion yields undefined behavior.

Can anyone suggest a correct way to check whether a 'double' is in the
range of 'long'?

One workaround would be to check for the range of 'int' (or int32_t, I guess
really), since there's not going to be any loss of precision converting
INT(32)_MAX to 'double'. But I'd rather support a wider range in the code
I'm working on.

Thanks,

Ben.



Re: adapting relocatable-prog to non-recursive make

2022-03-24 Thread Ben Pfaff
Does the patch I mentioned earlier make sense?



Re: adapting relocatable-prog to non-recursive make

2022-03-13 Thread Ben Pfaff
On Sun, Mar 13, 2022 at 3:44 PM Bruno Haible  wrote:
>
> Hi Ben,
>
> > When I try to use the Gnulib relocatable-prog module with a program
> > that has an uninstall-hook, Automake gives a warning like this:
> >
> > po/automake.mk:97: warning: uninstall-hook was already defined in
> > condition RELOCATABLE_VIA_WRAPPER, which is included in condition TRUE
> > ...
> > Makefile.am:84:   'po/automake.mk' included from here
> > gl/automake.mk:2187: ... 'uninstall-hook' previously defined here
> > Makefile.am:83:   'gl/automake.mk' included from here
> > Makefile.am:105: warning: uninstall-hook was already defined in
> > condition RELOCATABLE_VIA_WRAPPER, which is included in condition TRUE
> > ...
> > gl/automake.mk:2187: ... 'uninstall-hook' previously defined here
> > Makefile.am:83:   'gl/automake.mk' included from here
>
> What do these two Makefiles look like:
> 1) po/automake.mk around line 97 ?
> 2) gl/automake.mk around line 2187 ?

This is GNU PSPP, tip of master. If you download a configured source
snapshot from
https://benpfaff.org/~blp/pspp-master/20220313140713/source/pspp-1.5.3-g75c0ae.tar.gz
and then run "automake", you can see the messages (line numbers have
changed a little
because I've been doing work):

po/automake.mk:97: warning: uninstall-hook was already defined in
condition RELOCATABLE_
VIA_WRAPPER, which is included in condition TRUE ...
Makefile.am:82:   'po/automake.mk' included from here
gl/automake.mk:2187: ... 'uninstall-hook' previously defined here
Makefile.am:81:   'gl/automake.mk' included from here
Makefile.am:103: warning: uninstall-hook was already defined in
condition RELOCATABLE_VI
A_WRAPPER, which is included in condition TRUE ...
gl/automake.mk:2187: ... 'uninstall-hook' previously defined here
Makefile.am:81:   'gl/automake.mk' included from here

and then po/automake.mk starting from line 97 is:

uninstall-hook: uninstall-gmofiles
uninstall-gmofiles:
   for f in $(GMOFILES); do \
 lang=`echo $$f | $(SED) -e 's%po/\(.*\)\.gmo%\1%' ` ; \
 rm -f $(DESTDIR)$(prefix)/share/locale/$$lang/LC_MESSAGES/$(DOMAIN).mo
; \
   done
.PHONY: uninstall-gmofiles

and the relevant part of gl/automake.mk is:

if RELOCATABLE_VIA_WRAPPER
uninstall-hook: uninstall-relocwrapper
uninstall-relocwrapper:
   if test $(RELOCATABLE) = yes; then \
 case '$(EXEEXT)' in \
   .bin*) ;; \
   *) cd $(top_builddir) && \
  $(MAKE) $(AM_MAKEFLAGS) EXEEXT=.bin$(EXEEXT) \
  AM_MAKEFLAGS='$(AM_MAKEFLAGS) EXEEXT=.bin$(EXEEXT)' \
  uninstall ;; \
 esac; \
   fi
endif
## Automake warns about conditionally added dependencies to
unconditionally defined targ
ets.
.PHONY: uninstall-relocwrapper

Oh, also Makefile.am at line 103 is:

uninstall-hook: $(UNINSTALL_DATA_HOOKS)

Thanks,

Ben.



[PATCH] NEWS: Document Automake 1.14 requirement here too.

2022-03-13 Thread Ben Pfaff
It had been documented there before for the Automake 1.11 requirement.
* NEWS: Mention the change.
---
 ChangeLog | 6 ++
 NEWS  | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index dcdb38f8c8..0586a854d6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2022-03-13  Ben Pfaff  
+
+   Document Automake 1.14 requirement in NEWS, too, since it had been
+   documented there before for the Automake 1.11 requirement.
+   * NEWS: Mention the change.
+
 2022-03-13  Bruno Haible  
 
sigsegv: Add support for Linux/PowerPC (32-bit) with musl libc.
diff --git a/NEWS b/NEWS
index 1a1c21970a..e5a875021e 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,8 @@ Important general notes
 
 DateModules Changes
 
+2022-02-20  (all)   Automake >= 1.14 and Autoconf >= 2.64 are required.
+
 2021-06-04  (all)   The license notices in source files are now really
 stating the effective license, rather than a fake
 GPL notice.
-- 
2.35.1




adapting relocatable-prog to non-recursive make

2022-03-13 Thread Ben Pfaff
When I try to use the Gnulib relocatable-prog module with a program
that has an uninstall-hook, Automake gives a warning like this:

po/automake.mk:97: warning: uninstall-hook was already defined in
condition RELOCATABLE_VIA_WRAPPER, which is included in condition TRUE
...
Makefile.am:84:   'po/automake.mk' included from here
gl/automake.mk:2187: ... 'uninstall-hook' previously defined here
Makefile.am:83:   'gl/automake.mk' included from here
Makefile.am:105: warning: uninstall-hook was already defined in
condition RELOCATABLE_VIA_WRAPPER, which is included in condition TRUE
...
gl/automake.mk:2187: ... 'uninstall-hook' previously defined here
Makefile.am:83:   'gl/automake.mk' included from here

It appears that applying a patch similar to the following in Gnulib
will fix the problem. I am not confident about it and could use advice
or assistance.

diff --git a/modules/relocatable-prog b/modules/relocatable-prog
index 8e22b20dd0..bb4bc7ce9f 100644
--- a/modules/relocatable-prog
+++ b/modules/relocatable-prog
@@ -38,8 +38,8 @@ if GL_COND_OBJ_PROGRELOC
 lib_SOURCES += progreloc.c
 endif
 DEFS += -DEXEEXT=\"@EXEEXT@\"
-if RELOCATABLE_VIA_WRAPPER
 uninstall-hook: uninstall-relocwrapper
+if RELOCATABLE_VIA_WRAPPER
 uninstall-relocwrapper:
 if test $(RELOCATABLE) = yes; then \
   case '$(EXEEXT)' in \
@@ -50,8 +50,10 @@ uninstall-relocwrapper:
uninstall ;; \
   esac; \
 fi
+else
+uninstall-relocwrapper:
+@# Nothing to do.
 endif
-## Automake warns about conditionally added dependencies to
unconditionally defined targets.
 .PHONY: uninstall-relocwrapper

 Include:


 Include:

Thanks,

Ben.



Re: coloured emojis and regional indicators kill gitk

2021-12-31 Thread Ben Pfaff
On Fri, Dec 31, 2021 at 6:19 AM Bruno Haible  wrote:
> gitk crashes when displaying a specific commit in the gnulib git repo.
>
> How to reproduce:
> $ gitk
> Navigate to the commit 2995fb5e993a5d7434d96465758087b35a1488ac

Interesting. FWIW, I don't see the crash here with
git-gui version 0.21.0.99.gdf4f9e
libXft-2.3.3-7.fc35.x86_64
using Fedora Core 35.

I did get a funny message I've never seen before from "git fetch":
$ git fetch
remote: Counting objects: 19423, done.
remote: warning: suboptimal pack - out of memory
remote: Compressing objects: 100% (3819/3819), done.

I guess Savannah is running low on memory.



Re: Accommodate non-recursive Automake in a less hacky way

2021-12-15 Thread Ben Pfaff
On Wed, Dec 15, 2021 at 11:31 AM Bruno Haible  wrote:
> With the attached patches, 'gnulib-tool' gains the functionality to
> generate a "non-recursive" Automake Makefile.am snippet, similar to
> what the 'non-recursive-gnulib-prefix-hack' module does.

Thank you! I favor nonrecursive Makefiles and I intend to try this out in GNU
PSPP when I find the time.



Re: close() behavior when there are multiple fds for a socket

2021-10-21 Thread Ben Pfaff
On Thu, Oct 21, 2021 at 3:02 PM Bernhard Voelker
 wrote:
> Would you mind adding the opengroup bug here for reference?

Here you go:
https://www.austingroupbugs.net/view.php?id=1525



Re: close() behavior when there are multiple fds for a socket

2021-10-15 Thread Ben Pfaff
On Fri, Oct 15, 2021 at 11:40 AM Eric Blake  wrote:
> On Fri, Oct 15, 2021 at 09:10:04AM -0700, Ben Pfaff wrote:
> > What a bureaucratic organization! The bug report form, once I got a login,
> > requires one to specify a page number and line number, but it seems that
> > you can only get the PDF version of the spec that has that information  if
> > you belong to an organization that is a member, so I filled those in with 
> > "?"
> > and am hoping for the best.
>
> That's fine.  Adding page numbers after the fact is something that
> happens rather frequently; and I've done in for your bug in particular
> just now.

Thanks so much!



Re: close() behavior when there are multiple fds for a socket

2021-10-15 Thread Ben Pfaff
On Thu, Oct 14, 2021 at 4:55 PM Paul Eggert  wrote:
>
> On 10/14/21 16:53, Ben Pfaff wrote:
> > The chair of the group
> > appears to be Andrew Josey, but he hides his email
>
> Sorry, I should have told you it's Andrew Josey .

Thanks!

What a bureaucratic organization! The bug report form, once I got a login,
requires one to specify a page number and line number, but it seems that
you can only get the PDF version of the spec that has that information  if
you belong to an organization that is a member, so I filled those in with "?"
and am hoping for the best.



Re: close() behavior when there are multiple fds for a socket

2021-10-14 Thread Ben Pfaff
On Thu, Oct 14, 2021 at 4:09 PM Paul Eggert  wrote:
>
> On 10/14/21 16:03, Ben Pfaff wrote:
>
> > https://pubs.opengroup.org/onlinepubs/9699919799/
> >
> > This specification has a lot of cases that explicitly apply only to the
> > last close of an object, either using that language or by saying that
> > something happens when "all file descriptors associated with" an
> > object are closed. But it doesn't say that for sockets.
>
> Yes, and you're right that this is a typo.

Thanks.

> > (Should I submit a request for clarification to the spec maintainers?)
>
> Sure, and you can do so as described here:
>
> https://www.opengroup.org/austin/
>
> (look for "Defect Reports").

That's harder than it should be. To get a login to their bug tracker,
you have to email the chair of the group. The chair of the group
appears to be Andrew Josey, but he hides his email so you can't
find it. Anyway, after some web searches I think I found his real
email address, so I'll report it once he creates my account.

Thanks again!



close() behavior when there are multiple fds for a socket

2021-10-14 Thread Ben Pfaff
I guess this isn't the best place to ask this question, but I think there
are knowledgeable people here. I don't know the right place, so I'd
welcome being redirected to a better one.

The POSIX spec for close() is here:
https://pubs.opengroup.org/onlinepubs/9699919799/

This specification has a lot of cases that explicitly apply only to the
last close of an object, either using that language or by saying that
something happens when "all file descriptors associated with" an
object are closed. But it doesn't say that for sockets. Instead, it
just says, "If fildes refers to a socket, close() shall cause the
socket to be destroyed," which implies that closing any file
descriptor for a socket destroys the socket. I think that must be
wrong, because I don't think that's the behavior on the systems
I've used.

Do I misunderstand the spec? Or the behavior on real systems?
(Should I submit a request for clarification to the spec maintainers?)

Thanks,

Ben.



Re: double _close()?

2021-10-14 Thread Ben Pfaff
On Thu, Oct 14, 2021 at 10:38 AM Paul Eggert  wrote:
>
> On 10/14/21 3:47 AM, Gisle Vanem wrote:
> > Maybe a memory-mapped I/O in GNU-diff
> > could improve the speed?
>
> I tried this lng ago (circa 1990) with either GNU grep or GNU diff
> (can't remember which) and it made performance worse. Of course it's
> platform-dependent.

That was 31 years ago!  (Linux kernel 1.0 was released in 1994.)

ripgrep uses mmap sometimes, depending on heuristics, and there are
reports online that it is sometimes much faster. For example, this Hacker
News claims 13x improvement in one case:
https://news.ycombinator.com/item?id=19806804



Re: Undefined use of weak symbols in gnulib

2021-04-29 Thread Ben Pfaff
On Wed, Apr 28, 2021 at 7:40 AM Bruno Haible  wrote:
> So, in the normal cases (link with '-lpthread', link without '-lpthread',
> and even with dlopen()), everything will work fine. The only problematic
> case thus is the the use of LD_PRELOAD. Right?
>
> I think few packages in a distro will be affected. And few users are
> using LD_PRELOAD on their own, because since the time when glibc
> started to use 'internal' calls to system calls where possible, there
> are not a lot of uses of LD_PRELOAD that still work.

One important use of LD_PRELOAD is for the "fakeroot" program that
Debian and derived distributions tend to use to build packages.  I haven't
followed along carefully enough to know whether this is likely to cause
problems for what we're talking about now.



Re: Portabilty of poll trick

2021-04-27 Thread Ben Pfaff
On Tue, Apr 27, 2021 at 3:47 PM Bastien ROUCARIES
 wrote:
>
> Le mar. 27 avr. 2021 à 22:40, Bruno Haible  a écrit :
> >
> > Hi Bastien,
> >
> > > I want to assess the safety and portability of the following trick,
> >
> > I would want to help you with this, but I can't. You have not stated:
> >   - What is this code supposed to do?
> I want to shutdown (2) a pipe, in a multithread application, in order
> to get out a poll(2) wait state
> >   - Why is it a "trick"? What advantages does it have compared to the code
> > a naïve developer would write?
> The naive delevopper will:
> - close the end of the pipe, but it does not break poll(2), and
> moreover in multithread context
> it is not safe due to fd reuse
> -use timeout but in this case why use poll...
>
> The goal is also to shutdown an eventfd but without kernel support, I
> suppose it is not possible...

Can you use socketpair() instead of pipe()?



Re: gl_list API

2021-04-18 Thread Ben Pfaff
On Tue, Apr 6, 2021 at 1:30 PM Bruno Haible  wrote:
> Paul Eggert wrote:
> > Yes, that would be portable. But that cast indicates another problem
> > with the API. It should return void *, not void const * (think of strchr
> > as the model here).
>
> strchr is a bad model here: You pass it a pointer to a read-only storage
> (such as a literal string) and receive a pointer that you "can" write
> to: The compiler cannot warn about
>strchr ("literal", 'e') [2] = 'x';

I've gotten in the habit of having two functions in cases like this. If
strchr were designed according to the principles that I try to apply,
we'd have functions like this:

const char *strchr (const char *, int);
char *strchr_rw (char *, int);

The implementation of strchr is then just a wrapper around the
strchr_rw():

const char *
strchr (const char *s, int c)
{
return strchr_rw ((char *) s, c);
}

although if I'm doing this in PSPP I would probably use CONST_CAST:
return strchr_rw (CONST_CAST (char *, s), c);



Re: cast macros

2021-04-18 Thread Ben Pfaff
On Tue, Apr 6, 2021 at 4:04 PM Bruno Haible  wrote:
>
> I wrote:
> > So far we have been lacking type-casts that warn for invalid use;
> > it is well possible that with the PSPP macros (or with Marc's approach
> > of _Generic), we can design a type-safe wrapper around gl_list.h
>
> Here is a simplified test case: The simplest container type is a box type,
> that contain just one value. Can one of you complete this code, so that
> it produces the warnings / no warnings as indicated?
>
> 
> typedef const void ** gl_box_t;
>
> extern gl_box_t create_box (const void *);
> extern const void *box_get_value (gl_box_t);
>
> #include "cast.h"
>
> /* CAST_TO_FROM (TO, FROM, EXPR)
>gives a warning if EXPR is not of type FROM,
>and returns EXPR, cast to type TO.  */
>
> struct foo;
> #define ELEMENT_TYPE struct foo *
> #define CREATE_BOX(V) \
>   create_box (CAST_TO_FROM (const void *, ELEMENT_TYPE, V))
> #define BOX_GET_VALUE(BOX) \
>   CAST_TO_FROM (ELEMENT_TYPE, const void *, box_get_value (BOX))
>
> struct foo *a;
> struct foo *b;
> const void *c;
> void *d;
> int *i;
>
> int
> main ()
> {
>   (void) CAST_TO_FROM (int *, const void *, c); // no warning
>   (void) CAST_TO_FROM (int *, void *, d);   // no warning
>   (void) CAST_TO_FROM (int *, const void *, d); // no warning
>   (void) CAST_TO_FROM (int *, void *, c);   // warning
>   (void) CAST_TO_FROM (int *, void *, i);   // warning
>   (void) CAST_TO_FROM (int *, char *, i);   // warning
>
>   gl_box_t box = CREATE_BOX (a);// no warning
>   return BOX_GET_VALUE (box) == b;  // no warning
> }
> 

I spent some time experimenting and I don't know a way to do that.

One way to come close, using GCC extensions:
  #define CAST_TO_FROM(to, from, expr) ({ \
_Static_assert (__builtin_types_compatible_p (typeof(expr), from)); \
(to) (expr);  \
  })
but this will give an unwanted warning for
  (void) CAST_TO_FROM (int *, const void *, d); // no warning
because the 'const' is not outermost.

Another way, without GCC extensions, is:
  #define CAST_TO_FROM(to, from, expr) ({ \
(void) sizeof ((from) (expr) == (expr));  \
(to) (expr);  \
  })
but this fails to warn for either of the following because void mixes with
other types so freely in C:
  (void) CAST_TO_FROM (int *, void *, c);   // warning
  (void) CAST_TO_FROM (int *, void *, i);   // warning



Re: Using relocatable-prog with relocatable libraries

2021-04-08 Thread Ben Pfaff
On Mon, Apr 5, 2021 at 3:43 PM Reuben Thomas  wrote:
>
> On Mon, 5 Apr 2021 at 23:36, Reuben Thomas  wrote:
>>
>>
>> The comment about "all the copies of relocatable.c" in progreloc.c is surely 
>> is clue, but I cannot see how more than one copy of relocatable.c is ever 
>> compiled…
>
>
> Finally found, in relocatable-maint.texi, "If you need more than one module, 
> or you need to use them with different settings, you will need multiple 
> copies of gnulib (@pxref{Multiple instances})." Sorry for the noise!

Do you think it should be better documented, or documented somewhere
else? (I'm only getting to my email now.)



Re: Type-safe typecasts

2021-04-06 Thread Ben Pfaff
On Tue, Apr 6, 2021 at 12:18 AM Marc Nieper-Wißkirchen
 wrote:
> I have been wondering whether it makes sense to add a small utility trying to 
> make typecasts in C as type-safe as possible.

I've found a few macros for casts useful over years. PSPP uses
CONST_CAST and UP_CAST from the file below quite a bit:
https://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/cast.h



Re: data structures for use in signal handlers

2021-02-09 Thread Ben Pfaff
On Tue, Feb 9, 2021 at 6:07 PM Bruno Haible  wrote:
> Maybe I didn't describe accurately the situation I am asking for.
> I'm not attempting to transform a program to an event-based engine.
> I'm looking to let signal handlers (SIGTERM, SIGSEGV, SIGWINCH, etc.)
> read and traverse data structures that the main program has prepared
> (and that contain information about files to delete, memory areas,
> window lines to redraw etc.).

This is a situation that I've always worked hard to avoid. So little can
be safely done in a signal handler that it's worthwhile to try not to do
more than the minimum possible. Have you already concluded that
you must do significant work in a signal handler? I would not want to
try to, say, redraw lines in a window in a signal handler, because the
functions that I would need to call to do that would not be prepared
to run inside such a handler: I don't think ncurses or GTK+, for
example, would be happy at all about it.



Re: data structures for use in signal handlers

2021-02-09 Thread Ben Pfaff
On Tue, Feb 9, 2021 at 5:01 PM Bruno Haible  wrote:
>
> Ben Pfaff wrote:
> > Building an RCU implementation isn't necessarily difficult (I have done it,
> > but the implementation isn't suitable for gnulib).
> >
> > There is a liburcu that is under LGPL v2.1: https://liburcu.org/
>
> Thanks for the pointers, Ben. Yes, RCU is the technique I had in mind.
> So: "can it actually be made to work?" -> Yes.
>
> But the implementation of liburcu uses extra signals and a helper thread of
> its own (or even a helper thread per CPU).
>
> I have to rephrase the question: can it be made to work in a straightforward
> (not necessarily scalability-optimized) way?

Signals are not needed. I haven't studied liburcu (it wasn't suitable for my
userspace use case because it only worked with GCC), so I am not sure
why it needs signals.

The userspace RCU implementation that I built in OVS (which is free but
Apache licensed) does use a thread, but not signals. The nice thing about
using a thread is that you always have something freeing memory that is
known to no longer be in use.

But I can imagine implementations that don't even use an extra thread.
For example, each thread could keep its own local list of callbacks to be
called after the next grace period. Then, whenever a thread quiesces, it
goes through its own list and executes all of the callbacks that now can
be. It could take a long time for some callbacks to be called, though,
especially if a thread goes to sleep and thus won't call any of its own
until it wakes up again.



Re: data structures for use in signal handlers

2021-02-07 Thread Ben Pfaff
On Sun, Feb 7, 2021 at 2:57 PM Bruno Haible  wrote:
> (2) Let the signal handler work only on immutable copies of the data
> structure. Whenever the other code manipulates the data structure,
> it creates an immutable copy of it, for the signal handler to use.
> Use an 'asyncsafe-spin' lock through which the signal handler tells
> the other threads to not free that immutable copy while it running.
>
> This is tricky; can it actually be made to work?
>
> Btw, there is an obvious requirement: the technique should use malloc/
> free for memory management, and should not have memory leaks.
> Algorithms that assume a garbage collected memory management are not
> suitable here.

This makes me think of read-copy-update aka RCU:
https://en.wikipedia.org/wiki/Read-copy-update
https://lwn.net/Articles/262464/

In RCU, code that updates the data structure takes a lock, creates and
modifies a copy, and then installs a new pointer to the data structure,
which is otherwise immutable. Readers always access the data structure
through a pointer. Whichever pointer they happen to see when they
read the pointer remains available until they're done with it.

Using RCU is pretty straightforward once you've done a little of it, but
it takes some reading to understand all of its concepts. It's probably best
for me not to try to explain it all, because I'll surely get some parts of it
wrong.

Building an RCU implementation isn't necessarily difficult (I have done it,
but the implementation isn't suitable for gnulib).

There is a liburcu that is under LGPL v2.1: https://liburcu.org/



Re: Bug report: bad check for ELF binary format

2020-11-29 Thread Ben Pfaff
On Sun, Nov 29, 2020 at 9:43 PM comex  wrote:
>
> From m4/lib-prefix.m4:
>
> --
>   AC_CACHE_CHECK([for ELF binary format], [gl_cv_elf],
> [AC_EGREP_CPP([Extensible Linking Format],
>[#ifdef __ELF__
> Extensible Linking Format
> #endif
>],
>[gl_cv_elf=yes],
>[gl_cv_elf=no])
>  ])
>   if test $gl_cv_elf; then
> —
>
> I believe this does not work as intended.  'test $gl_cv_elf' is equivalent to 
> 'test -n $gl_cv_elf', i.e. it tests whether the variable is nonempty.  Both 
> ‘yes’ and ‘no’ are nonempty, so the ‘if’ will always be taken.

Hmm, also ELF stands for "Executable and Linking Format", not
"Extensible Linking Format".



Re: [PATCH] reloc-ldflags: Fix handling of multiple relocatable library directories.

2020-09-20 Thread Ben Pfaff
On Sun, Sep 20, 2020 at 7:50 AM Bruno Haible  wrote:
> Thanks for the patch! You are right: since none of the platforms for which
> reloc-ldflags is being used is AIX, HP-UX, IRIX — these are the platforms
> for which libtool.m4 defines hardcode_libdir_separator to ':' —, the rpath
> options are cumulative.
>
> I applied your patch, revising comments and a variable name.

Thanks a lot!



Re: recent gnulib has problems with read, write and close within structs

2020-09-14 Thread Ben Pfaff
On Mon, Sep 14, 2020 at 01:54:14PM +0200, John Darrington wrote:
> On Sun, Sep 13, 2020 at 11:11:55PM +0200, Bruno Haible wrote:
>  
>  
>  The workaround is simple: Make sure that you include the corresponding 
> file -
>  here  - before the struct definition. That is, in this case,
>  at the beginning of ags/object/ags_application_context.h, add
>#include 
>  
> 
> I don't think this will solve all problems.
> 
> For example if you have:
> 
> 
> 
> #include 
> 
> struct foo
> {
> void write (char *);
> };
> 
> func (struct foo *f)
> {
>f->write ("hello");
> }
> 
> 
> 
> then one will get an error similar to "rpl_write is not a member of foo"

Why?  The following is a valid program:

#include 

struct foo
{
void rpl_write (char *);
};

func (struct foo *f)
{
   f->rpl_write ("hello");
}



[PATCH] reloc-ldflags: Fix handling of multiple relocatable library directories.

2020-09-13 Thread Ben Pfaff
Each one needs its own -Wl,-rpath,$dir option, instead of being attached
to a single one in some way.  Otherwise, the build fails at link time.
---
2020-09-13  Ben Pfaff  

* build-aux/reloc-ldflags: Fix handling of multiple relocatable
library directories.  Each one needs its own -Wl,-rpath,$dir
option, instead of being attached to a single one.
 
diff --git a/build-aux/reloc-ldflags b/build-aux/reloc-ldflags
index 145e741f8f..b1188a8707 100755
--- a/build-aux/reloc-ldflags
+++ b/build-aux/reloc-ldflags
@@ -90,7 +90,7 @@ if test -n "$origin_token"; then
 done
 dir="$origin_token"`echo "$idir" | sed -e 's,//*[^/]*,/..,g'`"$dir"
 # Add dir to rpath.
-rpath="${rpath}${rpath:+ }$dir"
+rpath="${rpath}${rpath:+ }-Wl,-rpath,$dir"
 ;;
   *)
 if test -n "$dir"; then
@@ -106,9 +106,9 @@ if test -n "$origin_token"; then
   # At least some versions of FreeBSD, DragonFly, and OpenBSD need the
   # linker option "-z origin". See <https://lekensteyn.nl/rpath.html>.
   freebsd* | dragonfly* | openbsd*)
-echo "-Wl,-z,origin -Wl,-rpath,$rpath" ;;
+echo "-Wl,-z,origin $rpath" ;;
   *)
-echo "-Wl,-rpath,$rpath" ;;
+echo "$rpath" ;;
 esac
   fi
 else
-- 
2.28.0




Re: [PATCH] getpass: Do not check for nonnull prompt argument in Win32 implementation.

2020-09-13 Thread Ben Pfaff
On Sun, Sep 13, 2020 at 09:05:32PM +0200, Bruno Haible wrote:
> > 2020-09-12  Ben Pfaff  
> > 
> > Check for nonnull prompt argument while avoiding warnings.
> > * lib/getpass.c (_GL_ARG_NONNULL): Define to empty.
> > (getpass) [!_WIN32]: Print prompt only if nonnull.
> 
> Looks good. Applied.

Thank you!



Re: [PATCH] getpass: Do not check for nonnull prompt argument in Win32 implementation.

2020-09-13 Thread Ben Pfaff
On Sun, Sep 13, 2020 at 10:32:27AM +0200, Bruno Haible wrote:
> > The prompt parameter to getpass() is declared as nonnull (using a GCC
> > nonnull attribute), but the implementation checks whether it is null in
> > two places.  GCC warns about this.  This commit removes the checks
> 
> GCC warnings ought to help us make the code more robust. Removing the
> NULL check makes it less robust.
> 
> The problem has already occurred a couple of times:
> https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00050.html
> https://lists.gnu.org/archive/html/bug-gnulib/2018-08/msg00116.html
> https://lists.gnu.org/archive/html/bug-gnulib/2013-02/msg00060.html
> https://lists.gnu.org/archive/html/bug-gnulib/2009-12/msg00173.html
> 
> I would prefer that the same idiom gets used, that gets rid of the
> warning without removing the NULL check at run time.

Thanks.

It is a little complicated because there are three implementations of
getpass(), and two of them do not check the prompt:

  * The glibc implementation passes the prompt as %s to fprintf().  I
guess that glibc will generally print "(null)", although I've seen
GCC "optimize" similar things to fputs() in the past, which will
dereference null.  I guess I won't worry about it.

  * The POSIX implementation in gnulib passes the prompt to fputs()
without checking for null.  I guess we should fix it.

I like the trick from one of your references about defining
_GL_ARG_NONNULL to empty.  That works OK here.

So, how about like this?

-8<--cut here-->8--

From: Ben Pfaff 
Date: Sat, 12 Sep 2020 15:54:36 -0700
Subject: [PATCH] getpass: Check for nonnull prompt argument while avoiding
 warnings.

The prompt parameter to getpass() is declared as nonnull (using a GCC
nonnull attribute).  Gnulib contains two implementations of this function,
one for POSIX, one for Windows.  The Windows implementation checked for
a nonnull prompt, which caused a GCC warning.  This commit fixes that by
avoiding the nonnull attribute when building getpass.c.  The POSIX
implementation did not check for a nonnull prompt.  This commit increases
the robustness by adding such a check.

2020-09-12  Ben Pfaff  

Check for nonnull prompt argument while avoiding warnings.
* lib/getpass.c (_GL_ARG_NONNULL): Define to empty.
(getpass) [!_WIN32]: Print prompt only if nonnull.

Use __builtin_signbit* with clang.
diff --git a/lib/getpass.c b/lib/getpass.c
index 3b0552ec58..ca528fdc09 100644
--- a/lib/getpass.c
+++ b/lib/getpass.c
@@ -16,6 +16,9 @@
with this program; if not, see <https://www.gnu.org/licenses/>.  */
 
 #ifndef _LIBC
+/* Don't use __attribute__ __nonnull__ in this compilation unit.  Otherwise gcc
+   warns for the null checks on 'prompt' below.  */
+# define _GL_ARG_NONNULL(params)
 # include 
 #endif
 
@@ -124,9 +127,12 @@ getpass (const char *prompt)
 }
 # endif
 
-  /* Write the prompt.  */
-  fputs_unlocked (prompt, out);
-  fflush_unlocked (out);
+  if (prompt)
+{
+  /* Write the prompt.  */
+  fputs_unlocked (prompt, out);
+  fflush_unlocked (out);
+}
 
   /* Read the password.  */
   nread = getline (, , in);
-- 
2.28.0




[PATCH] getpass: Do not check for nonnull prompt argument in Win32 implementation.

2020-09-12 Thread Ben Pfaff
The prompt parameter to getpass() is declared as nonnull (using a GCC
nonnull attribute), but the implementation checks whether it is null in
two places.  GCC warns about this.  This commit removes the checks, which
don't appear in the Gnulib POSIX implementation or the glibc
implementation.
---
I haven't pushed this yet; I'm posting it for review.

2020-09-12  Ben Pfaff  

Remove null checks for parameter declared as nonnull.
* lib/getpass.c (getpass) [_WIN32]: Don't check whether 'prompt'
argument is nonnull.

diff --git a/lib/getpass.c b/lib/getpass.c
index 3b0552ec58..f256bacdd8 100644
--- a/lib/getpass.c
+++ b/lib/getpass.c
@@ -194,11 +194,8 @@ getpass (const char *prompt)
   size_t i = 0;
   int c;
 
-  if (prompt)
-{
-  fputs (prompt, stderr);
-  fflush (stderr);
-}
+  fputs (prompt, stderr);
+  fflush (stderr);
 
   for (;;)
 {
@@ -220,11 +217,8 @@ getpass (const char *prompt)
 }
 }
 
-  if (prompt)
-{
-  fputs ("\r\n", stderr);
-  fflush (stderr);
-}
+  fputs ("\r\n", stderr);
+  fflush (stderr);
 
   return strdup (getpassbuf);
 }
-- 
2.28.0




Re: openat wrapper assumes that O_RDONLY is disjoint from R_RDWR

2020-03-07 Thread Ben Pfaff
On Sat, Mar 7, 2020 at 9:36 AM Bruno Haible  wrote:
>
> Dan Gohman wrote:
> > That would fix the problem for systems that define O_RDONLY as Hurd does,
> > while not breaking any known systems.
> >
> > It wouldn't be correct on a hypothetical system which defines the constants
> > like this:
> >
> > #define O_WRONLY 0
> > #define O_RDONLY 1
> > #define O_RDWR 2
> >
> > though I don't know of any platforms which do this.
>
> Reviewing the include files of the various platforms shows that
> such a hypothetical system doesn't exist:
>
>  O_RDONLY   O_WRONLY   O_RDWR
>
> AIX
> Android
> BeOS
> Cygwin
> FreeBSD
> glibc/Linux
> Haiku
> HP-UX
> Interix
> IRIX
> macOS
> mingw, MSVC
> Minix
> MirBSD
> musl
> NetBSD
> OpenBSD
> OSF/1
> pips
> Plan 9
> Solaris
>  0  1 2
>
> glibc/Hurd   1  2 3

Is there some reason that O_ACCMODE isn't the best choice here?



Re: immutable string type

2019-12-28 Thread Ben Pfaff
On Sat, Dec 28, 2019 at 3:17 AM Bruno Haible  wrote:
> Would you find it useful to have an immutable string type in gnulib?

I like this idea! Actually the idea of having primitives for allocating and
filling data and then getting read-only access to it is a good one in
general.  I haven't worked with anything like this before, so perhaps the
real value of it won't be apparent without some experience.

This sort of thing won't work on systems with virtually indexed caches,
at least not without inserting explicit flushes.  I don't know whether
virtually indexed caches still exist in the wild.



Re: notation for documenting multithread-safety

2019-11-26 Thread Ben Pfaff
On Tue, Nov 26, 2019 at 9:59 AM Bruno Haible  wrote:
> Is anyone aware of a notation that allows to specify, unambiguously, under
> which calls to a C function are multithread-safe?
>
> I would like to start documenting the multithread-safety of the functions in
> gnulib and other libraries (libunistring, libgettextpo, ...).

The only thing I know of that is close to this is the thread-safety annotations
that Clang supports, in which one can mark a function as requiring a particular
mutex to be taken, or that a particular mutex must not be taken, or that the
function acquires the mutex and then returns holding it, and various other
helpful things. The compiler analyzes the code and reports violations of the
annotations where possible.

Details:
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html

The Clang docs mostly talk about C++ but it also supports C.

The "sparse" code analyzer has something a little like this but it is weaker.

This only covers one of your cases, however.



Re: critique of gnulib - string allocation

2019-09-08 Thread Ben Pfaff
On Sun, Sep 8, 2019 at 10:08 AM Bruno Haible  wrote:
> So, what we would need is are functions
>
>char * substring (const char *string, size_t length);
>char * concatenated_string2 (const char *string1, size_t length1,
> const char *string2, size_t length2);
>char * concatenated_string3 (const char *string1, size_t length1,
> const char *string2, size_t length2,
> const char *string3, size_t length3);
>...
>
> where the length arguments are set to SIZE_MAX to designate the entire
> string.

I think that substring() is the same as xstrndup().



Re: [PATCH] version-etc.c: Do not include URLS in translatable strings.

2019-05-06 Thread Ben Pfaff
On Mon, May 06, 2019 at 08:21:34PM +0200, John Darrington wrote:
> This method is better for several reasons:  1. It avoids the danger of
> cut and paste errors.  2. Naive translators can't translate the urls
> (it's amazing how many do things like that!) 3. Should the urls ever
> change, only this file has to be updated rather than every single
> translation.
> 
> * lib/version-etc.c: Take URLS out of translatable strings.
> ---
>  lib/version-etc.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/version-etc.c b/lib/version-etc.c
> index 9ca9a5628..4a060e0b5 100644
> --- a/lib/version-etc.c
> +++ b/lib/version-etc.c
> @@ -82,14 +82,13 @@ version_etc_arn (FILE *stream,
>   locale.  Otherwise, do not translate "(C)"; leave it as-is.  */
>fprintf (stream, version_etc_copyright, _("(C)"), COPYRIGHT_YEAR);
>  
> -  fputs (_("\
> +  fprintf (stream, _("\
>  \n\
> -License GPLv3+: GNU GPL version 3 or later 
> .\n\
> +License GPLv3+: GNU GPL version 3 or later <%s>.\n\

Does anyone ever try to translate the "GPLv3+" part?  If so, then it
could be taken out as well.



Re: relocatable: avoid compiler warnings (-Wshadow)

2019-01-23 Thread Ben Pfaff
On Wed, Jan 23, 2019 at 10:33:20PM +0100, Akim Demaille wrote:
> Hi,
> 
> This is to avoid warnings such as:
> 
> bison/lib/relocatable.c: In function 'compute_curr_prefix':
> bison/lib/relocatable.c:285:14: warning: declaration of 'curr_prefix_len' 
> shadows a global declaration [-Wshadow]
>size_t curr_prefix_len = cp - curr_installdir;
>   ^~~
> bison/lib/relocatable.c:114:15: note: shadowed declaration is here
>  static size_t curr_prefix_len;
>^~~
> bison/lib/relocatable.c:286:13: warning: declaration of 'curr_prefix' shadows 
> a global declaration [-Wshadow]
>char *curr_prefix;
>  ^~~
> bison/lib/relocatable.c:113:14: note: shadowed declaration is here
>  static char *curr_prefix;
>   ^~~

Thanks for looking after this code.



Re: relocatable-prog: avoid warnings from Automake

2019-01-23 Thread Ben Pfaff
On Wed, Jan 23, 2019 at 10:48:23PM +0100, Akim Demaille wrote:
> Hi Ben,
> 
> This is what I get currently:
> 
>  cd bison && /bin/sh build-aux/missing automake-1.16 --gnu Makefile
> lib/gnulib.mk:1677: warning: .PHONY was already defined in condition TRUE, 
> which includes condition RELOCATABLE_VIA_WRAPPER ...
> Makefile.am:71:   'lib/local.mk' included from here
> lib/local.mk:16:   'lib/gnulib.mk' included from here
> doc/local.mk:168: ... '.PHONY' previously defined here
> Makefile.am:68:   'doc/local.mk' included from here
> 
> this is because relocatable-prog's Makefile snippet uses .PHONY in a 
> conditional.  AFAICT the only difference is that when not enabled, "make 
> uninstall-relocwrapper" will do nothing instead of being an error.  The 
> rigorous alternative would be to define a variable in the conditional and use 
> it in .PHONY.  Would you prefer that?

I think your solution is fine.  Thank you.



Re: new module suggestion: fprintftime-check

2018-12-29 Thread Ben Pfaff
On Sat, Dec 29, 2018 at 09:22:17AM -0800, Paul Eggert wrote:
> (Using ptrdiff_t is part of my campaign to prefer ptrdiff_t to size_t. While
> we're at it, let's change the other size_t args to ptrdiff_t, but I
> digress)

Have you said anything about this campaign elsewhere?  I'd like to hear
more.



Re: [RFC] Adding a real HashTable implementation to gnulib

2018-11-26 Thread Ben Pfaff
Much as I like the PSPP hmaps, it probably makes sense for any hash
table implementation in gnulib to match the existing code.

On Tue, Nov 27, 2018 at 02:02:17AM +0100, Darshit Shah wrote:
> Here are the links to the sources in the GNU Wget tree:
> 
> http://git.savannah.gnu.org/cgit/wget.git/tree/src/hash.h
> http://git.savannah.gnu.org/cgit/wget.git/tree/src/hash.c
> 
> At first sight, the implementation in PSPP looks a lot more concise.
> Also, it's usage of fewer preprocessor statements makes me already like it.
> 
> However, it does seem that this implementation is not as general or, complete
> as the existing hash table implementation in gnulib. The version available in
> GNU Wget does implement all the same interfaces with very similar usage
> characteristics.
> 
> One of the things I noticed missing in the implementation in GNU PSPP is the
> ability to grow the size of the table based on a certain threshold of
> "fullness".
> 
> * Ben Pfaff  [181127 00:41]:
> > On Tue, Nov 27, 2018 at 12:16:16AM +0100, Darshit Shah wrote:
> > > I recently tried to use the hash table implementation in gnulib which 
> > > resides
> > > in the "hash" module. However, I quickly realised that the hash table in 
> > > gnulib
> > > seems to be what is otherwise popularly known as a hash set, i.e., it 
> > > supports
> > > storing and retrieving just values from the structure. 
> > > 
> > > On the other hand, a hash table is usually expected to have a key->value
> > > mapping that is stored.
> > > 
> > > Within GNU Wget, we have a fairly portable version of a hash table 
> > > implemented
> > > which I think would be a good addition for gnulib. What do you think?
> > > 
> > > If I get a positive response here, I could extract that code and turn it 
> > > into a
> > > hash table module for gnulib. We should be able to reuse some part of the
> > > existing code in "hash.c" for this purpose as well
> > 
> > Can you point to the Wget hash table?
> > 
> > I'm pretty fond of the hash table implementation we have in PSPP:
> > http://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/hmap.h
> > http://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/hmap.c
> > 
> > 
> 
> -- 
> Thanking You,
> Darshit Shah
> PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6





Re: [RFC] Adding a real HashTable implementation to gnulib

2018-11-26 Thread Ben Pfaff
On Tue, Nov 27, 2018 at 12:16:16AM +0100, Darshit Shah wrote:
> I recently tried to use the hash table implementation in gnulib which resides
> in the "hash" module. However, I quickly realised that the hash table in 
> gnulib
> seems to be what is otherwise popularly known as a hash set, i.e., it supports
> storing and retrieving just values from the structure. 
> 
> On the other hand, a hash table is usually expected to have a key->value
> mapping that is stored.
> 
> Within GNU Wget, we have a fairly portable version of a hash table implemented
> which I think would be a good addition for gnulib. What do you think?
> 
> If I get a positive response here, I could extract that code and turn it into 
> a
> hash table module for gnulib. We should be able to reuse some part of the
> existing code in "hash.c" for this purpose as well

Can you point to the Wget hash table?

I'm pretty fond of the hash table implementation we have in PSPP:
http://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/hmap.h
http://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/hmap.c



Re: Compile error on Slackware-current

2018-11-01 Thread Ben Pfaff
On Fri, Nov 02, 2018 at 01:46:39AM +0100, Bruno Haible wrote:
> Hi,
> 
> Frans Houweling wrote:
> >   I am getting this error building gnu pspp 1.0.1 on Slackware-current  
> > (gcc-8.2.0-x86_64-1)
> > 
> > fseterr.c:78:3: error: #error "Please port gnulib fseterr.c to your 
> > platform! Look at the definitions of ferror and clearerr on your system, 
> > then report this to bug-gnulib."
> 
> This was fixed in gnulib in March 2018. No release of pspp with these
> changes included apparently exists yet. In the meantime, you can build
> pspp from the Git repository [1]:
> 
> $ git clone https://git.savannah.gnu.org/git/pspp.git
> $ git clone git://git.savannah.gnu.org/gnulib.git gnulib
> $ cd pspp
> $ make -f Smake
> 
> [1] https://savannah.gnu.org/projects/pspp

We're talking about doing a release soon, which should fix the problem.



Re: write past end of buffer in vasnprintf() implementation of %f

2018-09-23 Thread Ben Pfaff
On Sun, Sep 23, 2018 at 02:25:50PM +0200, Bruno Haible wrote:
> > The line in convert_to_decimal() cited above is the assignment here:
> > 
> >   /* Terminate the string.  */
> >   *d_ptr = '\0';
> > 
> > I guess that the space calculation passed to malloc() at the top of the
> > same function is not precise.  I don't know whether the right thing to
> > do is to just add one.
> 
> Indeed, the right thing is to add just 1.
> 
> > This bug was originally reported against GNU PSPP:
> > https://savannah.gnu.org/bugs/?func=detailitem_id=54686
> > 
> > For this report, I've simplified it to remove the PSPP dependency (and
> > to make sure it isn't somehow a PSPP bug).
> 
> I found a smaller test case: 1.6314159265358979e+125 instead of
> 1.24726002000241678234e+269, and added that to the test suite.
> For the record, the issue occurs for all numbers in the ranges
>   10^125 <= arg < 2^416
>   10^134 <= arg < 2^448
>   10^260 <= arg < 2^864
>   10^269 <= arg < 2^896
>   ...

Thank you very much for the fix!



write past end of buffer in vasnprintf() implementation of %f

2018-09-22 Thread Ben Pfaff
When I apply the following patch to gnulib:

--
diff --git a/tests/test-vasnprintf.c b/tests/test-vasnprintf.c
index 19731bc93378..105ac24c94a3 100644
--- a/tests/test-vasnprintf.c
+++ b/tests/test-vasnprintf.c
@@ -59,6 +59,11 @@ test_function (char * (*my_asnprintf) (char *, size_t *, 
const char *, ...))
   if (result != buf)
 free (result);
 }
+
+  size_t length = 8;
+  char *result = my_asnprintf (buf, , "%2.0f", 0x1.e38417c792296p+893);
+  if (result != buf)
+free (result);
 }
 
 static char *
--

and then run:

CC='gcc -fsanitize=address -g -O0' ./gnulib-tool --test vasnprintf 
vasnprintf-posix

I get the following failure from test-vasnprintf:

--
==17880==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf4b03ece at 
pc 0xf770f8ed bp 0xffac9638 sp 0xffac962c
WRITE of size 1 at 0xf4b03ece thread T0
#0 0xf770f8ec in convert_to_decimal ../../gllib/vasnprintf.c:899
#1 0xf770f8ec in scale10_round_decimal_decoded ../../gllib/vasnprintf.c:1292
#2 0xf771057c in scale10_round_decimal_double ../../gllib/vasnprintf.c:1328
#3 0xf771755c in vasnprintf ../../gllib/vasnprintf.c:4119
#4 0xf770c692 in my_asnprintf ../../gltests/test-vasnprintf.c:76
#5 0xf770ca39 in test_function ../../gltests/test-vasnprintf.c:64
#6 0xf770c384 in test_vasnprintf ../../gltests/test-vasnprintf.c:84
#7 0xf770c384 in main ../../gltests/test-vasnprintf.c:96
#8 0xf6f2be80 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18e80)
#9 0xf770c4de  
(/home/blp/pspp/gnulib2/testdir21347/build/gltests/test-vasnprintf+0x14de)

0xf4b03ece is located 0 bytes to the right of 270-byte region 
[0xf4b03dc0,0xf4b03ece)
allocated by thread T0 here:
#0 0xf71cc334 in malloc (/usr/lib/i386-linux-gnu/libasan.so.4+0xe0334)
#1 0xf770f223 in convert_to_decimal ../../gllib/vasnprintf.c:863
#2 0xf770f223 in scale10_round_decimal_decoded ../../gllib/vasnprintf.c:1292

SUMMARY: AddressSanitizer: heap-buffer-overflow ../../gllib/vasnprintf.c:899 in 
convert_to_decimal
Shadow bytes around the buggy address:
  0x3e960780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e960790: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e9607a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e9607b0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x3e9607c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x3e9607d0: 00 00 00 00 00 00 00 00 00[06]fa fa fa fa fa fa
  0x3e9607e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e9607f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e960800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e960810: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e960820: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:   00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:   fa
  Freed heap region:   fd
  Stack left redzone:  f1
  Stack mid redzone:   f2
  Stack right redzone: f3
  Stack after return:  f5
  Stack use after scope:   f8
  Global redzone:  f9
  Global init order:   f6
  Poisoned by user:f7
  Container overflow:  fc
  Array cookie:ac
  Intra object redzone:bb
  ASan internal:   fe
  Left alloca redzone: ca
  Right alloca redzone:cb
==17880==ABORTING
FAIL test-vasnprintf (exit status: 1)
--

The line in convert_to_decimal() cited above is the assignment here:

  /* Terminate the string.  */
  *d_ptr = '\0';

I guess that the space calculation passed to malloc() at the top of the
same function is not precise.  I don't know whether the right thing to
do is to just add one.

This bug was originally reported against GNU PSPP:
https://savannah.gnu.org/bugs/?func=detailitem_id=54686

For this report, I've simplified it to remove the PSPP dependency (and
to make sure it isn't somehow a PSPP bug).

I'd appreciate it if someone more familiar with this part of the
vasnprintf() code, which seems rather opaque to a newcomer, would
investigate this.  Bruno, you're the main committer to this file, so I'm
guessing that's you, hence the CC.

Thanks a lot!



Re: _GL_INLINE issue

2018-04-13 Thread Ben Pfaff
On Fri, Apr 13, 2018 at 04:03:45PM +0200, Bruno Haible wrote:
> By the way, Paul, what is the problem with telling the compiler what to
> 'static inline'? In other words, what was the motivation of changing
> 'static inline' to 'static' - other than "it's not needed" - in the
> 13 patches presented on November 09, 2012 in
> https://lists.gnu.org/archive/html/bug-gnulib/2012-11/ and committed on
> 2012-11-29 ?

For functions defined in .c files, I like to avoid "static inline"
because "inline" prevents the compiler from warning me when a function
is unused, which I often find to be a useful warning.



Re: z/OS enum size pitfall

2017-08-22 Thread Ben Pfaff
On Tue, Aug 22, 2017 at 10:09:22PM -0400, Daniel Richard G. wrote:
> On Tue, 2017 Aug 22 15:01-0700, Ben Pfaff wrote:
> > 
> > I don't know what XLC conforms to.
> >
> > C11 has the same text in 6.7.2.2p4.  The specification for enums has
> > not changed significantly since C89.
> >
> > Paul Eggert already explained the distinction between enumeration
> > constants and enumeration types, so I won't repeat it.
> 
> All the commentary here is greatly appreciated; please excuse my delay
> in replying.
> 
> Here is what IBM said relating to standards:
> 
> The compiler behavior is correct. The C standard says that the
> enumerated type can be char, or any signed or unsigned integer type
> (6.7.2.2#4).
> 
> The test case uses a default enumsize which is small and for the
> range of values in the test case it reserves an unsigned int to hold
> the enumeration values (this is documented in the User's Guide).
> 
> The conversion rules make the signed into unsigned int (6.3.1.8#1)
> meaning that expression becomes (unsigned int)x/BILLION because
> BILLION is an unsigned int;
> 
> The conversion of -9 produces 3294967297 (UINT_MAX +1
> -9) which when divided by 10 produces 3 as the
> result.
> 
> There was no further qualification of "C standard" in the first
> paragraph. However, the test program still returns 3 when XLC is
> in C99 mode:
> 
> $ xlc -qlanglvl=stdc99 -o enumsize enumsize.c
> $ ./enumsize 
> BILLION = 10
> x = -9
> x / BILLION = 3
> 
> Now, from what I'm hearing here, it sounds like IBM may be in the wrong---
> and this would be quite convenient, because while they may not be too
> bothered when their interpretation of POSIX et al. is different from the
> rest of the world's, they _will_ care about a red-letter violation of a
> spec they claim to support.
> 
> I can't standards-lawyer my way out of a paper bag, but if someone here
> could provide a line of argument that IBM's enum shenanigans are
> categorically unsupported by the standard, I'll gladly send it on in the
> hope that it'll get the beast to move.

The C99 rationale is even clearer than the text, in section 6.4.4.3
"Enumeration constants":

Whereas an enumeration variable may have any integer type that
correctly represents all its values when widened to int, an
enumeration constant is only usable as the value of an
expression. Hence its type is simply int.



Re: z/OS enum size pitfall

2017-08-22 Thread Ben Pfaff
On Tue, Aug 22, 2017 at 10:43:07PM +0200, Tim Rühsen wrote:
> On Dienstag, 22. August 2017 13:25:55 CEST Ben Pfaff wrote:
> > On Tue, Aug 22, 2017 at 04:13:54PM -0400, Daniel Richard G. wrote:
> > > What happens is that BILLION is implicitly typed as an unsigned int,
> > > rather than an int. If you edit the code above to use BILLION2 instead
> > > of BILLION, you'll see the same result on GNU/Linux.
> > 
> > It's odd that they claim that this conforms to the C standard.  C11
> > says, in section 6.4.4.3 "Enumeration constants":
> > 
> > An identifier declared as an enumeration constant has type int.
> > 
> > It also says in section 6.7.2.2 "Enumeration specifiers":
> > 
> > The identifiers in an enumerator list are declared as constants that
> > have type int and may appear wherever such are permitted.
> > 
> > This seems pretty clear to me, so I wonder how this interpretation
> > arises.
> 
> Do you know to which C standard the XLC compiler complies to ?
> 
> C99, 6.7.2.2p4 says
> 
> Each enumerated type shall be compatible with char, a signed integer 
> type, 
> or an unsigned integer type. The choice of type is 
> implementation-defined,108) 
> but shall be capable of representing the values of all the members of the 
> enumeration.

I don't know what XLC conforms to.

C11 has the same text in 6.7.2.2p4.  The specification for enums has not
changed significantly since C89.

Paul Eggert already explained the distinction between enumeration
constants and enumeration types, so I won't repeat it.



Re: z/OS enum size pitfall

2017-08-22 Thread Ben Pfaff
On Tue, Aug 22, 2017 at 04:13:54PM -0400, Daniel Richard G. wrote:
> Hello list,
> 
> I'm writing in to report a bizarre issue with the IBM z/OS XLC compiler
> that is currently causing one gnulib test to fail (test-timespec), and
> may present an issue for application code simply because no other
> compiler does things this way. My hope is to have gnulib integrate a
> workaround so that this won't bite anyone else.
> 
> I have been in contact with IBM about this, originally reporting the
> issue as a compiler bug. However, they responded that the compiler
> behavior is conformant to the C standard and that they are less
> concerned with matching the behavior of other systems than keeping
> things as-is for the benefit of existing customer application code.
> 
> The problem has to do with the implicit integer type that is used for
> enum symbols. Here is a sample program that illustrates the issue:
> 
> 8<
> #include 
> 
> enum { BILLION = 10 };
> 
> static const unsigned int BILLION2 = 10;
> 
> int main(void) {
> int x = -9;
> printf("BILLION = %d\n", (int)BILLION);
> printf("x = %d\n", x);
> printf("x / BILLION = %d\n", (int)(x / BILLION));
> return 0;
> }
> >8
> 
> On GNU/Linux and AIX, with a minimal compiler invocation, this
> program prints
> 
> BILLION = 10
> x = -9
> x / BILLION = 0
> 
> However, on z/OS, it prints
> 
> BILLION = 10
> x = -9
> x / BILLION = 3
> 
> What happens is that BILLION is implicitly typed as an unsigned int,
> rather than an int. If you edit the code above to use BILLION2 instead
> of BILLION, you'll see the same result on GNU/Linux.

It's odd that they claim that this conforms to the C standard.  C11
says, in section 6.4.4.3 "Enumeration constants":

An identifier declared as an enumeration constant has type int.

It also says in section 6.7.2.2 "Enumeration specifiers":

The identifiers in an enumerator list are declared as constants that
have type int and may appear wherever such are permitted.

This seems pretty clear to me, so I wonder how this interpretation
arises.



Re: plans for file related modules on Windows

2017-05-14 Thread Ben Pfaff
On Sun, May 14, 2017 at 06:06:03PM +0200, Bruno Haible wrote:
> > I did not realize that Windows could even support a proper
> > implementation of the struct stat st_dev and st_ino.  I'd find this
> > useful in multiple programs, although in some of them I might really
> > just use the code you write as an educational resource.
> 
> I've considered your wish. This functionality is now implemented as
> module 'windows-stat-inodes'.

Thank you!  I learned a lot of useful information from reading the
module, and in addition I do expect to use the module directly.



Re: plans for file related modules on Windows

2017-05-09 Thread Ben Pfaff
On Tue, May 09, 2017 at 10:34:04PM +0200, Bruno Haible wrote:
> It's clear that different gnulib users need different levels of native Windows
> support. Some will want to avoid 'struct stat', some will want to use the 
> ino_t
> values in struct stat.
> 
> Here's my current plan: Introduce a set of orthogonal, transversal modules.
> ("transversal" in the sense that such a module does not provide a function,
> but rather provides guarantees for other modules.)
> 
> - windows-year2038 : define time_t as 64-bit (might involve renaming module 
> time to time-h)
> - windows-symlinks : add symlink support to stat, lstat, readlink etc.
> - windows-stat-timespec : add 100ns resolution to file times
> - windows-stat-inodes : redefine dev_t, ino_t
> - windows-uid : redefine uid_t [1]
> - windows-gid : redefine gid_t [1]
> - windows-utf8-filenames : implies override of all file-related functions
> - largefile : support for files > 2 GB (already partially implemented in 2012)
> 
> An override of 'struct stat' will be necessary for windows-year2038,
> windows-stat-timespec, windows-stat-inodes, windows-uid, windows-gid, 
> largefile.
> 
> What do you think about it?
> 
> Which of these modules would you like to see implemented first?

I did not realize that Windows could even support a proper
implementation of the struct stat st_dev and st_ino.  I'd find this
useful in multiple programs, although in some of them I might really
just use the code you write as an educational resource.



Re: Documentation update for relocatable-lib{,-lgpl}

2017-04-04 Thread Ben Pfaff
On Tue, Apr 04, 2017 at 01:07:58AM +0100, Reuben Thomas wrote:
> [Ben: Ccing you on Bruno Haible's advice]
> 
> Having gone through the changes I needed to make to my sources to use the
> relocatable-lib-lgpl module, here's a list of things I think should be
> documented in doc/relocatable-maint.texi. If they're agreed to be correct,
> I'll prepare a documentation patch.

I think that Bruno is really the expert here.  I helped out with
"relocatable" to a significant degree, but I don't have any expertise
with libraries.  If Bruno approves, then I second it.



Re: Question about portability guidelines

2017-01-04 Thread Ben Pfaff
On Tue, Jan 03, 2017 at 04:33:24PM -0800, Paul Eggert wrote:
> (An aside: It's typically safer in C to assign to a typed temporary than
> to cast to the type, as casts are too powerful. This is orthogonal to
> the long-vs-ptrdiff_t issue.)

Yes.

Sometimes, to make casts safer, I declare macros to help out, like
CONST_CAST below.

For safer "casting" to intmax_t, I guess that one could just write a
function:

intmax_t to_intmax(intmax_t x) { return x; }

/* Expands to a void expression that checks that POINTER is an
   expression whose type is a qualified or unqualified version of
   a type compatible with TYPE (a pointer type) and, if not,
   causes a compiler warning to be issued (on typical compilers).

   Examples:

   int *ip;
   const int *cip;
   const int **cipp;
   int ***ippp;
   double *dp;

   // None of these causes a warning:
   CHECK_POINTER_HAS_TYPE (ip, int *);
   CHECK_POINTER_HAS_TYPE (ip, const int *);
   CHECK_POINTER_HAS_TYPE (cip, int *);
   CHECK_POINTER_HAS_TYPE (cip, const int *);
   CHECK_POINTER_HAS_TYPE (dp, double *);
   CHECK_POINTER_HAS_TYPE (dp, const double *);
   CHECK_POINTER_HAS_TYPE (cipp, const int **);
   CHECK_POINTER_HAS_TYPE (cipp, const int *const *);
   CHECK_POINTER_HAS_TYPE (ippp, int ***);
   CHECK_POINTER_HAS_TYPE (ippp, int **const *);

   // None of these causes a warning either, although it is unusual to
   // const-qualify a pointer like this (it's like declaring a "const int",
   // for example).
   CHECK_POINTER_HAS_TYPE (ip, int *const);
   CHECK_POINTER_HAS_TYPE (ip, const int *const);
   CHECK_POINTER_HAS_TYPE (cip, int *const);
   CHECK_POINTER_HAS_TYPE (cip, const int *const);
   CHECK_POINTER_HAS_TYPE (cipp, const int **const);
   CHECK_POINTER_HAS_TYPE (cipp, const int *const *const);
   CHECK_POINTER_HAS_TYPE (ippp, int ***const);
   CHECK_POINTER_HAS_TYPE (ippp, int **const *const);

   // Provokes a warning because "int" is not compatible with "double":
   CHECK_POINTER_HAS_TYPE (dp, int *);

   // Provoke warnings because C's type compatibility rules only allow
   // adding a "const" qualifier to the outermost pointer:
   CHECK_POINTER_HAS_TYPE (ippp, const int ***);
   CHECK_POINTER_HAS_TYPE (ippp, int *const**);
*/
#define CHECK_POINTER_HAS_TYPE(POINTER, TYPE)   \
((void) sizeof ((TYPE) (POINTER) == (POINTER)))

/* Given expressions A and B, both of which have pointer type,
   expands to a void expression that causes a compiler warning if
   A and B are not pointers to qualified or unqualified versions
   of compatible types.

   Examples similar to those given for CHECK_POINTER_HAS_TYPE,
   above, can easily be devised. */
#define CHECK_POINTER_COMPATIBILITY(A, B) ((void) sizeof ((A) == (B)))

/* Equivalent to casting POINTER to TYPE, but also issues a
   warning if the cast changes anything other than an outermost
   "const" or "volatile" qualifier. */
#define CONST_CAST(TYPE, POINTER)   \
(CHECK_POINTER_HAS_TYPE (POINTER, TYPE),\
 (TYPE) (POINTER))



Re: Question about portability guidelines

2017-01-03 Thread Ben Pfaff
On Mon, Jan 02, 2017 at 04:09:59PM -0800, Paul Eggert wrote:
> Bruno Haible wrote:
> >I would vote for removing this sentence "Gnulib code can assume that standard
> >internal types like size_t are no wider than long." The fix is easy nowadays
> >(since MSVC now has ): Use intptr_t or ptrdiff_t or size_t as
> >appropriate.
> 
> It's not that easy for code that wants to print such integers, as %ld
> doesn't suffice on MinGW and older POSIXish libraries don't support %jd etc.
> Perhaps once Gnulib assumes C99-or-later libraries, and once somebody goes
> through all the Gnulib code and checks it. In the meantime we should
> probably leave something like that sentence in there, although we should
> warn people about the MSVC issues. I installed the attached to try to do
> that, and to try to clarify the issues about C89 vs C99 vs C11 that were in
> Paul's question.

One strategy is to use PRIdPTR for ptrdiff_t and PRIdMAX or PRIuMAX
(plus a cast) for other types.

In one project of mine (which does not use Gnulib) I introduced a
PRIuSIZE macro.  This is not a standard macro, but I don't understand
why not.



Re: install-sh and $RANDOM

2016-10-17 Thread Ben Pfaff
On Mon, Oct 17, 2016 at 04:56:05PM -0500, Eric Blake wrote:
> You're not the first person to complain that $RANDOM is a bashism, and
> this is not the first time we've had to retort that our use of $RANDOM
> is a nicety, but not a necessity, and that the code is perfectly safe
> and tested on shells where the expansion of $RANDOM is the empty string.

I can see why it would surprise people, so maybe it would be helpful to
add a comment mentioning that the code is still safe when $RANDOM is not
useful, and perhaps pointing to this discussion.



Re: [PATCH] IBM z/OS + EBCDIC support

2015-09-26 Thread Ben Pfaff
On Sat, Sep 26, 2015 at 12:39:52AM -0400, Daniel Richard G. wrote:
> I'm happy to report that test-c-ctype in Git ff1ef114 now passes with
> both signed and unsigned EBCDIC chars on z/OS. Thank you for chasing
> this down!

A "char" configured as signed in EBCDIC violates the ANSI C standard,
which says:

 If a member of the basic execution character set is stored in a
 char object, its value is guaranteed to be positive.

whereas the "basic execution character set" is defined as:

 Both the basic source and basic execution character sets shall have
 the following members: the 26 uppercase letters of the Latin
 alphabet

 A B C D E F G H I J K L M
 N O P Q R S T U V W X Y Z
 the 26 lowercase letters of the Latin alphabet
 a b c d e f g h i j k l m
 n o p q r s t u v w x y z
 the 10 decimal digits
 0 1 2 3 4 5 6 7 8 9
 the following 29 graphic characters
 ! " # % & ' ( ) * + , - . / :
 ; < = > ? [ \ ] ^ _ { | } ~
 the space character, and control characters representing horizontal
 tab, vertical tab, and form feed.

Do people actually used signed "char" with EBCDIC?



Re: Question on AC_CHECK_HEADERS/AC_DEFINE interaction

2015-07-24 Thread Ben Pfaff
On Fri, Jul 24, 2015 at 09:29:45AM -0700, Jim Meyering wrote:
 On Thu, Jul 23, 2015 at 8:29 AM, Pádraig Brady p...@draigbrady.com wrote:
  At line 40 we AC_DEFINE(HAVE_SELINUX_SELINUX_H,0)

  http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=m4/selinux-selinux-h.m4;h=fd09b299;hb=HEAD#l40
  However that may already defined to 1 which is the case
  when compiling in -m32 mode but with only the first of these installed:
 
$ rpm -qf /usr/include/selinux/selinux.h
libselinux-devel-2.3-9.fc22.x86_64
libselinux-devel-2.3-9.fc22.i686
 
  In that edge case, -Werror will fail due to the redefinition,
  thus causing -Werror to not be used in the build.
 
  Quite the edge case I know, but I'm not sure how best to avoid.
  Perhaps we should not use AC_CHECK_HEADERS here,
  instead going lower level with AC_PREPROC_IFELSE(.. AC_DEFINE ..) ?
 
 If you can find a clean way to avoid it, go ahead.
 However, isn't it easy just to refrain from using -Werror when running
 ./configure?

I agree.

If one wants to develop with -Werror, though, it can be a little
inconvenient to add -Werror after running configure.  For my own
projects, I add a --enable-Werror configure option that automatically
adds -Werror only after doing all of the configuration tests:

AC_DEFUN([PSPP_ENABLE_WERROR],
  [AC_ARG_ENABLE(
 [Werror],
 [AC_HELP_STRING([--enable-Werror], [Add -Werror to CFLAGS])],
 [], [enable_Werror=no])
   AC_CONFIG_COMMANDS_PRE(
 [if test X$enable_Werror = Xyes; then
CFLAGS=$CFLAGS -Werror
  fi])])



Re: Request update of Target Platforms section

2015-02-01 Thread Ben Pfaff
On Fri, Jan 30, 2015 at 05:38:23PM -0800, Nathaniel Hayden wrote:
 According to the content itself (As of 2011 ...) the section was last
 updated in 2011, and commit logs seem to confirm this.
 
 I'm a developer in a bioinformatics group that uses the R statistical
 analysis language, also part of the GNU Project. I came across Gnulib while
 trying to port an important bioinformatics library (HTSlib) to Windows on
 the way to making it available in R. I've already had success with making
 the library work with mingw32. Thank you for your excellent work!
 
 But the R language toolchain for Windows is based on MinGW-w64. Currently
 the Target Platforms section reads
 - mingw in 64-bit mode is not tested and low priority so far.
 
 The logs contain many commits around MinGW-w64. Can you comment on support
 for MinGW-w64--and is it now considered a reasonable target, and hence the
 Target Platforms section can be updated? I would like to sell adoption of
 Gnulib to the maintainers of HTSlib, but the ultimate goal is to make it
 work with R's toolchain.

My understanding, based on recalling previous discussions on this list,
is that mingw64 (despite its name) supports Windows in both 32-bit and
64-bit modes, and that Gnulib supports Windows only in 32-bit mode.



Re: [PATCH 03/14] relocatable: support UNIXROOT in relocate() on EMX

2014-12-08 Thread Ben Pfaff
On Tue, Dec 09, 2014 at 10:40:48AM +0900, KO Myung-Hun wrote:
 UNIXROOT is used to specify a drive of a root of FHS. So if a path is
 started with '/', then it should be translated to $UNIXROOT/.
 
 * lib/relocatable.c (relocate): Prepend $UNIXROOT to pathname if it is
 started with '/' on EMX.

As one of the maintainers of the relocatable modules, the code in this
patch seems reasonable to me.  I don't know enough about OS/2 or EMX to
know whether $UNIXROOT is a customary variable or whether this is a
correct context for using it.  Can you assure me that you've thought
about those?



Re: [PATCH 03/14] relocatable: support UNIXROOT in relocate() on EMX

2014-12-08 Thread Ben Pfaff
On Tue, Dec 09, 2014 at 02:51:51PM +0900, KO Myung-Hun wrote:
 Ben Pfaff wrote:
  On Tue, Dec 09, 2014 at 10:40:48AM +0900, KO Myung-Hun wrote:
  UNIXROOT is used to specify a drive of a root of FHS. So if a path is
  started with '/', then it should be translated to $UNIXROOT/.
 
  * lib/relocatable.c (relocate): Prepend $UNIXROOT to pathname if it is
  started with '/' on EMX.
  
  As one of the maintainers of the relocatable modules, the code in this
  patch seems reasonable to me.  I don't know enough about OS/2 or EMX to
  know whether $UNIXROOT is a customary variable or whether this is a
  correct context for using it.  Can you assure me that you've thought
  about those?
 
 OS/2 file system is based on drives[A-Z]. '/' means a root of a current
 drive. There are maximum 26 kinds of '/'.
 
 For examples, consider that xxx is installed into f:/usr/bin. If
 executing xxx on drive e:, '/' of xxx is e:/ not f:/. In this case, xxx
 fails to find its root.
 
 $UNIXROOT is used to overcome this problem. On OS/2, $UNIXROOT is used
 to specify a drive of '/'. And all programs following FHS are installed
 on a drive specified by $UNIXROOT. In the above, if UNIXROOT is set to
 'f:'. Then '/' is translated to 'f:/' as xxx expects.
 
 And see http://trac.netlabs.org/libc/wiki/UnixPenthouseApartement.
 
 Still not assured ?

Thank you for explaining.

I applied this to the gnulib repository.

Thanks,

Ben.



Re: [PATCH] poll: undef NOGDI

2014-09-28 Thread Ben Pfaff
On Thu, Sep 18, 2014 at 04:53:37PM +0400, Marat Radchenko wrote:
 On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI.
 
 -DNOGDI=1 prevents inclusion of tons of Windows headers, especially
 wingdi.h with weird #define ERROR 0 that has high chances to conflict
 with internal software defines. So, just #undef NOGDI.
 
 MinGW-W64 bug: http://sourceforge.net/p/mingw-w64/bugs/397/
 When it is fixed, this commit can be reverted.

According to that URL, this bug has already been fixed, so it seems that
this is not worth applying at all?



Re: `base_name' and `dir_name' not documented

2014-09-28 Thread Ben Pfaff
On Sun, Sep 21, 2014 at 04:43:07PM +0200, Werner LEMBERG wrote:
 
 The functions `base_name' and `dir_name' (or rather `mdir_name'),
 intended as replacements for `basename' and `dirname' that work on
 Windows also, are not documented in `gnulib.pdf'.  I suggest to
 mention those two function names at least in `modules/dirname'.

I applied the following.  I hope that it helps.

--8--cut here--8--

From: Ben Pfaff b...@cs.stanford.edu
Date: Sun, 28 Sep 2014 09:52:08 -0700
Subject: [PATCH] basename, dirname: Improve documentation.

* doc/posix-functions/basename.texi: Mention dirname module and
base_name() function.
* doc/posix-functions/dirname.texi: Mention dir_name() and
mdir_name() functions.

Suggested by Werner LEMBERG w...@gnu.org.
---
 ChangeLog | 9 +
 doc/posix-functions/basename.texi | 3 +++
 doc/posix-functions/dirname.texi  | 5 +++--
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 876174f..0bf83d4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2014-09-28  Ben Pfaff  b...@cs.stanford.edu
+
+   basename, dirname: Improve documentation.
+   * doc/posix-functions/basename.texi: Mention dirname module and
+   base_name() function.
+   * doc/posix-functions/dirname.texi: Mention dir_name() and
+   mdir_name() functions.
+   Suggested by Werner LEMBERG w...@gnu.org.
+
 2014-09-24  Jim Meyering  meyer...@fb.com
 
exclude: declare exclude_patopts static
diff --git a/doc/posix-functions/basename.texi 
b/doc/posix-functions/basename.texi
index 699f28e..bb068f4 100644
--- a/doc/posix-functions/basename.texi
+++ b/doc/posix-functions/basename.texi
@@ -22,3 +22,6 @@ the GNU version.
 @code{basename} assumes file names in POSIX syntax; it does not work with file
 names in Windows syntax.
 @end itemize
+
+The Gnulib module @code{dirname} provides similar API, with function
+@code{base_name}, that also works with Windows file names.
diff --git a/doc/posix-functions/dirname.texi b/doc/posix-functions/dirname.texi
index 026f404..dd86e58 100644
--- a/doc/posix-functions/dirname.texi
+++ b/doc/posix-functions/dirname.texi
@@ -20,5 +20,6 @@ IRIX 6.5, Solaris 2.5.1, mingw, MSVC 9, BeOS.
 names in Windows syntax.
 @end itemize
 
-The Gnulib module @code{dirname} provides similar API that also works with
-Windows file names.
+The Gnulib module @code{dirname} provides similar API, with functions
+@code{dir_name} and @{code{mdir_name}, that also works with Windows
+file names.
-- 
2.1.0




Re: [bug-libunistring] [PATCH v4] unistr: New modules for backward iteration in string.

2014-09-26 Thread Ben Pfaff
On Fri, Sep 19, 2014 at 8:06 AM, Ben Pfaff b...@cs.stanford.edu wrote:
 On Fri, Sep 19, 2014 at 04:55:24PM +0900, Daiki Ueno wrote:
 Ben Pfaff b...@cs.stanford.edu writes:

  New module 'unistr/u8-mb-prev-uc'.
  New module 'unistr/u16-mb-prev-uc'.
  New module 'unistr/u32-mb-prev-uc'.

 Thanks, looks good to me.  Some nit-picking below.

 * _GL_UNUSED_PARAMETER of u32_mb_prev_uc seems to be a leftover, as the
   argument is actually used in the implementation.

 * The largest value of possible leading octet tested in
   test-u8-mb-prev-uc.c:exhaustive_test is 0xf5, while there are checks
   against 0xf8 in u8-mb-prev-uc.c.  Also code units above the surrogate
   code-point are not checked in u16 and u32 tests.
 * The license of u{8,16,32}-mbtouc changed to LGPLv2+ some
   time ago, maybe good to follow the change?

 All of the above are good points.  I will fix them in v5.

  v3-v4: Changed the code to always be safe.  It looks to me like the
unsafe version that I had written originally reflected a 
  misunderstanding of how the gnulib option for that was supposed to work.

 Are you going to add unsafe version later, or is it not useful at all?

 I do not have a use myself for unsafe versions, but I will add them if
 you think they are a good idea.  Let me know.

What is your opinion on this issue?



Re: [bug-libunistring] [PATCH v4] unistr: New modules for backward iteration in string.

2014-09-19 Thread Ben Pfaff
On Fri, Sep 19, 2014 at 04:55:24PM +0900, Daiki Ueno wrote:
 Ben Pfaff b...@cs.stanford.edu writes:
 
  New module 'unistr/u8-mb-prev-uc'.
  New module 'unistr/u16-mb-prev-uc'.
  New module 'unistr/u32-mb-prev-uc'.
 
 Thanks, looks good to me.  Some nit-picking below.
 
 * _GL_UNUSED_PARAMETER of u32_mb_prev_uc seems to be a leftover, as the
   argument is actually used in the implementation.

 * The largest value of possible leading octet tested in
   test-u8-mb-prev-uc.c:exhaustive_test is 0xf5, while there are checks
   against 0xf8 in u8-mb-prev-uc.c.  Also code units above the surrogate
   code-point are not checked in u16 and u32 tests.
 * The license of u{8,16,32}-mbtouc changed to LGPLv2+ some
   time ago, maybe good to follow the change?

All of the above are good points.  I will fix them in v5.

  v3-v4: Changed the code to always be safe.  It looks to me like the
unsafe version that I had written originally reflected a 
  misunderstanding of how the gnulib option for that was supposed to work.
 
 Are you going to add unsafe version later, or is it not useful at all?

I do not have a use myself for unsafe versions, but I will add them if
you think they are a good idea.  Let me know.



[PATCH v4] unistr: New modules for backward iteration in string.

2014-09-18 Thread Ben Pfaff
New module 'unistr/u8-mb-prev-uc'.
* lib/unistr.in.h (u8_mb_prev_uc): New declaration.
(u8_mb_prev_uc_aux): New declaration.
* lib/unistr/u8-mb-prev-uc.c: New file.
* lib/unistr/u8-mb-prev-uc-aux.c: New file.
* tests/test-u8-mb-prev-uc.c: New file.
* modules/u8-mb-prev-uc: New file.
* modules/u8-mb-prev-uc-tests: New file.

New module 'unistr/u16-mb-prev-uc'.
* lib/unistr.in.h (u16_mb_prev_uc): New declaration.
(u16_mb_prev_uc_aux): New declaration.
* lib/unistr/u16-mb-prev-uc.c: New file.
* lib/unistr/u16-mb-prev-uc-aux.c: New file.
* tests/test-u16-mb-prev-uc.c: New file.
* modules/u16-mb-prev-uc: New file.
* modules/u16-mb-prev-uc-tests: New file.

New module 'unistr/u32-mb-prev-uc'.
* lib/unistr.in.h (u32_mb_prev_uc): New declaration.
* lib/unistr/u32-mb-prev-uc.c: New file.
* tests/test-u32-mb-prev-uc.c: New file.
* modules/u32-mb-prev-uc: New file.
* modules/u32-mb-prev-uc-tests: New file.
---
v1-v2: Revised based on Bruno Haible's feedback.
v2-v3: Rebase only.
v3-v4: Changed the code to always be safe.  It looks to me like the
  unsafe version that I had written originally reflected a misunderstanding 
of how the gnulib option for that was supposed to work.

 ChangeLog   |  27 
 lib/unistr.in.h |  71 ++
 lib/unistr/u16-mb-prev-uc-aux.c |  52 +++
 lib/unistr/u16-mb-prev-uc.c |  62 +
 lib/unistr/u32-mb-prev-uc.c |  43 ++
 lib/unistr/u8-mb-prev-uc-aux.c  | 131 +
 lib/unistr/u8-mb-prev-uc.c  | 142 +++
 modules/unistr/u16-mb-prev-uc   |  28 
 modules/unistr/u16-mb-prev-uc-tests |  12 ++
 modules/unistr/u32-mb-prev-uc   |  27 
 modules/unistr/u32-mb-prev-uc-tests |  12 ++
 modules/unistr/u8-mb-prev-uc|  28 
 modules/unistr/u8-mb-prev-uc-tests  |  14 ++
 tests/unistr/test-u16-mb-prev-uc.c  |  89 
 tests/unistr/test-u32-mb-prev-uc.c  |  89 
 tests/unistr/test-u8-mb-prev-uc.c   | 270 
 16 files changed, 1097 insertions(+)
 create mode 100644 lib/unistr/u16-mb-prev-uc-aux.c
 create mode 100644 lib/unistr/u16-mb-prev-uc.c
 create mode 100644 lib/unistr/u32-mb-prev-uc.c
 create mode 100644 lib/unistr/u8-mb-prev-uc-aux.c
 create mode 100644 lib/unistr/u8-mb-prev-uc.c
 create mode 100644 modules/unistr/u16-mb-prev-uc
 create mode 100644 modules/unistr/u16-mb-prev-uc-tests
 create mode 100644 modules/unistr/u32-mb-prev-uc
 create mode 100644 modules/unistr/u32-mb-prev-uc-tests
 create mode 100644 modules/unistr/u8-mb-prev-uc
 create mode 100644 modules/unistr/u8-mb-prev-uc-tests
 create mode 100644 tests/unistr/test-u16-mb-prev-uc.c
 create mode 100644 tests/unistr/test-u32-mb-prev-uc.c
 create mode 100644 tests/unistr/test-u8-mb-prev-uc.c

diff --git a/ChangeLog b/ChangeLog
index 2da7d9b..8c7ba46 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+2011-01-01  Ben Pfaff  b...@cs.stanford.edu
+
+   New module 'unistr/u8-mb-prev-uc'.
+   * lib/unistr.in.h (u8_mb_prev_uc): New declaration.
+   (u8_mb_prev_uc_aux): New declaration.
+   * lib/unistr/u8-mb-prev-uc.c: New file.
+   * lib/unistr/u8-mb-prev-uc-aux.c: New file.
+   * tests/test-u8-mb-prev-uc.c: New file.
+   * modules/u8-mb-prev-uc: New file.
+   * modules/u8-mb-prev-uc-tests: New file.
+
+   New module 'unistr/u16-mb-prev-uc'.
+   * lib/unistr.in.h (u16_mb_prev_uc): New declaration.
+   (u16_mb_prev_uc_aux): New declaration.
+   * lib/unistr/u16-mb-prev-uc.c: New file.
+   * lib/unistr/u16-mb-prev-uc-aux.c: New file.
+   * tests/test-u16-mb-prev-uc.c: New file.
+   * modules/u16-mb-prev-uc: New file.
+   * modules/u16-mb-prev-uc-tests: New file.
+
+   New module 'unistr/u32-mb-prev-uc'.
+   * lib/unistr.in.h (u32_mb_prev_uc): New declaration.
+   * lib/unistr/u32-mb-prev-uc.c: New file.
+   * tests/test-u32-mb-prev-uc.c: New file.
+   * modules/u32-mb-prev-uc: New file.
+   * modules/u32-mb-prev-uc-tests: New file.
+
 2014-09-05  Mathieu Anquetin  math...@anquetin.eu
 
Trivial change.
diff --git a/lib/unistr.in.h b/lib/unistr.in.h
index 73d2c23..41078cc 100644
--- a/lib/unistr.in.h
+++ b/lib/unistr.in.h
@@ -300,6 +300,77 @@ extern int
u32_mbtoucr (ucs4_t *puc, const uint32_t *s, size_t n);
 #endif
 
+/* Return the length (number of units) of the last character in S, putting
+   its 'ucs4_t' representation in *PUC.  Upon failure, *PUC is set to 0xfffd,
+   and an appropriate number of units is returned.
+   The number of available units, N, must be  0.  */
+
+#if GNULIB_UNISTR_U8_MB_PREV_UC || HAVE_LIBUNISTRING
+# if !HAVE_INLINE
+extern int
+   u8_mb_prev_uc (ucs4_t *puc, const uint8_t *s, size_t n);
+# else
+extern int
+   u8_mb_prev_uc_aux (ucs4_t *puc, const uint8_t *s, size_t n);
+static inline int
+u8_mb_prev_uc (ucs4_t *puc, const uint8_t *s, size_t n)
+{
+  uint8_t c = s[n - 1];
+
+  if (c  0x80

Re: [PATCH v2] gnulib-tool: Download PO files

2014-09-05 Thread Ben Pfaff
On Thu, Sep 04, 2014 at 05:41:08PM +0200, Mathieu Anquetin wrote:
 On Thu, Sep 4, 2014 at 5:12 PM, Ben Pfaff b...@cs.stanford.edu wrote:
  On Thu, Sep 04, 2014 at 08:54:47AM +0200, Mathieu Anquetin wrote:
  These two patches change the behavior of gnulib-tool for downloading PO
  files from the Translation Project. It synces it with what is being done
  by the build-aux/bootstrap script (falling back to wget when rsync
  isn't available and setting options)
 
  These look good to me.  Have you tested them?
 
 rsync didn't work for me (don't know if it's a firewall or a server
 issue though), so yes I have tested them (at least, the wget part). I
 don't think there should be any problem since the code is similar to
 what build-aux/bootstrap does (I didn't reinvent the wheel).
 
  Do you have gnulib commit
  rights or do you need someone to commit them for you?
 
 Never commited before (first contribution) so I don't think I have
 commit rights. Maybe I am wrong though.

Thanks, I applied these to the repository.  I marked them as trivial
changes in the change log.  This does not mean that they were trivial
problems or solutions but that we judged the changes to be small enough
to be insignificant for copyright purposes.



Re: [PATCH] error: drop spurious semicolon

2014-09-04 Thread Ben Pfaff
On Thu, Sep 04, 2014 at 06:28:19AM -0600, Eric Blake wrote:
 Noticed this while writing a syntax check rule to look for bogus
 doubled semicolons.  If there's interest, I could add this rule
 to maint.mk:
 
 # Except for shell files and for loops, double semicolon is probably a mistake
 sc_prohibit_double_semicolon:
   @prohibit=';;'  \
   in_vc_files='\.[ch]$$'  \
   exclude='for \(.*\)'\
   halt=Double semicolon detected\
 $(_sc_search_regexp)

That looks useful to me.



Re: [PATCH v2] gnulib-tool: Download PO files

2014-09-04 Thread Ben Pfaff
On Thu, Sep 04, 2014 at 08:54:47AM +0200, Mathieu Anquetin wrote:
 These two patches change the behavior of gnulib-tool for downloading PO
 files from the Translation Project. It synces it with what is being done
 by the build-aux/bootstrap script (falling back to wget when rsync
 isn't available and setting options)

These look good to me.  Have you tested them?  Do you have gnulib commit
rights or do you need someone to commit them for you?



Re: [PATCH] gnulib-tool: Fallback to wget when rsync fails

2014-09-02 Thread Ben Pfaff
On Tue, Sep 02, 2014 at 12:22:54PM +0200, Mathieu Anquetin wrote:
 Current implementation only tries to rsync PO files when rsync is
 installed on the host. In case of error, no files are downloaded even
 if they are available. This leads to bootstrap problems for hosts
 that lie behind a restrictive firewall.
 
 This patch always tries to rsync by default, falling back to wget if
 an error occurs.
 
 Signed-off-by: Mathieu Anquetin math...@anquetin.eu

Looking at wget(1), I'm a little surprised that the selected options
appear to avoid clobbering files whose names conflict.  I would have
expected that we'd want to overwrite them, to get the fresher version.
But the existing wget invocation does the same thing, so maybe it's
intentional?



Re: [PATCH] gnulib-tool: Fallback to wget when rsync fails

2014-09-02 Thread Ben Pfaff
On Tue, Sep 02, 2014 at 05:15:16PM +0200, Bernhard Voelker wrote:
 On 09/02/2014 05:06 PM, Ben Pfaff wrote:
 Looking at wget(1), I'm a little surprised that the selected options
 appear to avoid clobbering files whose names conflict.  I would have
 expected that we'd want to overwrite them, to get the fresher version.
 But the existing wget invocation does the same thing, so maybe it's
 intentional?
 
 clobbering is implicitly disabled with the -nd option:
 
   -nd
--no-directories
Do not create a hierarchy of directories when retrieving
recursively.  With this option turned on, all files will
get saved to the current directory, without clobbering (if
a name shows up more than once, the filenames will get
extensions .n).

Yes.  As I said, I'm a little surprised that the selected options appear
to avoid clobbering files whose names conflict.



Re: [PATCH] qsort_r: new module, for GNU-style qsort_r

2014-08-31 Thread Ben Pfaff
On Fri, Aug 29, 2014 at 01:50:48PM -0700, Paul Eggert wrote:
 This works even on FreeBSD, which has an incompatible qsort_r API.
 * MODULES.html.sh: Add it.
 * doc/glibc-functions/qsort_r.texi: It's now supported.
 * lib/qsort.c: New file, taken from glibc with minor changes
 inside #ifndef _LIBC and with an unnecessary #include alloca.h
 removed.
 * lib/qsort_r.c: New file, compiled only on FreeBSD.
 * lib/stdlib.in.h (qsort_r): Declare in the usual way.
 * m4/stdlib_h.m4 (gl_STDLIB_H_DEFAULTS):
 * modules/qsort_r, modules/qsort_r-tests: New files.
 * modules/stdlib (Makefile): Set up its defaults.
 * tests/test-qsort_r.c: New file.

I'm happy to see this module; it's often useful to have the ability to
pass auxiliary data to a sort comparison function.

I skimmed through the patch but I did not review it in detail.

Thanks,

Ben.



Re: relocatable-perl module

2014-01-09 Thread Ben Pfaff
On Thu, Jan 09, 2014 at 10:34:36PM +, Reuben Thomas wrote:
 [I'm sorry, I took us off-list a couple messages ago. Back on now.]

Happens to me from time to time too.

 On 9 January 2014 16:36, Ben Pfaff b...@cs.stanford.edu wrote:
 
  
   OK, do you need me to resend the patch?
 
  If you don't mind.
 
 
 Attached.
 
 
 
It looks to me like you don't have direct access to the gnulib Git
repository.  Do you want me to apply this on your behalf?
  
   Yes please.
 
  OK, I'll do that tonight.

Thanks, I'll do that in a minute.

I am folding in one incremental change:

diff --git a/doc/relocatable-maint.texi b/doc/relocatable-maint.texi
index 75b1318..23b5048 100644
--- a/doc/relocatable-maint.texi
+++ b/doc/relocatable-maint.texi
@@ -171,7 +171,8 @@ $some_datadir = relocate(@@datadir@@/something);
 
 You must adapt the definition of @code{$orig_installdir}, depending on
 where the script gets installed.  Also, at the end, instead of
-@code{$gettext_dir}, transform those variables that you need.
+@code{sysconfdir} and @code{some_datadir}, transform those variables
+that you need.
 
 @item
 In your @file{Makefile.am}, for every program @command{foo} that gets



Re: relocatable-perl module

2014-01-08 Thread Ben Pfaff
On Wed, Jan 08, 2014 at 11:09:45AM +, Reuben Thomas wrote:
 On 8 January 2014 06:10, Ben Pfaff b...@cs.stanford.edu wrote:
 
  Reuben Thomas r...@sc3d.org writes:
 
   The attached patch adds a module relocatable-perl that does for Perl
   scripts what relocatable-script does for shell scripts.
 
  It looks to me like it's successfully in the same spirit as the
  shell implementation.  I didn't review the Perl code in detail
  for correctness.
 
  I suggest changing the suggested usage in the documentation to be
  closer to the revised suggestion that I made for the shell
  implementation in a separate email a few minutes ago:
  http://www.mail-archive.com/bug-gnulib@gnu.org/msg29960.html
 
 
 Done; new patch attached. (The structure of the code in the Perl case is
 slightly different because of the way that subroutine definitions happen in
 Perl, which I've tried to explain in a comment.)

It makes sense to me.

I guess that you should make yourself a maintainer of this module.

It looks to me like you don't have direct access to the gnulib Git
repository.  Do you want me to apply this on your behalf?



Re: Relocatable Perl scripts

2014-01-08 Thread Ben Pfaff
On Wed, Jan 08, 2014 at 09:36:39AM +, Reuben Thomas wrote:
 On 8 January 2014 06:01, Ben Pfaff b...@cs.stanford.edu wrote:
 
  Reuben Thomas r...@sc3d.org writes:
 
   On 22 November 2013 00:24, Reuben Thomas r...@sc3d.org wrote:
  
   On 21 November 2013 23:50, Reuben Thomas r...@sc3d.org wrote:
  
   I just realised that I never had this working properly, and I track
  the
   problem all the way back to the gnulib manual. In the line:
  
  
   echo $gettext_dir/ \
  
   should that not rather be @gettext_dir@? If not, then where does
   gettext_dir get its value? It's not mentioned previously in the
  example
   script, and it's not mentioned in relocatable.sh.in, as far as I
  can see.
  
   Ping! The same question applies to the relocatable-perl module whose
  first
   version I've just finished.
 
  Sorry I've been so slow to respond.  I don't keep up with the
  gnulib mailing list very well.  You will get replies more
  promptly if you CC me on emails related to my modules.
 
 Thanks for the tip; since the module has Bruno's name on it too, it wasn't
 obvious to me I should've addressed you specifically.

I generally defer to Bruno in all things gnulib, but he's been more or
less absent for a few years so I guess I'm the de facto primary
maintainer of modules for which we are marked as co-maintainers.



Re: Relocatable Perl scripts

2014-01-08 Thread Ben Pfaff
On Wed, Jan 08, 2014 at 09:44:35AM +, Reuben Thomas wrote:
 On 8 January 2014 05:59, Ben Pfaff b...@cs.stanford.edu wrote:
 
  I think that the following change to Gnulib would clarify.  I
  have not actually tested the shell code it suggests, although I
  probably should.  Will you take a look at it?
 
  diff --git a/doc/relocatable-maint.texi b/doc/relocatable-maint.texi
  index 58160cf..86d1438 100644
  --- a/doc/relocatable-maint.texi
  +++ b/doc/relocatable-maint.texi
  @@ -122,12 +122,20 @@ if test @@RELOCATABLE@@ = yes; then
 orig_installdir=$bindir # see Makefile.am's *_SCRIPTS variables
 func_find_curr_installdir # determine curr_installdir
 func_find_prefixes
  -  # Relocate the directory variables that we use.
  -  gettext_dir=`
  -echo $gettext_dir/ \
  +  relocate () {
  +echo $1/ \
   | sed -e s%^$@{orig_installprefix@}/%$@{curr_installprefix@}/% \
  -| sed -e 's,/$,,'`
  +| sed -e 's,/$,,'
  +  }
  +else
  +  relocate () {
  +echo $1
  +  }
   fi
  +
  +# Get some relocated directory names.
  +sysconfdir=`relocate @@sysconfdir@@`
  +some_datadir=`relocate @@datadir@@/something`
   @end example
 
   You must adapt the definition of @code{orig_installdir}, depending on
 
 
 This looks good to me. I don't currently have a shell use via which to test
 it, but I will implement the same idea in my Perl version.

Thanks for the review.  I applied this to master, as follows.

--8--cut here--8--

From: Ben Pfaff b...@cs.stanford.edu
Date: Wed, 8 Jan 2014 22:15:21 -0800
Subject: [PATCH] relocatable-shell: Update suggested usage in maintainer
 documentation.

Instead of suggesting an inline usage of sed, that would have to be
cut-and-pasted for every directory to be relocated, suggest a shell
function.

Make the example obviously an example, whereas previously it looked like
it might be literal text.

Thanks to Reuben Thomas r...@sc3d.org for pointing out these issues.  See
http://lists.gnu.org/archive/html/bug-gnulib/2014-01/msg00039.html for
further context.
---
 doc/relocatable-maint.texi |   19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/doc/relocatable-maint.texi b/doc/relocatable-maint.texi
index 8780b84..f972b2f 100644
--- a/doc/relocatable-maint.texi
+++ b/doc/relocatable-maint.texi
@@ -122,17 +122,26 @@ if test @@RELOCATABLE@@ = yes; then
   orig_installdir=$bindir # see Makefile.am's *_SCRIPTS variables
   func_find_curr_installdir # determine curr_installdir
   func_find_prefixes
-  # Relocate the directory variables that we use.
-  gettext_dir=`
-echo $gettext_dir/ \
+  relocate () @{
+echo $1/ \
 | sed -e s%^$@{orig_installprefix@}/%$@{curr_installprefix@}/% \
-| sed -e 's,/$,,'`
+| sed -e 's,/$,,'
+  @}
+else
+  relocate () @{
+echo $1
+  @}
 fi
+
+# Get some relocated directory names.
+sysconfdir=`relocate @@sysconfdir@@`
+some_datadir=`relocate @@datadir@@/something`
 @end example
 
 You must adapt the definition of @code{orig_installdir}, depending on
 where the script gets installed.  Also, at the end, instead of
-@code{gettext_dir}, transform those variables that you need.
+@code{sysconfdir} and @code{some_datadir}, transform those variables
+that you need.
 
 @item
 In your @file{Makefile.am}, for every program @command{foo} that gets
-- 
1.7.10.4




Re: Relocatable Perl scripts

2014-01-07 Thread Ben Pfaff
Reuben Thomas r...@sc3d.org writes:

 I just realised that I never had this working properly, and I track the
 problem all the way back to the gnulib manual. In the line:
  

     echo $gettext_dir/ \

 should that not rather be @gettext_dir@? If not, then where does gettext_dir
 get its value? It's not mentioned previously in the example script, and it's
 not mentioned in relocatable.sh.in, as far as I can see.

This is confusing, so I spent a few minutes looking around.  I
believe that this comes directly from
gettext-tools/misc/gettextize.in in GNU gettext.  That script
initializes gettext_dir in a stanza near the top, as:

# Set variables
# - gettext_dir directory where the sources are stored.
prefix=@prefix@
datarootdir=@datarootdir@
gettext_dir=@datadir@/gettext

and then much later relocates it as:

if test @RELOCATABLE@ = yes; then
  exec_prefix=@exec_prefix@
  bindir=@bindir@
  orig_installdir=$bindir # see Makefile.am's *_SCRIPTS variables
  func_find_curr_installdir # determine curr_installdir
  func_find_prefixes
  # Relocate the directory variables that we use.
  gettext_dir=`echo $gettext_dir/ | sed -e 
s%^${orig_installprefix}/%${curr_installprefix}/% | sed -e 's,/$,,'`
fi

So, here gettext_dir is an example.  In reality, your script
might want to relocate any installation directory or directories,
but it will probably not want to relocate that particular
directory (because you are probably not writing GNU gettext).

I think that the following change to Gnulib would clarify.  I
have not actually tested the shell code it suggests, although I
probably should.  Will you take a look at it?

diff --git a/doc/relocatable-maint.texi b/doc/relocatable-maint.texi
index 58160cf..86d1438 100644
--- a/doc/relocatable-maint.texi
+++ b/doc/relocatable-maint.texi
@@ -122,12 +122,20 @@ if test @@RELOCATABLE@@ = yes; then
   orig_installdir=$bindir # see Makefile.am's *_SCRIPTS variables
   func_find_curr_installdir # determine curr_installdir
   func_find_prefixes
-  # Relocate the directory variables that we use.
-  gettext_dir=`
-echo $gettext_dir/ \
+  relocate () { 
+echo $1/ \
 | sed -e s%^$@{orig_installprefix@}/%$@{curr_installprefix@}/% \
-| sed -e 's,/$,,'`
+| sed -e 's,/$,,'
+  }
+else
+  relocate () { 
+echo $1
+  }
 fi
+
+# Get some relocated directory names.
+sysconfdir=`relocate @@sysconfdir@@`
+some_datadir=`relocate @@datadir@@/something`
 @end example
 
 You must adapt the definition of @code{orig_installdir}, depending on



Re: Relocatable Perl scripts

2014-01-07 Thread Ben Pfaff
Reuben Thomas r...@sc3d.org writes:

 On 22 November 2013 00:24, Reuben Thomas r...@sc3d.org wrote:

 On 21 November 2013 23:50, Reuben Thomas r...@sc3d.org wrote:

 I just realised that I never had this working properly, and I track the
 problem all the way back to the gnulib manual. In the line:
  

     echo $gettext_dir/ \

 should that not rather be @gettext_dir@? If not, then where does
 gettext_dir get its value? It's not mentioned previously in the example
 script, and it's not mentioned in relocatable.sh.in, as far as I can see.

 Ping! The same question applies to the relocatable-perl module whose first
 version I've just finished.

Sorry I've been so slow to respond.  I don't keep up with the
gnulib mailing list very well.  You will get replies more
promptly if you CC me on emails related to my modules.



Re: relocatable-perl module

2014-01-07 Thread Ben Pfaff
Reuben Thomas r...@sc3d.org writes:

 The attached patch adds a module relocatable-perl that does for Perl
 scripts what relocatable-script does for shell scripts.

It looks to me like it's successfully in the same spirit as the
shell implementation.  I didn't review the Perl code in detail
for correctness.

I suggest changing the suggested usage in the documentation to be
closer to the revised suggestion that I made for the shell
implementation in a separate email a few minutes ago:
http://www.mail-archive.com/bug-gnulib@gnu.org/msg29960.html



Re: Small improvement to relocatable-maint.texi

2013-11-18 Thread Ben Pfaff
Reuben Thomas r...@sc3d.org writes:

 s/RedHat, Debian, and similar package systems/Yum, apt and similar
 package-management systems/

I agree that this was not well worded.

I applied the following commit to master.

--8--cut here--8--

From: Ben Pfaff b...@cs.stanford.edu
Date: Mon, 18 Nov 2013 22:17:47 -0800
Subject: [PATCH] doc: Improve wording in relocatable-maint.texi.

Reported by Reuben Thomas r...@sc3d.org.
---
 ChangeLog  |6 ++
 doc/relocatable-maint.texi |2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index aae8c64..aa5c36f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2013-11-18  Ben Pfaff  b...@cs.stanford.edu
+
+   * doc/relocatable-maint.texi (Supporting Relocation): Improve
+   wording.
+   Reported by Reuben Thomas r...@sc3d.org.
+
 2013-11-13  Paul Eggert  egg...@cs.ucla.edu
 
* lib/getgroups.c (posix_getgroups, getgroups) [__APPLE__]:
diff --git a/doc/relocatable-maint.texi b/doc/relocatable-maint.texi
index 58160cf..8780b84 100644
--- a/doc/relocatable-maint.texi
+++ b/doc/relocatable-maint.texi
@@ -8,7 +8,7 @@ and have it work correctly (including i18n).  So many users 
need to go
 through @code{configure; make; make install} with all its
 dependencies, options, and hurdles.
 
-Red Hat, Debian, and similar package systems solve the ``ease of
+Red Hat, Debian, and other binary distributions solve the ``ease of
 installation'' problem, but they hardwire path names, usually to
 @file{/usr} or @file{/usr/local}.  This means that users need root
 privileges to install a binary package, and prevents installing two
-- 
1.7.10.4




Re: relocatable-prog status

2013-10-27 Thread Ben Pfaff
Sylvain b...@beuc.net writes:

 On Thu, Oct 24, 2013 at 08:10:17PM -0700, Ben Pfaff wrote:
 After waiting about 72 hours, I received no comments, whether positive,
 negative, or neutral, so I applied this to master.

 Thanks much!

You're welcome!

(If anything else comes up, please feel free to ping me about it
much sooner than a year afterward!  I am forgetful...)



Re: relocatable-prog status

2013-10-24 Thread Ben Pfaff
On Mon, Oct 21, 2013 at 10:51:38PM -0700, Ben Pfaff wrote:
 On Mon, Oct 21, 2013 at 08:08:26PM +0200, Sylvain wrote:
  Hi,
  
  On Sun, Oct 20, 2013 at 01:48:19PM -0700, Ben Pfaff wrote:
   Sylvain b...@beuc.net writes:
   
It's been a while (1 year 1/2) since
http://lists.debian.org/debian-bsd/2012/05/msg00032.html and I still
need to manually patch gnulib before releasing.
   
If I assume the relocatable-prog module is not maintained, that I'm
probably the only person on earth to use it, and that I should just
drop it from my package, am I wrong? ;)
   
   No, we use relocatable-prog in GNU PSPP as well.  I build with it
   all the time.  But I don't use Debian/kFreeBSD or Debian/Hurd, so
   I don't see this problem.
  
  I don't use them either, but the Debian autobuilder does :)
  In the case of pspp, I see that it compiles fine on these
  platforms, e,g.:
  https://buildd.debian.org/status/fetch.php?pkg=pspparch=kfreebsd-amd64ver=0.7.9%2bgit20120620-1.2stamp=1369659905
  
  AFAIU you'll hit the bug as soon as you have more than 1 program in a
  bin_PROGRAMS statement.  Mine is:
bin_PROGRAMS = freedink freedinkedit
  
  In this case automake makes 1 call to 'install' with multiple
  arguments, but 'install-reloc' only support one. Bruno's patch adds
  support for multiple arguments.  It's as simple as that.
  
  
   I do read this list, and I am a maintainer of relocatable-prog,
   but somehow I missed the discussion.  Maybe I thought that Bruno
   was going to take care of it, since he suggested the patch.
   
   Are you still happy with install-reloc with Bruno's patch?  If
   so, then I will commit it to gnulib.
  
  The patch is manually applied for each FreeDink release and compiles
  on all Debian architectures (as well as other distros and OSes).
  
  When discussing with Bruno at GHM 2012, he told me he absolutely
  wanted to write a test case first.  But after this much time I wonder
  why a non-test-cased bug should prevail over an non-test-cased fix ;)
  
  So as far as I'm concerned, I'd be happy to see this fix committed :)
 
 That sounds reasonable to me.
 
 Here is what I plan to commit.  Does anyone see anything I should change
 beforehand?  It has been a long time since I did much work on gnulib
 (mainly because it just works for me these days), so I want to make
 sure I'm not overlooking something.
 
 All of the substantive change in the patch is by Bruno, hence I am
 making him the author of the commit (I will be the committer, if I push
 it).

After waiting about 72 hours, I received no comments, whether positive,
negative, or neutral, so I applied this to master.

Thanks,

Ben.



Re: relocatable-prog status

2013-10-21 Thread Ben Pfaff
On Mon, Oct 21, 2013 at 08:08:26PM +0200, Sylvain wrote:
 Hi,
 
 On Sun, Oct 20, 2013 at 01:48:19PM -0700, Ben Pfaff wrote:
  Sylvain b...@beuc.net writes:
  
   It's been a while (1 year 1/2) since
   http://lists.debian.org/debian-bsd/2012/05/msg00032.html and I still
   need to manually patch gnulib before releasing.
  
   If I assume the relocatable-prog module is not maintained, that I'm
   probably the only person on earth to use it, and that I should just
   drop it from my package, am I wrong? ;)
  
  No, we use relocatable-prog in GNU PSPP as well.  I build with it
  all the time.  But I don't use Debian/kFreeBSD or Debian/Hurd, so
  I don't see this problem.
 
 I don't use them either, but the Debian autobuilder does :)
 In the case of pspp, I see that it compiles fine on these
 platforms, e,g.:
 https://buildd.debian.org/status/fetch.php?pkg=pspparch=kfreebsd-amd64ver=0.7.9%2bgit20120620-1.2stamp=1369659905
 
 AFAIU you'll hit the bug as soon as you have more than 1 program in a
 bin_PROGRAMS statement.  Mine is:
   bin_PROGRAMS = freedink freedinkedit
 
 In this case automake makes 1 call to 'install' with multiple
 arguments, but 'install-reloc' only support one. Bruno's patch adds
 support for multiple arguments.  It's as simple as that.
 
 
  I do read this list, and I am a maintainer of relocatable-prog,
  but somehow I missed the discussion.  Maybe I thought that Bruno
  was going to take care of it, since he suggested the patch.
  
  Are you still happy with install-reloc with Bruno's patch?  If
  so, then I will commit it to gnulib.
 
 The patch is manually applied for each FreeDink release and compiles
 on all Debian architectures (as well as other distros and OSes).
 
 When discussing with Bruno at GHM 2012, he told me he absolutely
 wanted to write a test case first.  But after this much time I wonder
 why a non-test-cased bug should prevail over an non-test-cased fix ;)
 
 So as far as I'm concerned, I'd be happy to see this fix committed :)

That sounds reasonable to me.

Here is what I plan to commit.  Does anyone see anything I should change
beforehand?  It has been a long time since I did much work on gnulib
(mainly because it just works for me these days), so I want to make
sure I'm not overlooking something.

All of the substantive change in the patch is by Bruno, hence I am
making him the author of the commit (I will be the committer, if I push
it).

Thanks,

Ben.

--8--cut here--8--

From: Bruno Haible br...@clisp.org
Date: Mon, 21 Oct 2013 22:48:35 -0700
Subject: [PATCH] install-reloc: Support multi-binary installation.

* build-aux/install-reloc: Support installing multiple programs in
one invocation, as done by Automake starting with commit
4295fe33eb23f (Multi-file install for PROGRAMS.).  From Bruno
Haible br...@clisp.org, archived at
http://lists.debian.org/debian-bsd/2012/05/msg00032.html.
Reported by Sylvain b...@gnu.org.
---
 ChangeLog   |   10 ++
 build-aux/install-reloc |  272 ++-
 2 files changed, 184 insertions(+), 98 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1c6a7af..31aaacc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2013-10-21  Ben Pfaff  b...@cs.stanford.edu
+
+   install-reloc: Support multi-binary installation.
+   * build-aux/install-reloc: Support installing multiple programs in
+   one invocation, as done by Automake starting with commit
+   4295fe33eb23f (Multi-file install for PROGRAMS.).  From Bruno
+   Haible br...@clisp.org, archived at
+   http://lists.debian.org/debian-bsd/2012/05/msg00032.html.
+   Reported by Sylvain b...@gnu.org.
+
 2013-10-21  Jim Meyering  meyer...@fb.com
 
regex: also remove dependency on HAVE_WCSCOLL
diff --git a/build-aux/install-reloc b/build-aux/install-reloc
index 7edc541..0eb1326 100755
--- a/build-aux/install-reloc
+++ b/build-aux/install-reloc
@@ -16,11 +16,11 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see http://www.gnu.org/licenses/.
 
-# Usage:
-#   install-reloc library_path_var library_path_value prefix destdir \
-# compile_command srcdir builddir config_h_dir exeext \
-# strip_command \
-# install_command... destprog
+# Usage 1:
+#   install-reloc -- library_path_var library_path_value prefix destdir \
+#compile_command srcdir builddir config_h_dir exeext \
+#strip_command \
+#install_command... destprog
 # where
 #   - library_path_var is the platform dependent runtime library path variable
 #   - library_path_value is a colon separated list of directories that contain
@@ -39,51 +39,67 @@
 # stripping is desired
 #   - install_command is the install command line, excluding the final destprog
 #   - destprog is the destination program name
+# Usage 2:
+#   env

Re: relocatable-prog status

2013-10-20 Thread Ben Pfaff
Sylvain b...@beuc.net writes:

 It's been a while (1 year 1/2) since
 http://lists.debian.org/debian-bsd/2012/05/msg00032.html and I still
 need to manually patch gnulib before releasing.

 If I assume the relocatable-prog module is not maintained, that I'm
 probably the only person on earth to use it, and that I should just
 drop it from my package, am I wrong? ;)

No, we use relocatable-prog in GNU PSPP as well.  I build with it
all the time.  But I don't use Debian/kFreeBSD or Debian/Hurd, so
I don't see this problem.

I do read this list, and I am a maintainer of relocatable-prog,
but somehow I missed the discussion.  Maybe I thought that Bruno
was going to take care of it, since he suggested the patch.

Are you still happy with install-reloc with Bruno's patch?  If
so, then I will commit it to gnulib.



c-xvasprintf: Fix implicit declaration of function GCC warning.

2012-12-25 Thread Ben Pfaff
I pushed this to master as an obvious fix.

--8--cut here--8--

From: Ben Pfaff b...@cs.stanford.edu
Date: Tue, 25 Dec 2012 21:18:14 -0800
Subject: [PATCH] c-xvasprintf: Fix implicit declaration of function GCC
 warning.

* lib/c-xvasprintf.c: Add missing #include c-vasprintf.h, for
c_vasprintf() prototype.
---
 ChangeLog  |6 ++
 lib/c-xvasprintf.c |1 +
 2 files changed, 7 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 211cc59..a454de2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2012-12-25  Ben Pfaff  b...@cs.stanford.edu
+
+   c-xvasprintf: Fix implicit declaration of function GCC warning.
+   * lib/c-xvasprintf.c: Add missing #include c-vasprintf.h, for
+   c_vasprintf() prototype.
+
 2012-12-24  Ben Pfaff  b...@cs.stanford.edu
 
c-vasprintf: Fix empty declaration warning reported by GCC.
diff --git a/lib/c-xvasprintf.c b/lib/c-xvasprintf.c
index 87be542..b345c39 100644
--- a/lib/c-xvasprintf.c
+++ b/lib/c-xvasprintf.c
@@ -22,6 +22,7 @@
 #include errno.h
 #include stdio.h
 
+#include c-vasprintf.h
 #include xalloc.h
 
 char *
-- 
1.7.10.4




c-vasprintf: Fix empty declaration warning reported by GCC.

2012-12-24 Thread Ben Pfaff
I already pushed this as an obvious fix.

--8--cut here--8--

From: Ben Pfaff b...@cs.stanford.edu
Date: Mon, 24 Dec 2012 16:50:37 -0800
Subject: [PATCH] c-vasprintf: Fix empty declaration warning reported by
 GCC.

* lib/c-vasprintf.h: Remove stray semicolon.
---
 ChangeLog |5 +
 lib/c-vasprintf.h |2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index cfe09f3..211cc59 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2012-12-24  Ben Pfaff  b...@cs.stanford.edu
+
+   c-vasprintf: Fix empty declaration warning reported by GCC.
+   * lib/c-vasprintf.h: Remove stray semicolon.
+
 2012-12-23  Paul Eggert  egg...@cs.ucla.edu
 
gettext: avoid obsolete macro AM_PROG_MKDIR_P
diff --git a/lib/c-vasprintf.h b/lib/c-vasprintf.h
index 347679e..1b85de2 100644
--- a/lib/c-vasprintf.h
+++ b/lib/c-vasprintf.h
@@ -39,7 +39,7 @@ extern C {
 /* asprintf() and vasprintf(), but formatting takes place in the C locale, that
is, the decimal point used in floating-point formatting directives is always
'.'. */
-int c_asprintf (char **resultp, const char *format, ...);
+int c_asprintf (char **resultp, const char *format, ...)
_GL_ATTRIBUTE_FORMAT ((__printf__, 2, 3));
 int c_vasprintf (char **resultp, const char *format, va_list args)
_GL_ATTRIBUTE_FORMAT ((__printf__, 2, 0));
-- 
1.7.10.4




Re: C locale *printf functions ?

2012-12-18 Thread Ben Pfaff
On Tue, Dec 18, 2012 at 06:41:47PM +0100, Jim Meyering wrote:
 Ben Pfaff wrote:
  Here is a revised version.  It passes the included tests, so I'll
  wait 72 hours for comments and then commit it if no one objects.

 That sounds great.
 I haven't reviewed all of that, but really like the idea
 and am glad to see so many tests.

Thanks a lot for the review.  I pushed this to master.



Re: C locale *printf functions ?

2012-12-15 Thread Ben Pfaff
Here is a revised version.  It passes the included tests, so I'll
wait 72 hours for comments and then commit it if no one objects.

--8--cut here--8--

From: Ben Pfaff b...@cs.stanford.edu
Date: Sat, 15 Dec 2012 22:11:03 -0800
Subject: [PATCH] New 'c-*printf' modules for formatted output in C locale.

New module 'c-vasnprintf'.
* modules/c-vasnprintf: New file.
* lib/c-vasnprintf.c: New file.
* lib/c-vasnprintf.h: New file.

New module 'c-snprintf'.
* modules/c-snprintf: New file.
* modules/c-snprintf-tests: New file.
* lib/c-snprintf.c: New file.
* lib/c-snprintf.h: New file.
* tests/test-c-snprintf.c: New file.
* tests/test-c-snprintf.sh: New file.

New module 'c-vsnprintf'.
* modules/c-vsnprintf: New file.
* modules/c-vsnprintf-tests: New file.
* lib/c-vsnprintf.c: New file.
* lib/c-vsnprintf.h: New file.
* tests/test-c-vsnprintf.c: New file.
* tests/test-c-vsnprintf.sh: New file.

New module 'c-vasprintf'.
* modules/c-vasprintf: New file.
* modules/c-vasprintf-tests: New file.
* lib/c-asprintf.c: New file.
* lib/c-vasprintf.c: New file.
* lib/c-vasprintf.h: New file.
* tests/test-c-vasprintf.c  +: New file.
* tests/test-c-vasprintf.sh: New file.

New module 'c-xvasprintf'.
* modules/c-xvasprintf: New file.
* modules/c-xvasprintf-tests: New file.
* lib/c-xasprintf.c: New file.
* lib/c-xvasprintf.c: New file.
* lib/c-xvasprintf.h: New file.
* tests/test-c-xvasprintf.c: New file.
* tests/test-c-xvasprintf.sh: New file.
---
 ChangeLog  |   43 
 lib/c-asprintf.c   |   35 +++
 lib/c-snprintf.c   |   75 +
 lib/c-snprintf.h   |   46 +
 lib/c-vasnprintf.c |   43 
 lib/c-vasnprintf.h |   76 +
 lib/c-vasprintf.c  |   46 +
 lib/c-vasprintf.h  |   51 
 lib/c-vsnprintf.c  |   74 
 lib/c-vsnprintf.h  |   49 +++
 lib/c-xasprintf.c  |   34 +++
 lib/c-xvasprintf.c |   40 ++
 lib/c-xvasprintf.h |   58 
 modules/c-snprintf |   23 +
 modules/c-snprintf-tests   |   17 ++
 modules/c-vasnprintf   |   55 ++
 modules/c-vasprintf|   24 +
 modules/c-vasprintf-tests  |   17 ++
 modules/c-vsnprintf|   24 +
 modules/c-vsnprintf-tests  |   17 ++
 modules/c-xvasprintf   |   24 +
 modules/c-xvasprintf-tests |   18 ++
 tests/test-c-snprintf.c|   58 
 tests/test-c-snprintf.sh   |   15 +
 tests/test-c-vasprintf.c   |   80 
 tests/test-c-vasprintf.sh  |   15 +
 tests/test-c-vsnprintf.c   |   73 
 tests/test-c-vsnprintf.sh  |   15 +
 tests/test-c-xvasprintf.c  |   78 ++
 tests/test-c-xvasprintf.sh |   15 +
 30 files changed, 1238 insertions(+)
 create mode 100644 lib/c-asprintf.c
 create mode 100644 lib/c-snprintf.c
 create mode 100644 lib/c-snprintf.h
 create mode 100644 lib/c-vasnprintf.c
 create mode 100644 lib/c-vasnprintf.h
 create mode 100644 lib/c-vasprintf.c
 create mode 100644 lib/c-vasprintf.h
 create mode 100644 lib/c-vsnprintf.c
 create mode 100644 lib/c-vsnprintf.h
 create mode 100644 lib/c-xasprintf.c
 create mode 100644 lib/c-xvasprintf.c
 create mode 100644 lib/c-xvasprintf.h
 create mode 100644 modules/c-snprintf
 create mode 100644 modules/c-snprintf-tests
 create mode 100644 modules/c-vasnprintf
 create mode 100644 modules/c-vasprintf
 create mode 100644 modules/c-vasprintf-tests
 create mode 100644 modules/c-vsnprintf
 create mode 100644 modules/c-vsnprintf-tests
 create mode 100644 modules/c-xvasprintf
 create mode 100644 modules/c-xvasprintf-tests
 create mode 100644 tests/test-c-snprintf.c
 create mode 100755 tests/test-c-snprintf.sh
 create mode 100644 tests/test-c-vasprintf.c
 create mode 100755 tests/test-c-vasprintf.sh
 create mode 100644 tests/test-c-vsnprintf.c
 create mode 100755 tests/test-c-vsnprintf.sh
 create mode 100644 tests/test-c-xvasprintf.c
 create mode 100755 tests/test-c-xvasprintf.sh

diff --git a/ChangeLog b/ChangeLog
index 3e04979..9ebac20 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,46 @@
+2012-12-15  Ben Pfaff  b...@cs.stanford.edu
+
+   New 'c-*printf' modules for formatted output in C locale.
+
+   New module 'c-vasnprintf'.
+   * modules/c-vasnprintf: New file.
+   * lib/c-vasnprintf.c: New file.
+   * lib/c-vasnprintf.h: New file.
+
+   New module 'c-snprintf'.
+* modules/c-snprintf: New file.
+* modules/c-snprintf

Re: C locale *printf functions ?

2012-12-10 Thread Ben Pfaff
Right, I'll work on those.

What is vsnformat?

John Darrington j...@darrington.wattle.id.au writes:

 I gave it a rudimentary test and it seems to do what I want.
 I will also need other related functions, though, Eg xasprintf,
 vsnformat, etc.

 J'

 On Thu, Dec 06, 2012 at 10:59:44PM -0800, Ben Pfaff wrote:
  John Darrington j...@darrington.wattle.id.au writes:
  
   Gnulib has a number of c-* variants of string processing functions,
   eg c-strtod, c-strcasecmp etc  But notably absent are any locale
   independent printf routines.   We could use some in PSPP.
  
  Here's some initial work on that.  It only defines c_snprintf()
  so far.  The test passes for me.
  
  Comments are welcome, from anyone.
  
  ---
   lib/c-snprintf.c |   74 
 
   lib/c-snprintf.h |   46 
   lib/c-vasnprintf.c   |   43 ++
   lib/c-vasnprintf.h   |   76 
 ++
   modules/c-snprintf   |   23 ++
   modules/c-snprintf-tests |   17 +++
   modules/c-vasnprintf |   55 +
   tests/test-c-snprintf.c  |   58 +++
   tests/test-c-snprintf.sh |   15 +
   9 files changed, 407 insertions(+)
   create mode 100644 lib/c-snprintf.c
   create mode 100644 lib/c-snprintf.h
   create mode 100644 lib/c-vasnprintf.c
   create mode 100644 lib/c-vasnprintf.h
   create mode 100644 modules/c-snprintf
   create mode 100644 modules/c-snprintf-tests
   create mode 100644 modules/c-vasnprintf
   create mode 100644 tests/test-c-snprintf.c
   create mode 100755 tests/test-c-snprintf.sh
  
  diff --git a/lib/c-snprintf.c b/lib/c-snprintf.c
  new file mode 100644
  index 000..2dadbfc
  --- /dev/null
  +++ b/lib/c-snprintf.c
  @@ -0,0 +1,74 @@
  +/* Formatted output to strings in C locale.
  +   Copyright (C) 2004, 2006-2012 Free Software Foundation, Inc.
  +   Written by Simon Josefsson, Paul Eggert, and Ben Pfaff.
  +
  +   This program is free software; you can redistribute it and/or modify
  +   it under the terms of the GNU General Public License as published by
  +   the Free Software Foundation; either version 3, or (at your option)
  +   any later version.
  +
  +   This program is distributed in the hope that it will be useful,
  +   but WITHOUT ANY WARRANTY; without even the implied warranty of
  +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  +   GNU General Public License for more details.
  +
  +   You should have received a copy of the GNU General Public License 
 along
  +   with this program; if not, see http://www.gnu.org/licenses/.  */
  +
  +#include config.h
  +
  +/* Specification.  */
  +#include stdio.h
  +
  +#include errno.h
  +#include limits.h
  +#include stdarg.h
  +#include stdlib.h
  +#include string.h
  +
  +#include c-vasnprintf.h
  +
  +/* Print formatted output to string STR.  Similar to sprintf, but
  +   additional length SIZE limit how much is written into STR.  Returns
  +   string length of formatted string (which may be larger than SIZE).
  +   STR may be NULL, in which case nothing will be written.  On error,
  +   return a negative value.
  +
  +   Formatting takes place in the C locale, that is, the decimal point
  +   used in floating-point formatting directives is always '.'. */
  +int
  +c_snprintf (char *str, size_t size, const char *format, ...)
  +{
  +  char *output;
  +  size_t len;
  +  size_t lenbuf = size;
  +  va_list args;
  +
  +  va_start (args, format);
  +  output = c_vasnprintf (str, lenbuf, format, args);
  +  len = lenbuf;
  +  va_end (args);
  +
  +  if (!output)
  +return -1;
  +
  +  if (output != str)
  +{
  +  if (size)
  +{
  +  size_t pruned_len = (len  size ? len : size - 1);
  +  memcpy (str, output, pruned_len);
  +  str[pruned_len] = '\0';
  +}
  +
  +  free (output);
  +}
  +
  +  if (INT_MAX  len)
  +{
  +  errno = EOVERFLOW;
  +  return -1;
  +}
  +
  +  return len;
  +}
  diff --git a/lib/c-snprintf.h b/lib/c-snprintf.h
  new file mode 100644
  index 000..3192a2d
  --- /dev/null
  +++ b/lib/c-snprintf.h
  @@ -0,0 +1,46 @@
  +/* vsprintf with automatic memory allocation in C locale.
  +   Copyright (C) 2002-2004, 2007-2012 Free Software Foundation, Inc.
  +
  +   This program is free software; you can redistribute it and/or modify

Re: ASCII_ONLY?

2012-12-07 Thread Ben Pfaff
Bruno Haible br...@clisp.org writes:

 gnulib has three definitions of ASCII_ONLY in files that #include
 vasnprintf.c:
 
 lib/unistdio/u16-vasnprintf.c:#define ASCII_ONLY 1
 lib/unistdio/u32-vasnprintf.c:#define ASCII_ONLY 1
 lib/unistdio/u8-vasnprintf.c:#define ASCII_ONLY 1
 
 But I don't see any actual uses of this macro.  What is the
 intent?  (Is it related to FCHAR_T_ONLY_ASCII?)

 Bingo! The comments in vasnprintf.c say:

  FCHAR_T_ONLY_ASCII Set to 1 to enable verification that all characters
 in the format string are ASCII. MUST be set if
 FCHAR_T and DCHAR_T are not the same type.

 And in lib/unistdio/u{8,16,32}-vasnprintf.c FCHAR_T and DCHAR_T are indeed
 not the same type. Therefore the 3 files should define FCHAR_T_ONLY_ASCII,
 not ASCII_ONLY.

 I would be grateful to you if you could commit the obvious fix.

 Did you find this by code inspection, or through a gcc or clang warning?

I found it by code inspection.  Working on a new user of vasnprintf.c
(see http://lists.gnu.org/archive/html/bug-gnulib/2012-12/msg00022.html),
and examining the existing users, I saw the definition of ASCII_ONLY,
but couldn't figure out what it was good for.

Thanks, Bruno and Paul.



Re: [PATCH 02/11] count-leading-zeros: better 'inline'

2012-10-29 Thread Ben Pfaff
Paul Eggert egg...@cs.ucla.edu writes:

 * lib/count-leading-zeros.c: New file.
 * lib/count-leading-zeros.h (COUNT_LEADING_ZEROS_INLINE):
 New macro.  Replace all uses of 'static inline' with it.
 Use _GL_INLINE_HEADER_BEGIN, _GL_INLINE_HEADER_END.
 * m4/count-leading-zeros.m4 (gl_COUNT_LEADING_ZEROS):
 Do not require AC_C_INLINE.
 * modules/count-leading-zeros (Files, lib_SOURCES):
 Add lib/count-leading-zeros.c.
 (Depends-on): Add extern-inline.

This seems reasonable to me.

Thanks!



Re: [PATCH 02/11] count-leading-zeros: better 'inline'

2012-10-29 Thread Ben Pfaff
Ben Pfaff b...@cs.stanford.edu writes:

 Paul Eggert egg...@cs.ucla.edu writes:

 * lib/count-leading-zeros.c: New file.
 * lib/count-leading-zeros.h (COUNT_LEADING_ZEROS_INLINE):
 New macro.  Replace all uses of 'static inline' with it.
 Use _GL_INLINE_HEADER_BEGIN, _GL_INLINE_HEADER_END.
 * m4/count-leading-zeros.m4 (gl_COUNT_LEADING_ZEROS):
 Do not require AC_C_INLINE.
 * modules/count-leading-zeros (Files, lib_SOURCES):
 Add lib/count-leading-zeros.c.
 (Depends-on): Add extern-inline.

 This seems reasonable to me.

Oops, I meant to say that for count-one-bits (which I maintain),
not count-leading-zeros (which I don't).



Re: [PATCH 03/11] count-one-bits: better 'inline'

2012-10-29 Thread Ben Pfaff
Paul Eggert egg...@cs.ucla.edu writes:

 * lib/count-one-bits.c: New file.
 * lib/count-one-bits.h (COUNT_ONE_BITS_INLINE):
 New macro.  Replace all uses of 'static inline' with it.
 Use _GL_INLINE_HEADER_BEGIN, _GL_INLINE_HEADER_END.
 * m4/count-one-bits.m4 (gl_COUNT_ONE_BITS):
 Do not require AC_C_INLINE.
 * modules/count-one-bits (Files, lib_SOURCES):
 Add lib/count-one-bits.c.
 (Depends-on): Add extern-inline.

This seems reasonable to me.  Thanks!




Re: avoiding 'static inline'

2012-08-19 Thread Ben Pfaff
Paul Eggert egg...@cs.ucla.edu writes:

 On 08/18/2012 10:04 PM, Ben Pfaff wrote:
 I see that some of your patches remove inline from some static
 inline functions in header files, but I don't see anything
 added, such as __attribute__((unused)), that would suppress an
 unused static function warning.  (Perhaps I just missed it.)

 Thanks for bringing that up.  I didn't observe those warnings
 even when configuring with --enable-gcc-warnings.  I just now
 looked into matter and it turns out that the warnings are
 suppressed by the #pragma GCC system_header at the start of
 all the files that contain the patches you're referring to.
 So we should be OK.

Thanks for checking into this.



Re: avoiding 'static inline'

2012-08-18 Thread Ben Pfaff
Paul Eggert egg...@cs.ucla.edu writes:

 I recently added a bunch of static inline functions to Emacs,
 and just for fun I tried changing them to plain 'static' to
 see how much that would hurt performance.  To my surprise, it
 made Emacs 14% faster on my standard Lisp benchmark (taking the nth
 element of a long list).  It also shrank the size of the text segment
 by 1.7%, which was not as much of a surprise.

 Clearly I've been using 'static inline' too much.  Come to think of
 it, these days there's little need for 'static inline' as opposed to
 'static', since GCC and other modern compilers will inline 'static'
 functions for you.  And apparently they do a better job if they're not
 given bad advice by people who use the 'inline' keyword.

 I'd like to install some changes to gnulib, so that its code does
 not use 'static inline' in the modules that Emacs uses.  This is part
 of a similar, larger change to Emacs proper.  These patches won't
 affect extern inline functions, just static ones.

With GCC, adding inline to static has the side effect of
suppressing unused static function warnings.  For static
inline functions in header files in particular, that's an
important side effect, since most C source files that #include a
header won't have a call to every static inline function in the
header.

I see that some of your patches remove inline from some static
inline functions in header files, but I don't see anything
added, such as __attribute__((unused)), that would suppress an
unused static function warning.  (Perhaps I just missed it.)
Did you consider this issue?

Thanks,

Ben.



Re: [PATCH] count-leading-zeros: new module

2012-08-10 Thread Ben Pfaff
Eric Blake ebl...@redhat.com writes:

 +/* Expand the code which computes the number of leading zeros of the local
 +   variable 'x' of type TYPE (an unsigned integer type) and returns it
 +   from the current function.  */
 +#if 0  __GNUC__  3 || (__GNUC__ == 3  __GNUC_MINOR__ = 4)

Do you really want 0  there?



Re: gnulib-tool and --with-tests

2012-06-21 Thread Ben Pfaff
Bruno Haible br...@clisp.org writes:

 What do you think of making --with-tests a no-op, and add a
 --without-tests to disable it?  This would save us all some typing...

 You mean to make this change only in combination with --test,
 --create-testdir, and --create-megatestdir? Not with --import.

 I'm undecided. What do the others think?

I've always found it surprising that creating a test directory
didn't include the tests.  I remember to use --with-tests these
days, but I think that having that be the default would be less
surprising.



Re: isnanl, printf, and non-IEEE values

2012-06-18 Thread Ben Pfaff
Bruno Haible br...@clisp.org writes:

 In theory you would be right that data should be validated at the boundaries
 of the program, that is, when they are read from outside sources. But no
 program I know of does this for unconstrained floating-point numbers.

That's an interesting point.  GNU PSPP reads unconstrained
doubles from SPSS data files without validating them[*], since
it never occurred to me that this was a bad idea.  A function to
validate a floating-point number would therefore be useful in
PSPP.

[*] Except in the case where it detects that the data file uses a
nonnative floating point format such as one of the old IBM or
DEC floating-point formats, in which case it does a format
conversion.



Re: isnanl, printf, and non-IEEE values

2012-06-18 Thread Ben Pfaff
Rich Felker dal...@aerifal.cx writes:

 On Mon, Jun 18, 2012 at 10:21:34AM -0700, Ben Pfaff wrote:
 Bruno Haible br...@clisp.org writes:
 
  In theory you would be right that data should be validated at the 
  boundaries
  of the program, that is, when they are read from outside sources. But no
  program I know of does this for unconstrained floating-point numbers.
 
 That's an interesting point.  GNU PSPP reads unconstrained
 doubles from SPSS data files without validating them[*], since

 This is not a problem. As long as you're assuming IEEE, all 64-bit
 patterns are valid long double values. The issue only occurs for long
 double, a type that's very different in both arithmetic properties and
 representation between systems. On typical systems, it has padding
 bits (which might or might not be required to be all-zero) as well as
 bit combinations that are completely invalid.

OK, thanks for the information.  Never mind, then.



Re: gnulib portability issues

2012-06-12 Thread Ben Pfaff
Eric Blake ebl...@redhat.com writes:

 Wrong.  Pretty much every libc out there lets you ungetc() more than one
 byte.

Does that include glibc?  Then there is a bug in the manual,
which says:

The GNU C library only supports one character of
pushback—in other words, it does not work to call ungetc
twice without doing input in between.

in the description of ungetc().



  1   2   3   4   >