Re: functions with empty parameter lists

[Paul Eggert]

On 2/6/23 14:48, Bruno Haible wrote:

   * In the long term, assuming ISO C 23 and newer, writing (void) is just
 clutter in those places where one can just as well write (). The keyword
 'void' here only tells that the programmer has accomplished the migration
 from K C to ISO C 23, nothing else.

   * We can assume that the readers and contributors of our code know the
 difference between a function declaration/type and a function definition.
 Therefore, I'm in favour of turning all (void) in function*definitions*
 to ().
 This can happen now or any time.


This all makes sense, yes.



   * We should stop compiling with -Wstrict-prototypes and instead (not always,
 but frequently enough) compile with the '-std=gnu23' option. Clang
 currently implements it better. GCC 13 may be on par with clang again on
 this topic [1].


We could have 'configure' check -Wstrict-prototypes specially. If the 
compiler complains about the abovementioned style, 'configure' would 
omit -Wstrict-prototypes; otherwise it could keep it. The idea is that 
eventually GCC will be smart enough so that 'gcc -Wstrict-prototypes' 
will do the right thing even when not in C23 mode.


I filed a GCC bug report about this:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108690



   * We need to continue to use (void) in function declarations/types, as long
 as we support compilers for ISO C standards before C 23. This will
 probably take 10 or 15 years.


Alternatively we could define a macro NOARGS that expands to nothing in 
C23 and later, and to void otherwise. This might be more trouble than 
it's worth, though.

Re: Gnulib and nullptr

[Paul Eggert]

On 2/6/23 15:16, Bruno Haible wrote:


Ignoring or underestimating the "barrier to entry" issue
was my biggest mistake in GNU clisp development so far.


Fair enough, but surely there are differences between the clisp and 
Gnulib cases. In C++ there are more advantages to nullptr (so more of a 
pressure to use it) than in C. As C++ is popular, many potential 
contributors will be used to nullptr being better; for these people, 
using NULL will be a barrier to entry.



Thus, it is better to have a rule (at least per project). Any of the
two rules
   - "Avoid NULL, write nullptr always."
   - "In varargs calls, write nullptr instead of  (foo *) NULL."
is better than a random mix of NULL and nullptr in the same project.


In diffutils proper I switched to the former rule (though code imported 
from Gnulib still uses NULL so people that build from tarballs will 
still see a mixture). I plan to try things out this way for a while, to 
see whether there are problems with the Gnulib nullptr implementation. 
We can convert more intensely-maintained projects later, depending on 
diffutils goes.


A couple of other things.

1. Since the nullptr issue affects C++ so much (something I wasn't aware 
of until now), I'm inclined to rename the module from c-nullptr to 
nullptr as was hinted earlier. It's not just C so the "c-" is a bit 
misleading.


2. The issues you mentioned about C++ compilers possibly not supporting 
nullptr seem to be serious enough that it seems that we should test the 
C++ compiler at configure time, as we already test the C compiler. That 
way we wouldn't have to worry whether our __cplusplus-related #ifdefs 
are OK.

Re: [g.branden.robin...@gmail.com: macOS 12.6.3 and vasnprintf compilation failure]

[Bjarni Ingi Gislason]

See https://savannah.gnu.org/bugs/index.php?63078

  I renamed the file "groff/src/include/assert.h" in my privat branch
of groff to ".../assert.h.original".

Re: Differences between "module license" and "file license"

[Bruno Haible]

Paul,

Did you intend to put the 'alignalloc' module under GPL or LGPLv2+,
when adding it on 2022-01-23?

Bjarni Ingi Gislason wrote:
> Module LicenseFile License   File name
> = == =
> LGPLv2+   GPLlib/alignalloc.h
> LGPLv2+   GPLlib/alignalloc.c

Re: [g.branden.robin...@gmail.com: macOS 12.6.3 and vasnprintf compilation failure]

[Bruno Haible]

Hi Branden,

> A problem immediately arose on macOS 12.6.3.  It's our good friend
> vasnprintf again.  Logs are available on Savannah[2].
> 
> lib/vasnprintf.c:411:16: error: expected parameter declarator
> static_assert (sizeof (mp_limb_t) * CHAR_BIT == GMP_LIMB_BITS);
>^
> lib/vasnprintf.c:411:16: error: expected ')'
> lib/vasnprintf.c:411:15: note: to match this '('
> static_assert (sizeof (mp_limb_t) * CHAR_BIT == GMP_LIMB_BITS);
>   ^
> lib/vasnprintf.c:411:1: warning: type specifier missing, defaults to 'int' 
> [-Wimplicit-int]
> static_assert (sizeof (mp_limb_t) * CHAR_BIT == GMP_LIMB_BITS);
> ^
> lib/vasnprintf.c:415:16: error: expected parameter declarator
> static_assert (sizeof (mp_twolimb_t) * CHAR_BIT == GMP_TWOLIMB_BITS);
>^
> lib/vasnprintf.c:415:16: error: expected ')'
> lib/vasnprintf.c:415:15: note: to match this '('
> static_assert (sizeof (mp_twolimb_t) * CHAR_BIT == GMP_TWOLIMB_BITS);
>   ^
> lib/vasnprintf.c:415:1: warning: type specifier missing, defaults to 'int' 
> [-Wimplicit-int]
> static_assert (sizeof (mp_twolimb_t) * CHAR_BIT == GMP_TWOLIMB_BITS);
> ^

I can reproduce the issue on a macOS 12.5 machine (gcc104.fsffrance.org —
you can surely get an account there too).

The command
  $ make lib/vasnprintf.o V=1
shows me the compiler command line that failed:
depbase=`echo lib/vasnprintf.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
cc -DHAVE_CONFIG_H -I. -I.. -I./src/include  -I../src/include -I../lib 
-I./src/include -I./lib -I/Users/haible/include -Wall  -g -O2 -MT 
lib/vasnprintf.o -MD -MP -MF $depbase.Tpo -c -o lib/vasnprintf.o 
../lib/vasnprintf.c &&\
mv -f $depbase.Tpo $depbase.Po
...

Take this command, remove the -c and -o options, and instead add -E:

  $ cc -DHAVE_CONFIG_H -I. -I.. -I./src/include  -I../src/include -I../lib 
-I./src/include -I./lib -I/Users/haible/include -Wall  -g -O2 
../lib/vasnprintf.c -E > i

Since it looks like the 'static_assert' macro is not defined, and it
ought to be defined by  via , two questions arise:

1) Does  contain the boilerplate for including ?
   config.h is found in src/include/config.h.
   Inspection shows: Yes, it does.

2) Does  contain the boilerplate for defining 'static_assert'?
   Here's the problem: There are two assert.h files:
 $(builddir)/lib/assert.hgnulib generated, contains the #define 
static_assert
 $(srcdir)/src/include/assert.h  groff-owned

in the newest ISO C 23 standard is supposed to make
   'static_assert' available. [1]

   The problem is a combination of:

 - The groff-owned $(srcdir)/src/include/assert.h takes precedence
   over the one in $(builddir)/lib — due to the particular order of the
   -I options. This can be seen by looking into the 'i' file.

 - The groff-owned $(srcdir)/src/include/assert.h does not use
   #include_next ; this would include the next ,
   namely $(builddir)/lib/assert.h.

 - The groff-owned $(srcdir)/src/include/assert.h does not define
   'static_assert' by itself.

   So, to fix things, you need to either
 - change the -I options order, or
 - use #include_next  if the compiler supports #include_next,
   otherwise #include , or
 - implement your own logic for defining 'static_assert' in C.

   Note: If you go by the first option, you may still get problems on
   those systems where /usr/include/assert.h defines 'static_assert' and
   thus gnulib's $(builddir)/lib/assert.h is just a wrapper without much
   added value. Namely, the lack of #include_next  in groff's
   assert.h will prevent /usr/include/assert.h from being read.

Hope this helps. This is not a complete solution, but you get some ideas.

Bruno

[1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3047.pdf

Differences between "module license" and "file license"

[Bjarni Ingi Gislason]

Debian testing (bookworm/sid)

  Compiling in the "build" directory:

/usr/local/bin/make
GNU Make 4.4.0.90
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
make check
Module LicenseFile License   File name
= == =
LGPLv2+   GPLlib/alignalloc.h
LGPLv2+   GPLlib/alignalloc.c
GPL   LGPLv2+lib/glob.c
GPL   LGPLv2+lib/glob_internal.h
GPL   LGPLv2+lib/glob_pattern_p.c
GPL   LGPLv2+lib/globfree.c
LGPLv2+   LGPLv3+ or GPLv2+ lib/uniwidth/width0.h
LGPLv2+   LGPLv3+ or GPLv2+ lib/uniwidth/width2.h
make: *** [Makefile:142: sc_check_copyright] Error 1



./check-copyright shows in the main (master) directory:

Module LicenseFile License   File name
= == =
LGPLv2+   GPLlib/alignalloc.h
LGPLv2+   GPLlib/alignalloc.c
GPL   LGPLv2+lib/glob.c
GPL   LGPLv2+lib/glob_internal.h
GPL   LGPLv2+lib/glob_pattern_p.c
GPL   LGPLv2+lib/globfree.c
LGPLv2+   LGPLv3+ or GPLv2+ lib/uniwidth/width0.h
LGPLv2+   LGPLv3+ or GPLv2+ lib/uniwidth/width2.h

with return status 1.

Re: Gnulib and nullptr

[Bruno Haible]

Paul Eggert wrote:
> >   The diffutils patch shows that the possible bugs had already been caught
> 
> Sure, but the idea is to make such bugs less likely in the future, by 
> encouraging the use of nullptr now.

The gain here is small (it's only varargs and contrived cases), whereas ...

> > 2) We should avoid gratuitous style differences between code in Gnulib and
> > general coding habits in the community, because that increases the barrier
> > to entry and confuses newcomers.
> 
> This barrier is so small as to not be worth worrying about.

I disagree here. Ignoring or underestimating the "barrier to entry" issue
was my biggest mistake in GNU clisp development so far. Even such simple
things tend to turn contributors off.

> People 
> accustomed to NULL can still submit patches containing NULL, and we can 
> accept them.

This would be even worse, because it would lead to a random mix of code
with NULL and code with nullptr. Each time programmers have the choice
between two equivalent ways of coding something, it has three negative
effects:
  * They spend time reflecting whether they should use one or the other.
Since the two are equivalent, this is wasted time.
  * They get attached to doing it one way, developing a personal style.
And then they change past contributions by other developers, because
they get annoyed or even angry about the other style.
  * When searching for some code by pattern or idiom, a developer may
need two 'grep' runs instead of one.

We've seen this happen
  - with the indentation width and braces style, before the GCS were
invented,
  - with spaces indentation vs. mixed tab/spaces indentation,
  - in Common Lisp, with SETQ vs. SETF,
  - in C++, with many other constructs. Last time I counted, there were
12 different ways to define a function in C++. And the programmers
spend a lot of time deliberating which of them to choose in each
particular situation. What a waste of time!

Thus, it is better to have a rule (at least per project). Any of the
two rules
  - "Avoid NULL, write nullptr always."
  - "In varargs calls, write nullptr instead of  (foo *) NULL."
is better than a random mix of NULL and nullptr in the same project.

Bruno

Re: functions with empty parameter lists

[Bruno Haible]

Paul Eggert wrote:
>  static void
> -init_sh_quoting_options ()
> +init_sh_quoting_options (void)
>  {

Your change prompted me to wonder whether this patch should be generalized,
that is, whether all function definitions with empty parameter lists should
be changed to use this style. Like in the attached patch.

I got doubts because
  * I'm using the style with () in function definitions in many places, and
neither 'gcc -Wall' nor 'clang -Wall' has warned about it, in many years.
  * The ISO C committee attempts to align C with C++ (e.g. regarding the
attributes syntax), and in C++ a function declaration or definition with
() denotes zero arguments.

There were two essential changes in this area in ISO C 23:
  * https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2432.pdf
1) removed the K C syntax for function definitions
 int foo (x, y) int x; long y; { ... }
2) in function definitions, () is equivalent to (void).
  * https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2841.htm
3) A function declarator with a parameter list of () declares a prototype
   for a function that takes no parameters (like it does in C++).
   See ISO C 23 § 6.7.6.3.(13).

For those of us that are not standards experts, the effects can be seen
through two test cases, with clang 15.0.6.

=== foo1.c ===
#include 

void func();

int main() {
func("AAA");
return 0;
}

void func() {
printf("in func()\n");
}
=== foo2.c ===
#include 

void func() {
printf("in func()\n");
}

int main() {
func("AAA");
return 0;
}

==

clang diagnostics for older standard versions:

$ clang -Wall foo1.c
foo1.c:6:9: warning: passing arguments to 'func' without a prototype is 
deprecated in all versions of C and is not supported in C2x 
[-Wdeprecated-non-prototype]
func("AAA");
^
1 warning generated.
$ clang -Wall foo2.c
foo2.c:8:15: warning: too many arguments in call to 'func'
func("AAA");
  ^
foo2.c:8:9: warning: passing arguments to 'func' without a prototype is 
deprecated in all versions of C and is not supported in C2x 
[-Wdeprecated-non-prototype]
func("AAA");
^
2 warnings generated.

clang diagnostics for C23:

$ clang -std=gnu2x -Wall foo1.c
foo1.c:6:10: error: too many arguments to function call, expected 0, have 1
func("AAA");
 ^
foo1.c:3:6: note: 'func' declared here
void func();
 ^
1 error generated.
$ clang -std=gnu2x -Wall foo2.c
foo2.c:8:10: error: too many arguments to function call, expected 0, have 1
func("AAA");
 ^
foo2.c:3:6: note: 'func' declared here
void func() {
 ^
1 error generated.


What does this mean?

(I) In function *definitions*, () and (void) mean the same: zero arguments.
This was always the case, in C89, in K C, and equally in C++.

(II) Regarding function *declarations*, this gives a whole new view of the
 history.

 * In C23, () and (void) mean the same: zero arguments. It's like this
   in C++ as well, for many years.

 * The (void) syntax was only introduced as a temporary workaround,
   as long as - in C only, not in C++ - () in function declarations meant
   a varargs list: (...).

 * The GCC warning -Wstrict-prototypes is a migration aid, whose purpose
   is to remind the programmer to convert declarations such as
 void func ();
   to either
 void func (void);
   or
 void func (first_t arg, ...);

   Unfortunately, this GCC warning also warns about function definitions
 void func () { ... }
   although there was never a need to warn about these; see (I) above.

(III) So, why is (void) allowed in function *definitions* at all?
  I can only guess:
- Maybe the future standardization path of this area was not clear
  in 1989. (For instance, C++ did not exist at that time, and nowadays
  ISO C is following ISO C++ in some aspects.)
- There was the desire to have the same syntax across function
  declarations/types and function definitions.

Now, how should we go ahead in this situation? This is my opinion:

  * In the long term, assuming ISO C 23 and newer, writing (void) is just
clutter in those places where one can just as well write (). The keyword
'void' here only tells that the programmer has accomplished the migration
from K C to ISO C 23, nothing else.

  * We can assume that the readers and contributors of our code know the
difference between a function declaration/type and a function definition.
Therefore, I'm in favour of turning all (void) in function *definitions*
to ().
This can happen now or any time.

  * We should stop compiling with -Wstrict-prototypes and instead (not always,
but frequently enough) compile 

Re: Gnulib and nullptr

[Dmitrii Pasechnik]

On Mon, Feb 06, 2023 at 04:22:46PM -0500, Jeffrey Walton wrote:
> On Sun, Feb 5, 2023 at 9:45 PM Paul Eggert  wrote:
> >
> > On 2023-02-05 18:00, Bruno Haible wrote:
> > > Why call it 'c-nullptr', not 'nullptr'?
> >
> > I was worried about C++, not that I know much about it, and operated by
> > analogy with the name of m4/c-bool.m4. If 'nullptr' is a better name
> > then let's switch to it. I assume we'd also switch the file names, the
> > macro names, etc.
> 
> In C++, nullptr is not convertible to an integral. So this no longer
> causes confusion:
> 
> g(void*);
> g(int);
> 
> g(nullptr) will always match g(void*). g(int) will never be matched,
> which could happen with g(NULL).

As compilers C++ compilers get stricter, more and more old C++ code
breaks due to NULL being of wrong type. I did quite a bit of patching
of such code replacing NULL with nullptr in the last few years.

Dima

Re: Gnulib and nullptr

[Jeffrey Walton]

On Sun, Feb 5, 2023 at 9:45 PM Paul Eggert  wrote:
>
> On 2023-02-05 18:00, Bruno Haible wrote:
> > Why call it 'c-nullptr', not 'nullptr'?
>
> I was worried about C++, not that I know much about it, and operated by
> analogy with the name of m4/c-bool.m4. If 'nullptr' is a better name
> then let's switch to it. I assume we'd also switch the file names, the
> macro names, etc.

In C++, nullptr is not convertible to an integral. So this no longer
causes confusion:

g(void*);
g(int);

g(nullptr) will always match g(void*). g(int) will never be matched,
which could happen with g(NULL).

Jeff

Re: Gnulib and nullptr

[Paul Eggert]

On 2/5/23 16:57, Bruno Haible wrote:


  The diffutils patch shows that the possible bugs had already been caught


Sure, but the idea is to make such bugs less likely in the future, by 
encouraging the use of nullptr now.



  The last platform in which pointers and integers were passed differently
  as function arguments were m68k machines, around 1990.


As I understand it they can still be passed differently in IBM i 7.5, 
released 2022-05-10, and I expect (though I haven't checked) that some 
of that platform's null pointer representations are not all bits zero.

Of course this is not one of Gnulib's supported targets.



The only argument in favour of 'nullptr' in C that I see is that it has some
use in C++


It's not just avoiding varargs bug (which is an advantage on some 
platforms). Also, erroneous code like this:


   #if _AIX || __HAIKU__
 exit (i < j ? j - i : nullptr);
   #endif

will be caught at compile-time on AIX with a proper nullptr, whereas if 
the code uses NULL instead of nullptr we won't get diagnostics until we 
compile on Haiku as well. Although a contrived example, this sort of 
thing can be an advantage when compiling code intended for these 
rarely-used platforms.




2) We should avoid gratuitous style differences between code in Gnulib and
general coding habits in the community, because that increases the barrier
to entry and confuses newcomers.


This barrier is so small as to not be worth worrying about. People 
accustomed to NULL can still submit patches containing NULL, and we can 
accept them. And readers won't be significantly confused by seeing the 
newer "nullptr" style in the code, or by seeing it highlighted 
incorrectly by older Emacs.


NULL is a tricky anachronism. I'm willing to switch to nullptr 
downstream and be the guinea pig, if you'd rather have Gnulib be 
cautious about this. But there seems little reason to hang on to NULL 
indefinitely.

Re: [PATCH] Do not decorate symbols as dllexport on Cygwin

[Reuben Thomas]

On Mon, 6 Feb 2023 at 20:38, Bruno Haible  wrote:

>   1. 'recode' is updated to a current gnulib and publish a corresponding
>  tarball. (Hi Reuben :) )
>

I've updated gnulib in recode git; I'd be grateful if I could have a report
that it's doing what's needed here before I make a release, if possible!

Git at: https://github.com/rrthomas/recode.git

-- 
https://rrt.sc3d.org

Re: [PATCH] Do not decorate symbols as dllexport on Cygwin

[Corinna Vinschen]

On Feb  6 21:38, Bruno Haible wrote:
> Corinna Vinschen wrote:
> > I just ran the setlocale_null-mt-* tests with gnulib from git master
> > under Cygwin with my newlib patch, and the tests succeeded.  I checked
> > that configure is doing the right thing and config.h contains the right
> > settings:
> > 
> >   $ ./configure
> >   [...]
> >   checking whether setlocale (LC_ALL, NULL) is multithread-safe... yes
> >   checking whether setlocale (category, NULL) is multithread-safe... yes
> >   [...]
> >   $ grep SETLOCALE_NULL config.h
> >   #define GNULIB_TEST_SETLOCALE_NULL 1
> >   #define SETLOCALE_NULL_ALL_MTSAFE 1
> >   #define SETLOCALE_NULL_ONE_MTSAFE 1
> >   $ grep SETLOCALE_NULL gltests/config.h
> >   #define GNULIB_TEST_SETLOCALE_NULL 1
> >   #define SETLOCALE_NULL_ALL_MTSAFE 1
> >   #define SETLOCALE_NULL_ONE_MTSAFE 1
> 
> Excellent! Thank you.
> 
> The lasts remaining steps, to resolve the issue, are (as far as I see it):
>   1. 'recode' is updated to a current gnulib and publish a corresponding
>  tarball. (Hi Reuben :) )
>   2. Then the Cygwin program packagers build this tarball on a Cygwin of
>  version ≥ 3.4.6.

Incidentally I'm the maintainer of the recode package in the Cygwin distro
and I built 3.7.12 (as in Fedora 37) with -Wl,--export-all-symbols, which
also does the trick for now.


Thanks,
Corinna

Re: [PATCH] Do not decorate symbols as dllexport on Cygwin

[Bruno Haible]

Corinna Vinschen wrote:
> I just ran the setlocale_null-mt-* tests with gnulib from git master
> under Cygwin with my newlib patch, and the tests succeeded.  I checked
> that configure is doing the right thing and config.h contains the right
> settings:
> 
>   $ ./configure
>   [...]
>   checking whether setlocale (LC_ALL, NULL) is multithread-safe... yes
>   checking whether setlocale (category, NULL) is multithread-safe... yes
>   [...]
>   $ grep SETLOCALE_NULL config.h
>   #define GNULIB_TEST_SETLOCALE_NULL 1
>   #define SETLOCALE_NULL_ALL_MTSAFE 1
>   #define SETLOCALE_NULL_ONE_MTSAFE 1
>   $ grep SETLOCALE_NULL gltests/config.h
>   #define GNULIB_TEST_SETLOCALE_NULL 1
>   #define SETLOCALE_NULL_ALL_MTSAFE 1
>   #define SETLOCALE_NULL_ONE_MTSAFE 1

Excellent! Thank you.

The lasts remaining steps, to resolve the issue, are (as far as I see it):
  1. 'recode' is updated to a current gnulib and publish a corresponding
 tarball. (Hi Reuben :) )
  2. Then the Cygwin program packagers build this tarball on a Cygwin of
 version ≥ 3.4.6.

Bruno

Re: [PATCH] Do not decorate symbols as dllexport on Cygwin

[Corinna Vinschen]

On Feb  6 18:37, Bruno Haible wrote:
> Corinna Vinschen wrote:
> > This patch will be in the next Cygwin release 3.4.6.
> 
> Thanks!
> 
> > I'm just a bit fuzzy what patches will be required for gnulib now...
> 
> With this patch, the setlocale_null lock should be gone in Cygwin >= 3.4.6.
> I can't really test it, but I hope the patch is correct.

I just ran the setlocale_null-mt-* tests with gnulib from git master
under Cygwin with my newlib patch, and the tests succeeded.  I checked
that configure is doing the right thing and config.h contains the right
settings:

  $ ./configure
  [...]
  checking whether setlocale (LC_ALL, NULL) is multithread-safe... yes
  checking whether setlocale (category, NULL) is multithread-safe... yes
  [...]
  $ grep SETLOCALE_NULL config.h
  #define GNULIB_TEST_SETLOCALE_NULL 1
  #define SETLOCALE_NULL_ALL_MTSAFE 1
  #define SETLOCALE_NULL_ONE_MTSAFE 1
  $ grep SETLOCALE_NULL gltests/config.h
  #define GNULIB_TEST_SETLOCALE_NULL 1
  #define SETLOCALE_NULL_ALL_MTSAFE 1
  #define SETLOCALE_NULL_ONE_MTSAFE 1


Thanks a lot,
Corinna



> 
> 
> 2023-02-06  Bruno Haible  
> 
>   setlocale-null: Don't use a lock in Cygwin >= 3.4.6.
>   Road paved by Corinna Vinschen .
>   * m4/setlocale_null.m4 (gl_FUNC_SETLOCALE_NULL): Assume that
>   setlocale (LC_ALL, NULL) is multithread-safe in Cygwin >= 3.4.6.
>   * lib/setlocale_null.c: Update comments.
>   * tests/test-setlocale_null-mt-all.c: Likewise.
> 
> diff --git a/lib/setlocale_null.c b/lib/setlocale_null.c
> index 6ac563db14..89c8a06598 100644
> --- a/lib/setlocale_null.c
> +++ b/lib/setlocale_null.c
> @@ -173,7 +173,7 @@ setlocale_null_unlocked (int category, char *buf, size_t 
> bufsize)
>  #endif
>  }
>  
> -#if !(SETLOCALE_NULL_ALL_MTSAFE && SETLOCALE_NULL_ONE_MTSAFE) /* musl libc, 
> macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku, Cygwin */
> +#if !(SETLOCALE_NULL_ALL_MTSAFE && SETLOCALE_NULL_ONE_MTSAFE) /* musl libc, 
> macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku, Cygwin < 3.4.6 */
>  
>  /* Use a lock, so that no two threads can invoke setlocale_null_unlocked
> at the same time.  */
> @@ -198,7 +198,7 @@ setlocale_null_with_lock (int category, char *buf, size_t 
> bufsize)
>return ret;
>  }
>  
> -# elif HAVE_PTHREAD_API /* musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, 
> Haiku, Cygwin */
> +# elif HAVE_PTHREAD_API /* musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, 
> Haiku, Cygwin < 3.4.6 */
>  
>  extern
>  #  if defined _WIN32 || defined __CYGWIN__
> diff --git a/m4/setlocale_null.m4 b/m4/setlocale_null.m4
> index dd6a5ef538..b41df499a8 100644
> --- a/m4/setlocale_null.m4
> +++ b/m4/setlocale_null.m4
> @@ -1,4 +1,4 @@
> -# setlocale_null.m4 serial 6
> +# setlocale_null.m4 serial 7
>  dnl Copyright (C) 2019-2023 Free Software Foundation, Inc.
>  dnl This file is free software; the Free Software Foundation
>  dnl gives unlimited permission to copy and/or distribute it,
> @@ -13,9 +13,23 @@ AC_DEFUN([gl_FUNC_SETLOCALE_NULL],
>AC_CACHE_CHECK([whether setlocale (LC_ALL, NULL) is multithread-safe],
>  [gl_cv_func_setlocale_null_all_mtsafe],
>  [case "$host_os" in
> -   # Guess no on musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku, 
> Cygwin.
> -   *-musl* | darwin* | freebsd* | midnightbsd* | netbsd* | openbsd* | 
> aix* | haiku* | cygwin*)
> +   # Guess no on musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku.
> +   *-musl* | darwin* | freebsd* | midnightbsd* | netbsd* | openbsd* | 
> aix* | haiku*)
>   gl_cv_func_setlocale_null_all_mtsafe=no ;;
> +   # Guess no on Cygwin < 3.4.6.
> +   cygwin*)
> + AC_EGREP_CPP([Lucky user],
> +   [
> +#if defined __CYGWIN__
> + #include 
> + #if CYGWIN_VERSION_DLL_COMBINED >= CYGWIN_VERSION_DLL_MAKE_COMBINED (3004, 
> 6)
> +  Lucky user
> + #endif
> +#endif
> +  ],
> +  [gl_cv_func_setlocale_null_all_mtsafe=yes],
> +  [gl_cv_func_setlocale_null_all_mtsafe=no])
> +;;
> # Guess yes on glibc, HP-UX, IRIX, Solaris, native Windows.
> *-gnu* | gnu* | hpux* | irix* | solaris* | mingw*)
>   gl_cv_func_setlocale_null_all_mtsafe=yes ;;
> diff --git a/tests/test-setlocale_null-mt-all.c 
> b/tests/test-setlocale_null-mt-all.c
> index 6036c260cd..7480406639 100644
> --- a/tests/test-setlocale_null-mt-all.c
> +++ b/tests/test-setlocale_null-mt-all.c
> @@ -166,7 +166,7 @@ Solaris 11.0 OK
>  Solaris 11.4 OK
>  Solaris OpenIndiana  OK
>  Haikucrash < 1 sec
> -Cygwin   crash < 1 sec
> +Cygwin < 3.4.6   crash < 1 sec
>  mingwOK
>  MSVC OK (assuming compiler option /MD !)
>  */
> 
> 

Re: FAIL in diffutils/gnulib-tests

[Bruno Haible]

Bjarni Ingi Gislason wrote:
> On Mon, Feb 06, 2023 at 02:28:03AM +0100, Bruno Haible wrote:
> > Bjarni Ingi Gislason wrote:
> > >GNU diffutils 3.9.12-c05c: gnulib-tests/test-suite.log
> > 
> > Please try again with the tarball that you can download from
> > https://gitlab.com/gnu-diffutils/ci-distcheck
> > (full URL: 
> > https://gitlab.com/gnu-diffutils/ci-distcheck/-/jobs/artifacts/main/raw/diffutils-snapshot.tar?job=check-optimized
> >  ).
> > 
> [...]
> 
> Debian testing (bookworm/sid)
> 
> gcc (Debian 12.2.0-14) 12.2.0
> 
> GNU Make 4.4.0.90
> 
> 
>   Differences between latest git repository and the tar file
> "diffutils-2023-01-31".
> 
>   Similar text is shown on the terminal in both cases:
> 
> traps: test-c-stack[287115] trap invalid opcode ip:56214e4522af
> sp:7ffde48af170 error:0 in test-c-stack[56214e452000+2000]
> 
> traps: test-sigsegv-ca[288174] trap invalid opcode ip:55f3cc5d047a
> sp:7ffe4f87a9c0 error:0 in
> test-sigsegv-catch-stackoverflow2[55f3cc5d+2000]
> 
>   The only difference concerning FAIL that I see is '>>' versus '>'.
> 
> -: from the git repository
> 
> +: the snapshot (tar file)
> 
> 
>  PASS: test-locale
> -../../build-aux/test-driver: line 112: 268125 Aborted
> (core dumped) "$@" >> "$log_file" 2>&1
> +../../build-aux/test-driver: line 107: 287758 Aborted
> (core dumped) "$@" > $log_file 2>&1
>  FAIL: test-localeconv
>  PASS: test-lstat
>  PASS: test-malloc-gnu
> @@ -1734,7 +1734,7 @@ PASS: test-sigprocmask
>  PASS: test-sigsegv-catch-segv1
>  PASS: test-sigsegv-catch-segv2
>  PASS: test-sigsegv-catch-stackoverflow1
> -../../build-aux/test-driver: line 112: 268554 Illegal instruction
> (core dumped) "$@" >> "$log_file" 2>&1
> +../../build-aux/test-driver: line 107: 288174 Illegal instruction
> (core dumped) "$@" > $log_file 2>&1
>  FAIL: test-sigsegv-catch-stackoverflow2
>  PASS: test-sleep

So, for you, this diffutils-2023-01-31 tarball fails
  - the stack overflow tests,
  - the localeconv test.

Where for me, on Debian testing (bookworm), downloaded from
https://cdimage.debian.org/cdimage/weekly-builds/amd64/iso-dvd/
with timestamp 2023-01-23, all tests pass.

This indicates that you are hitting bugs in this Debian "testing"
distro, that are already fixed.

Nothing to do in Gnulib, since it's normal for bugs to appear and
disappear in a "testing" (= alpha quality) distro.

Bruno

Re: [PATCH] Do not decorate symbols as dllexport on Cygwin

[Bruno Haible]

Corinna Vinschen wrote:
> This patch will be in the next Cygwin release 3.4.6.

Thanks!

> I'm just a bit fuzzy what patches will be required for gnulib now...

With this patch, the setlocale_null lock should be gone in Cygwin >= 3.4.6.
I can't really test it, but I hope the patch is correct.


2023-02-06  Bruno Haible  

setlocale-null: Don't use a lock in Cygwin >= 3.4.6.
Road paved by Corinna Vinschen .
* m4/setlocale_null.m4 (gl_FUNC_SETLOCALE_NULL): Assume that
setlocale (LC_ALL, NULL) is multithread-safe in Cygwin >= 3.4.6.
* lib/setlocale_null.c: Update comments.
* tests/test-setlocale_null-mt-all.c: Likewise.

diff --git a/lib/setlocale_null.c b/lib/setlocale_null.c
index 6ac563db14..89c8a06598 100644
--- a/lib/setlocale_null.c
+++ b/lib/setlocale_null.c
@@ -173,7 +173,7 @@ setlocale_null_unlocked (int category, char *buf, size_t 
bufsize)
 #endif
 }
 
-#if !(SETLOCALE_NULL_ALL_MTSAFE && SETLOCALE_NULL_ONE_MTSAFE) /* musl libc, 
macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku, Cygwin */
+#if !(SETLOCALE_NULL_ALL_MTSAFE && SETLOCALE_NULL_ONE_MTSAFE) /* musl libc, 
macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku, Cygwin < 3.4.6 */
 
 /* Use a lock, so that no two threads can invoke setlocale_null_unlocked
at the same time.  */
@@ -198,7 +198,7 @@ setlocale_null_with_lock (int category, char *buf, size_t 
bufsize)
   return ret;
 }
 
-# elif HAVE_PTHREAD_API /* musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, 
Haiku, Cygwin */
+# elif HAVE_PTHREAD_API /* musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, 
Haiku, Cygwin < 3.4.6 */
 
 extern
 #  if defined _WIN32 || defined __CYGWIN__
diff --git a/m4/setlocale_null.m4 b/m4/setlocale_null.m4
index dd6a5ef538..b41df499a8 100644
--- a/m4/setlocale_null.m4
+++ b/m4/setlocale_null.m4
@@ -1,4 +1,4 @@
-# setlocale_null.m4 serial 6
+# setlocale_null.m4 serial 7
 dnl Copyright (C) 2019-2023 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -13,9 +13,23 @@ AC_DEFUN([gl_FUNC_SETLOCALE_NULL],
   AC_CACHE_CHECK([whether setlocale (LC_ALL, NULL) is multithread-safe],
 [gl_cv_func_setlocale_null_all_mtsafe],
 [case "$host_os" in
-   # Guess no on musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku, 
Cygwin.
-   *-musl* | darwin* | freebsd* | midnightbsd* | netbsd* | openbsd* | aix* 
| haiku* | cygwin*)
+   # Guess no on musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku.
+   *-musl* | darwin* | freebsd* | midnightbsd* | netbsd* | openbsd* | aix* 
| haiku*)
  gl_cv_func_setlocale_null_all_mtsafe=no ;;
+   # Guess no on Cygwin < 3.4.6.
+   cygwin*)
+ AC_EGREP_CPP([Lucky user],
+   [
+#if defined __CYGWIN__
+ #include 
+ #if CYGWIN_VERSION_DLL_COMBINED >= CYGWIN_VERSION_DLL_MAKE_COMBINED (3004, 6)
+  Lucky user
+ #endif
+#endif
+  ],
+  [gl_cv_func_setlocale_null_all_mtsafe=yes],
+  [gl_cv_func_setlocale_null_all_mtsafe=no])
+;;
# Guess yes on glibc, HP-UX, IRIX, Solaris, native Windows.
*-gnu* | gnu* | hpux* | irix* | solaris* | mingw*)
  gl_cv_func_setlocale_null_all_mtsafe=yes ;;
diff --git a/tests/test-setlocale_null-mt-all.c 
b/tests/test-setlocale_null-mt-all.c
index 6036c260cd..7480406639 100644
--- a/tests/test-setlocale_null-mt-all.c
+++ b/tests/test-setlocale_null-mt-all.c
@@ -166,7 +166,7 @@ Solaris 11.0 OK
 Solaris 11.4 OK
 Solaris OpenIndiana  OK
 Haikucrash < 1 sec
-Cygwin   crash < 1 sec
+Cygwin < 3.4.6   crash < 1 sec
 mingwOK
 MSVC OK (assuming compiler option /MD !)
 */

Re: [PATCH] Do not decorate symbols as dllexport on Cygwin

[Bruno Haible]

Hi Corinna,

Sorry, I wanted to reply sooner.

> > May I ask what's the idea to provide a thread-safe setlocale?  It was
> > never defined as thread-safe and POSIX explicitely mentions that.  Any
> > application expecting to call setlocale thread-safe is broken by design.

The 'recode' package includes a shared library 'librecode', and shared
libraries are supposed to be multithread-safe.

Multithread-safe code is supposed to be able to obey the current locale,
that is,
  - if uselocale(NULL) != LC_GLOBAL_LOCALE, uselocale()
  - if uselocale(NULL) == LC_GLOBAL_LOCALE, the global locale, that was
last set through setlocale().

For example:
  - sprintf() needs to know the LC_CTYPE and LC_NUMERIC categories of the
current locale, in order to transform wide strings via %ls or to
format numbers via %f or %g.
  - iconv() needs to know the LC_CTYPE category of the current locale,
for determining the appropriate transliterations.
  - strftime() needs to know the LC_TIME category of the current locale.
  - gettext() needs to know the LC_MESSAGES category of the current locale.
All these functions are supposed to be writable in application code
(if, for whatever reason, the sprintf, iconv, strftime, gettext functions
in libc are not considered adequate).

When POSIX specifies that applications cannot call setlocale() when there
are multiple threads, this implies that in order to *inspect* the locale
they need to do so before spawning the threads, and cache the value in a
global variable, for later use. But this is not the application architecture
that is in use when there are shared libraries. Shared libraries commonly
don't have an initialization hook by which the application informs the
library about the current locale for the rest of the execution of the
process.

So, conventional wisdom is to use setlocale(category, NULL). But this
faces MT-safety problems. On some platforms the problem is that the
setlocale(_, NULL) calls themselves will trash each other's data structures
and thus provoke a crash. On other platforms the problem is that the return
value produced in one thread is being clobbered by the second thread, and
thus the first thread has no chance to copy the return value to a safe
area in due time.

For Cygwin, the problem until yesterday was the second one, for
category == LC_ALL only.

> > It should use the newlocale/duplocale/uselocale/freelocale API instead,
> > isn't it?

Even code that pays attention to the per-thread locale has to be prepared
for the case that uselocale(NULL) returns LC_GLOBAL_LOCALE. In this case,
the caller needs to fetch the values from the global locale. There is no
locale_t object that could be queried in this case.

> Ahhh, I finally see what's going on.  The problem is not thread-safety
> as such, but thread-safety when reading the value of the LC_ALL category.

Yup, exactly.

> Glibc's setlocale isn't entirely thread-safe either, but there's a
> difference:
> 
> - GLibc creates the global strings returned by setlocale(LC_xxx, NULL)
>   at the time the locale data is changed.  All setlocale(LC_xxx, NULL)
>   calls only return pointer to strings created earlier.

Yes. Thus each thread can be sure to be able to inspect the return value.
(Assuming that there is no call to setlocale that *changes* the global
locale. This is forbidden by POSIX, and reasonable applications don't
do this.)

> - Cygwin or, better, newlib, also return a pointer to global strings.
>   However, while the global strings for the specific categories are
>   created when the locale is changed, the string returned for LC_ALL
>   gets created on the fly when setlocale(LC_ALL, NULL) is called.
... and gets overwritten by another thread, since the buffer is not
per-thread.
>   That's why test-setlocale_null-mt-all fails almost immediately.
> 
> I created a patch to newlib's setlocale to tweak the LC_ALL string
> each time the locale is changed, while setlocale(LC_ALL, NULL)
> just returns the already prepared string.
> 
> https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=23e49b18ce39
> 
> This patch will be in the next Cygwin release 3.4.6.

Thanks a lot! I'm adjusting the Gnulib code now, accordingly.

Bruno

Re: [PATCH] Do not decorate symbols as dllexport on Cygwin

[Corinna Vinschen]

Hi Bruno,

On Feb  5 22:22, Corinna Vinschen wrote:
> On Feb  5 21:41, Bruno Haible wrote:
> > Another option — since we are talking about a single symbol and a single
> > platform — would be if the locking for setlocale_null were not necessary
> > on Cygwin in the first place. I determined that it is necessary by running
> > the unit test gnulib/tests/test-setlocale_null-mt-all.c [3] on Cygwin:
> > without the lock, it crashed within less than 1 second. Could the
> > implementation of setlocale() in Cygwin be changed in such a way that this
> > test does not crash? Then the lock would be necessary.
> 
> Well, we could do that by adding Cygwin-internal locking to setlocale
> calls.  But that would only be available in the next Cygwin version
> of course.
> 
> May I ask what's the idea to provide a thread-safe setlocale?  It was
> never defined as thread-safe and POSIX explicitely mentions that.  Any
> application expecting to call setlocale thread-safe is broken by design.
> It should use the newlocale/duplocale/uselocale/freelocale API instead,
> isn't it?

Ahhh, I finally see what's going on.  The problem is not thread-safety
as such, but thread-safety when reading the value of the LC_ALL category.

Glibc's setlocale isn't entirely thread-safe either, but there's a
difference:

- GLibc creates the global strings returned by setlocale(LC_xxx, NULL)
  at the time the locale data is changed.  All setlocale(LC_xxx, NULL)
  calls only return pointer to strings created earlier.

- Cygwin or, better, newlib, also return a pointer to global strings.
  However, while the global strings for the specific categories are
  created when the locale is changed, the string returned for LC_ALL
  gets created on the fly when setlocale(LC_ALL, NULL) is called.
  That's why test-setlocale_null-mt-all fails almost immediately.

I created a patch to newlib's setlocale to tweak the LC_ALL string
each time the locale is changed, while setlocale(LC_ALL, NULL)
just returns the already prepared string.

https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=23e49b18ce39

This patch will be in the next Cygwin release 3.4.6.

I'm just a bit fuzzy what patches will be required for gnulib now...


Thanks,
Corinna

Re: Gnulib and nullptr

[Bruno Haible]

Paul Eggert wrote:
> > Why call it 'c-nullptr', not 'nullptr'?
> 
> I was worried about C++, not that I know much about it, and operated by 
> analogy with the name of m4/c-bool.m4.

OK, I see. So the prefix 'c-' in 'c-bool' and 'c-nullptr' means something
like "ISO C standard compliant". We could also have been using a prefix
'std-' maybe. Anyway, it's OK; it just wasn't immediately clear to me.

Bruno

Re: module c-nullptr: error compiling groff

[Bruno Haible]

Paul Eggert wrote:
> > +# ifdef __cplusplus
> > +/* For the C++ compiler the result of the configure test is irrelevant.
> > +   We know that at least g++ and clang with option -std=c++11 or higher, 
> > as well
> > +   as MSVC 14 or newer, already have nullptr.  */
> > +#  if !(((defined __GNUC__ || defined __clang__) && __cplusplus >= 
> > 201103L) \
> > +|| (defined _MSC_VER && 1900 <= _MSC_VER))
> 
>  says nullptr is 
> part of C++11, so would it be better to omit the "(defined __GNUC__ || 
> defined __clang__) && "? It seems unlikely that a compiler would 
> advertise conformance to C++11 without supporting nullptr.

Unlikely?! You must be joking. We have seen so many occurrences where
compilers define __STDC_VERSION__ to a certain value without implementing
the corresponding standard entirely and correctly. Why should it be different
with __cplusplus? It is a common human behaviour to claim "we have XYZ!"
when in fact they only have 90% of XYZ.

In this particular case,
  - if we leave 'nullptr' alone although the compiler doesn't support it,
there will be a compilation error quickly,
  - if we define 'nullptr' although the compiler supports it already, there
are good chances that there will be no compilation error. (That's what
I observed with all compilers except GCC.)
Therefore I did not want to make bets regarding C++ compilers that set
__cplusplus = 201103L but that I have not tested.

Bruno

Re: Gnulib and nullptr

[Simon Josefsson via Gnulib discussion list]

Bruno Haible  writes:

> Therefore, I would be in favour of EITHER
> * doing this when the community as a whole has adopted 'nullptr' in C, i.e.
>   this keyword is no longer something that is new to an average newcomer,
>   (even if that's only 10 years from now),
> OR
> * doing the change only in those places where it actually matters, that is,
>   in varargs argument lists.

I agree with this conclucsion -- and pending 1) above, I believe 2) is
sufficient and I would argue that we should all generally continue to
use NULL in all other cases than varargs because it is a well-known
idiom.  This may cause 1) above to never occur, which seems acceptable.
This assumes there aren't other important use-cases for nullptr than
varargs that aren't clear.  Personally I don't believe consistency with
C++ is important (usually this makes C code uglier and less idiomatic in
my experience) but opinion may vary.

/Simon


signature.asc
Description: PGP signature

[g.branden.robin...@gmail.com: macOS 12.6.3 and vasnprintf compilation failure]

[G. Branden Robinson]

[re-sending to correct mailing list]

- Forwarded message from "G. Branden Robinson" 
 -

Date: Mon, 6 Feb 2023 02:28:10 -0600
From: "G. Branden Robinson" 
To: gnu...@gnu.org
Cc: gr...@gnu.org
Subject: macOS 12.6.3 and vasnprintf compilation failure

[please CC groff@ on replies]

Hi folks,

groff recently updated its gnulib submodule[1] and published 1.23.0.rc2;
we're hoping to get a final release out soon.

A problem immediately arose on macOS 12.6.3.  It's our good friend
vasnprintf again.  Logs are available on Savannah[2].

lib/vasnprintf.c:411:16: error: expected parameter declarator
static_assert (sizeof (mp_limb_t) * CHAR_BIT == GMP_LIMB_BITS);
   ^
lib/vasnprintf.c:411:16: error: expected ')'
lib/vasnprintf.c:411:15: note: to match this '('
static_assert (sizeof (mp_limb_t) * CHAR_BIT == GMP_LIMB_BITS);
  ^
lib/vasnprintf.c:411:1: warning: type specifier missing, defaults to 'int' 
[-Wimplicit-int]
static_assert (sizeof (mp_limb_t) * CHAR_BIT == GMP_LIMB_BITS);
^
lib/vasnprintf.c:415:16: error: expected parameter declarator
static_assert (sizeof (mp_twolimb_t) * CHAR_BIT == GMP_TWOLIMB_BITS);
   ^
lib/vasnprintf.c:415:16: error: expected ')'
lib/vasnprintf.c:415:15: note: to match this '('
static_assert (sizeof (mp_twolimb_t) * CHAR_BIT == GMP_TWOLIMB_BITS);
  ^
lib/vasnprintf.c:415:1: warning: type specifier missing, defaults to 'int' 
[-Wimplicit-int]
static_assert (sizeof (mp_twolimb_t) * CHAR_BIT == GMP_TWOLIMB_BITS);
^

This appears to be a distinct issue from one in macOS 10.13.

https://lists.gnu.org/archive/html/bug-gnulib/2017-07/msg00056.html

The above output is followed by 14 further warnings howling about the
deprecation of sprintf, but I see that is also being discussed[3], and I
won't gate the groff release on warnings of that nature.

Outright failures, as seen above, though, are a problem.

Any suggestions?

Regards,
Branden

[1] $ git submodule status
 4e9fcc7b84fcac07a3e5a3cd5f66d1ff320dc8e8 gnulib (v0.1-5805-g4e9fcc7b84)
[2] https://savannah.gnu.org/bugs/?63767
[3] https://lists.gnu.org/archive/html/bug-gnulib/2022-11/msg00132.html



- End forwarded message -


signature.asc
Description: PGP signature