Re: Pacify -Wmissing-variable-declarations in unit tests.

[Collin Funk]

Hi Paul,

On 4/28/24 4:27 PM, Paul Eggert wrote:
> For test cases this is more a judgment call, but I prefer doing either
> the above or adjusting the warning flags, to ignoring warnings, as the
> other warnings can be useful at time.

Yeah, I could see these warnings making it hard to see ones that
actually matter. Lets see what Bruno thinks.

The patch in the original email should contain all of the variables
that trigger it. If we decide to follow the coding style you mentioned
I can make all the obvious ones static, i.e. checking for variables
being defined in gnulib headers.

Collin

Re: Pacify -Wmissing-variable-declarations in unit tests.

[Paul Eggert]

On 2024-04-28 04:03, Collin Funk wrote:

I will listen to the Makefile and*ignore*  them now, or disable them
if they start annoying me. :)


Another possibility is to make each such variable 'static' if it's OK to 
make it static, and to precede every other variable declaration like this:


int foo;

with a declaration like this:

extern int foo;

I do this sort of thing in code that's not a test case, as I find it 
helpful to put the extern declaration into a .h file so that it's tested 
for compatibility when other modules use it.


For test cases this is more a judgment call, but I prefer doing either 
the above or adjusting the warning flags, to ignoring warnings, as the 
other warnings can be useful at time.

doc: Update macro list in gnulib-cache.m4 documentation.

[Collin Funk]

I've applied the attached patch updating the macro list in the
gnulib-cache.m4 documentation.

The macros seemed to match the order of the sed invocation in
func_import of gnulib-tool.sh, so most of it was just copying from
there and adjusting the gnulib-tool --help messages to fit the
existing documentation.

CollinFrom 431b58497c6ae0e2bc79c8ed41b3dcfc32248405 Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Sun, 28 Apr 2024 15:52:27 -0700
Subject: [PATCH] doc: Update macro list in gnulib-cache.m4 documentation.

* doc/gnulib-tool.texi (Modified imports): Add missing macros from the
sed invocation in func_import of gnulib-tool.sh.
---
 ChangeLog|  6 
 doc/gnulib-tool.texi | 79 
 2 files changed, 85 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 137830499a..c52dee0b6f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2024-04-28  Collin Funk  
+
+	doc: Update macro list in gnulib-cache.m4 documentation.
+	* doc/gnulib-tool.texi (Modified imports): Add missing macros from the
+	sed invocation in func_import of gnulib-tool.sh.
+
 2024-04-28  Collin Funk  
 
 	doc: Mention AC_CONFIG_MACRO_DIRS in configure.ac.
diff --git a/doc/gnulib-tool.texi b/doc/gnulib-tool.texi
index 1a34673eb2..9ed30c2055 100644
--- a/doc/gnulib-tool.texi
+++ b/doc/gnulib-tool.texi
@@ -374,10 +374,45 @@ @node Modified imports
 
 In the @file{gnulib-cache.m4} file, the macros have the following meaning:
 @table @code
+@item gl_LOCAL_DIR
+The argument is a colon separated list of local directories where
+@code{gnulib-tool} will search before looking at gnulib's directory.
+Corresponds to the @samp{--local-dir} command line argument.
+
 @item gl_MODULES
 The argument is a space separated list of the requested modules, not including
 dependencies.
 
+@item gl_WITH_OBSOLETE
+The presence of this macro corresponds to the @samp{--with-obsolete} command
+line argument.  It takes no arguments.
+
+@item gl_WITH_CXX_TESTS
+The presence of this macro corresponds to the @samp{--with-c++-tests} command
+line argument and to the absence of the @samp{--without-c++-tests} command line
+argument.  It takes no arguments.
+
+@item gl_WITH_LONGRUNNING_TESTS
+The presence of this macro corresponds to the @samp{--with-longrunning-tests}
+command line argument and to the absence of the
+@samp{--without-longrunning-tests} command line argument.  It takes no
+arguments.
+
+@item gl_WITH_PRIVILEGED_TESTS
+The presence of this macro corresponds to the @samp{--with-longrunning-tests}
+command line argument and to the absence of the
+@samp{--without-longrunning-tests} command line argument.  It takes no
+arguments.
+
+@item gl_WITH_UNPORTABLE_TESTS
+The presence of this macro corresponds to the @samp{--with-unportable-tests}
+command line argument and to the absence of the
+@samp{--without-unportable-tests} command line argument.  It takes no arguments.
+
+@item gl_WITH_ALL_TESTS
+The presence of this macro corresponds to the @samp{--with-all-tests} command
+line argument.  It takes no arguments.
+
 @item gl_AVOID
 The argument is a space separated list of modules that should not be used,
 even if they occur as dependencies.  Corresponds to the @samp{--avoid}
@@ -393,10 +428,22 @@ @node Modified imports
 M4 macros (*.m4 files).  Corresponds to the @samp{--m4-base} command line
 argument.
 
+@item gl_PO_BASE
+The argument is the relative file name of the directory containing *.po files.
+Corresponds to the @samp{--po-base} command line argument.
+
+@item gl_DOC_BASE
+The argument is the relative file name of the directory containing documentation
+files.  Corresponds to the @samp{--doc-base} command line argument.
+
 @item gl_TESTS_BASE
 The argument is the relative file name of the directory containing the gnulib
 unit test files.  Corresponds to the @samp{--tests-base} command line argument.
 
+@item gl_WITH_TESTS
+The presence of this macro corresponds to the @samp{--with-tests} command line
+argument.  It takes no arguments.
+
 @item gl_LIB
 The argument is the name of the library to be created.  Corresponds to the
 @samp{--lib} command line argument.
@@ -407,6 +454,24 @@ @node Modified imports
 value must be 2 or 3) corresponds to the @samp{--lgpl=@var{arg}} command line
 argument.
 
+@item gl_MAKEFILE_NAME
+The argument is the name of the makefile in the source-base and tests-base
+directories.  Corresponds to the @samp{--makefile-name} command line argument.
+
+@item gl_TESTS_MAKEFILE_NAME
+The argument is the name of the makefile in the tests-base directory.
+Corresponds to the @samp{--tests-makefile-name} command line argument.
+
+@item gl_AUTOMAKE_SUBDIR
+The presence of this macro corresponds to the @samp{--automake-subdir} command
+line argument.  It takes no arguments.
+
+@item gl_CONDITIONAL_DEPENDENCIES
+The presence of this macro corresponds to the @samp{--conditional-dependencies}
+command line argument and to the absence of the
+@samp{--no-conditional-dependencies} command 

Re: future Python evolution

[Bernhard Voelker]

On 4/21/24 16:50, Bruno Haible wrote:

Good point. Yes, Python occasionally (rarely?) makes incompatible changes.
So, I've now created a continuous integration at [2]. If a Python release
is made that breaks gnulib-tool, this CI will notify me shortly afterwards,
and we will have time to adapt gnulib-tool, even before the new Python
release lands in the various distros.


great, thanks!  (sorry, the mail didn't go out earlier.)

Have a nice day,
Berny

Re: [PATCH] doc: Mention AC_CONFIG_MACRO_DIRS in configure.ac.

[Bruno Haible]

Hi Collin,

> I also noticed some macros were missing from the gnulib-cache.m4 list
> starting at line 375.
> 
> Is it just out of date (git blame says ~2005)

Yes. I probably forgot that these macros were documented, when adding
extensions later.

> If it is the first case I can add them later
> today while the gnulib-tool knowledge is still in my brain. :)

Yes, please!

Bruno

Re: [PATCH] doc: Mention AC_CONFIG_MACRO_DIRS in configure.ac.

[Collin Funk]

Hi Bruno,

On 4/28/24 5:02 AM, Bruno Haible wrote:
>> +* doc/gnulib-tool.texi (Initial import): Update the example gnulib-tool
>> +invocation. Document the use of AC_CONFIG_MACRO_DIRS as an alternative
>> +to ACLOCAL_AMFLAGS.
> Thanks.

I also noticed some macros were missing from the gnulib-cache.m4 list
starting at line 375.

Is it just out of date (git blame says ~2005) or was there a reason
for not adding the rest? If it is the first case I can add them later
today while the gnulib-tool knowledge is still in my brain. :)

Collin

Re: [PATCH] doc: Mention AC_CONFIG_MACRO_DIRS in configure.ac.

[Bruno Haible]

> + * doc/gnulib-tool.texi (Initial import): Update the example gnulib-tool
> + invocation. Document the use of AC_CONFIG_MACRO_DIRS as an alternative
> + to ACLOCAL_AMFLAGS.

Thanks.

[PATCH] doc: Mention AC_CONFIG_MACRO_DIRS in configure.ac.

[Collin Funk]

Document the following gnulib-tool change:
2021-12-19  Bruno Haible  
gnulib-tool: Don't insist on ACLOCAL_AMFLAGS.

* doc/gnulib-tool.texi (Initial import): Update the example gnulib-tool
invocation. Document the use of AC_CONFIG_MACRO_DIRS as an alternative
to ACLOCAL_AMFLAGS.
---
 ChangeLog| 10 ++
 doc/gnulib-tool.texi |  8 
 2 files changed, 18 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 5948faf102..137830499a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2024-04-28  Collin Funk  
+
+   doc: Mention AC_CONFIG_MACRO_DIRS in configure.ac.
+   Document the following gnulib-tool change:
+   2021-12-19  Bruno Haible  
+   gnulib-tool: Don't insist on ACLOCAL_AMFLAGS.
+   * doc/gnulib-tool.texi (Initial import): Update the example gnulib-tool
+   invocation. Document the use of AC_CONFIG_MACRO_DIRS as an alternative
+   to ACLOCAL_AMFLAGS.
+
 2024-04-28  Bruno Haible  
 
bootstrap: Support checking out a recent GNULIB_REVISION, part 2.
diff --git a/doc/gnulib-tool.texi b/doc/gnulib-tool.texi
index 0eb3b247e2..1a34673eb2 100644
--- a/doc/gnulib-tool.texi
+++ b/doc/gnulib-tool.texi
@@ -156,6 +156,7 @@ @node Initial import
   - add "lib/Makefile" to AC_CONFIG_FILES in ./configure.ac,
   - mention "lib" in SUBDIRS in Makefile.am,
   - mention "-I m4" in ACLOCAL_AMFLAGS in Makefile.am,
+or add an AC_CONFIG_MACRO_DIRS([m4]) invocation in ./configure.ac,
   - invoke gl_EARLY in ./configure.ac, right after AC_PROG_CC,
   - invoke gl_INIT in ./configure.ac.
 ~/src/libfoo$
@@ -223,6 +224,13 @@ @node Initial import
 ACLOCAL_AMFLAGS = -I m4
 @end example
 
+Alternatively, add an @code{AC_CONFIG_MACRO_DIRS} invocation in your
+@file{configure.ac} file, as in:
+
+@example
+AC_CONFIG_MACRO_DIRS([m4])
+@end example
+
 You are now ready to call the M4 macros in @code{gnulib-comp.m4} from
 @file{configure.ac}.  The macro @code{gl_EARLY} must be called as soon
 as possible after verifying that the C compiler is working.
-- 
2.44.0

Re: gnulib-tool.sh: Match sorting of gnulib-tool.py in output.

[Collin Funk]

On 4/28/24 2:52 AM, Bruno Haible wrote:
> Thanks! OK to push in 1 or 2 days.

Sounds good.

>> Though, the correct way to fix this would be making instance variables
>> local when they are only used in one function.
> 
> I agree that this kind of doing side effects on a GLConfig object that
> is already held by another object is not super maintainable.

Yeah, there are many things I dislike about that class. I'll probably
have a look at *slowly* cleaning it up eventually. There is a bunch of
dead code in there if I remember correctly.

> However, this is not the only call to setLibTests. There is another one
> at GLTestDir.py:392. And there is a call to setAuxDir in GLTestDir.py:401.
> It seems that a conversion to use instance variables or local variables
> would be quite intrusive. I'm not sure this refactoring would be worth it.

Good point. I'll probably have to double check those too.

Collin

Re: Pacify -Wmissing-variable-declarations in unit tests.

[Collin Funk]

Hi Bruno,

On 4/28/24 3:11 AM, Bruno Haible wrote:
>> Can I apply the attached patch which adds the 'static' specifier to
>> global variables in unit tests?
> 
> No! While adding 'static' would be no harm in some tests (such as
> tests/jit/test-cache.c, tests/test-argp-version-etc.c, tests/test-argp.c),
> in other tests the purpose of the global variables is to defeat compiler
> optimizations.
> Maybe in 10 years, everything will be compiled with '-flto' and thus
> compiler optimizations on global variables will be the same as on 'static'
> variables. But we're not there yet, and thus it helps us avoid compiler
> optimizations that would make the unit test a no-op.

I see. It looks like I didn't spend enough time reading GCC's
optimization documentation or I would have known it was a silly
idea...

> (You just discovered how tedious it is to investigate a compiler's
> behaviour that causes a test failure. A compiler optimization that turns
> a unit test into a no-op is even worse: You then notice, by chance, that
> the unit test has not been effective for two years or so...)

Yes, that doesn't sound very fun. I have a feeling you have had that
happen before?

> Recall that [1]
>   Many GCC warning options usually don’t point to mistakes in the code;
>   these warnings enforce a certain programming style.

Yes, good point. So I guess here the warning wants you to be explicit
about the visibility of the symbol. I think ISO C uses the terms
internal/external linkage.

> Yup, this is a programming style.
>   - Other developers do it differently.
>   - In many test cases, this programming style invites the compiler
> to cause trouble.
> 
> Therefore it's not the programming style that we use in the tests.

Makes sense. Thanks for the explanations.

I will listen to the Makefile and *ignore* them now, or disable them
if they start annoying me. :)

Collin

Re: Pacify -Wmissing-variable-declarations in unit tests.

[Bruno Haible]

Hi Collin,

> Can I apply the attached patch which adds the 'static' specifier to
> global variables in unit tests?

No! While adding 'static' would be no harm in some tests (such as
tests/jit/test-cache.c, tests/test-argp-version-etc.c, tests/test-argp.c),
in other tests the purpose of the global variables is to defeat compiler
optimizations.
Maybe in 10 years, everything will be compiled with '-flto' and thus
compiler optimizations on global variables will be the same as on 'static'
variables. But we're not there yet, and thus it helps us avoid compiler
optimizations that would make the unit test a no-op.

(You just discovered how tedious it is to investigate a compiler's
behaviour that causes a test failure. A compiler optimization that turns
a unit test into a no-op is even worse: You then notice, by chance, that
the unit test has not been effective for two years or so...)

> When building Coreutils I noticed that GCC 14's
> -Wmissing-variable-declarations option spams the test output.

There are two ways to deal with this:

a) Listen to the message in the banner

##  ##
## --- Gnulib tests --- ##
## You can ignore compiler warnings in this directory.  ##
##  ##

b) Exploit the existing GL_CFLAG_GNULIB_WARNINGS mechanism to annihilate
this type of warning.

Recall that [1]
  Many GCC warning options usually don’t point to mistakes in the code;
  these warnings enforce a certain programming style.

> I'm used to marking things static even in small single file programs

Yup, this is a programming style.
  - Other developers do it differently.
  - In many test cases, this programming style invites the compiler
to cause trouble.

Therefore it's not the programming style that we use in the tests.

Bruno

[1] https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html

Re: gnulib-tool.sh: Match sorting of gnulib-tool.py in output.

[Bruno Haible]

Hi Collin,

> > The 'libtests' value read from the config in self.emitter is
> > incorrect.
> 
> This patch seems to support what I was saying here.
> 
> diff --git a/pygnulib/GLTestDir.py b/pygnulib/GLTestDir.py
> index 002eb30267..11b067e085 100644
> --- a/pygnulib/GLTestDir.py
> +++ b/pygnulib/GLTestDir.py
> @@ -244,8 +244,7 @@ def execute(self) -> None:
>  if file.startswith('lib/'):
>  libtests = True
>  break
> -if libtests:
> -self.config.setLibtests(True)
> +self.emitter.config.setLibtests(libtests)
>  
>  if single_configure:
>  # Add the dummy module to the main module list if needed.

Thanks! OK to push in 1 or 2 days.

> Though, the correct way to fix this would be making instance variables
> local when they are only used in one function.

I agree that this kind of doing side effects on a GLConfig object that
is already held by another object is not super maintainable.

However, this is not the only call to setLibTests. There is another one
at GLTestDir.py:392. And there is a call to setAuxDir in GLTestDir.py:401.
It seems that a conversion to use instance variables or local variables
would be quite intrusive. I'm not sure this refactoring would be worth it.

Bruno

Re: Fetch from existing gnulib Git repository if needed

[Bruno Haible]

Markus Mützel wrote:
> However, it looks like $GNULIB_SRCDIR is empty for us. So, the change doesn't 
> seem to make a difference. Executing bootstrap after a revision bump still 
> fails if the bootstrap script was already run before.

Oh, there were two 'git checkout' commands and I enhanced only one of them...

Should now be fixed, like this. Thanks for the feedback!


2024-04-28  Bruno Haible  

bootstrap: Support checking out a recent GNULIB_REVISION, part 2.
Reported by Markus Mützel  in
.
* top/bootstrap-funclib.sh (prepare_GNULIB_SRCDIR): If using a submodule
and the 'git checkout' command fails, fetch the newer commits and then
retry it.
* build-aux/bootstrap: Regenerated.

diff --git a/top/bootstrap-funclib.sh b/top/bootstrap-funclib.sh
index 620006d320..f7905eb208 100644
--- a/top/bootstrap-funclib.sh
+++ b/top/bootstrap-funclib.sh
@@ -1,6 +1,6 @@
 # A library of shell functions for autopull.sh, autogen.sh, and bootstrap.
 
-scriptlibversion=2024-04-27.17; # UTC
+scriptlibversion=2024-04-28.09; # UTC
 
 # Copyright (C) 2003-2024 Free Software Foundation, Inc.
 #
@@ -465,8 +465,8 @@ prepare_GNULIB_SRCDIR ()
   # The 'git checkout "$GNULIB_REVISION"' command succeeds if the
   # GNULIB_REVISION is a commit hash that exists locally, or if it is
   # branch name that can be fetched from origin. It fails, however,
-  # if the GNULIB_REVISION is a commit hash that only exists in origin.
-  # In this case, we need a 'git fetch' and then retry
+  # if the GNULIB_REVISION is a commit hash that only exists in
+  # origin. In this case, we need a 'git fetch' and then retry
   # 'git checkout "$GNULIB_REVISION"'.
   (cd "$GNULIB_SRCDIR" \
&& { git checkout "$GNULIB_REVISION" 2>/dev/null \
@@ -542,7 +542,17 @@ prepare_GNULIB_SRCDIR ()
 # The subdirectory 'gnulib' already exists.
 if test -n "$GNULIB_REVISION"; then
   if test -d "$gnulib_path/.git"; then
-(cd "$gnulib_path" && git checkout "$GNULIB_REVISION") || exit 1
+# The 'git checkout "$GNULIB_REVISION"' command succeeds if the
+# GNULIB_REVISION is a commit hash that exists locally, or if it is
+# branch name that can be fetched from origin. It fails, however,
+# if the GNULIB_REVISION is a commit hash that only exists in
+# origin. In this case, we need a 'git fetch' and then retry
+# 'git checkout "$GNULIB_REVISION"'.
+(cd "$gnulib_path" \
+ && { git checkout "$GNULIB_REVISION" 2>/dev/null \
+  || { git fetch origin && git checkout "$GNULIB_REVISION"; }
+}
+) || exit $?
   else
 die "Error: GNULIB_REVISION is specified in bootstrap.conf," \
 "but '$gnulib_path' contains no git history"

Re: Fetch from existing gnulib Git repository if needed

[Markus Mützel]

Hi Bruno,

> > As a workaround we are applying the attached patch to the 
> > bootstrap-funclib.sh script to automatically fetch from the remote gnulib 
> > repository if the GNULIB_REVISION isn't found in the local gnulib Git 
> > repository.
>
> Thanks for the patch. But note that GNULIB_REVISION can hold either a commit
> hash or the name of a branch (such as 'stable-202401'). So, we have 4 cases:
>   (a) a commit hash, already present
>   (b) a commit hash, not known
>   (c) a branch, already present
>   (d) a branch, not known
>
> The command 'git cat-file commit $GNULIB_REVISION' returns true in the cases
> (a) and (c). So, your patch would trigger a 'git fetch' in the cases (b) and
> (d). But in case (d), the 'git fetch' is useless:
> 'git cat-file commit $GNULIB_REVISION' would still fail afterwards.
>
> One can distinguish the four cases in more detail using the commands
>   git rev-list --quiet $GNULIB_REVISION --
> which tests for case (a) and
>   git show-ref --verify --quiet refs/heads/$GNULIB_REVISION
> which tests for case (c). This would allow us to do 'git fetch' only in case
> (b).
>
> However, I believe the patch below is simpler and achieves the same goal.

Thank you for looking into this. However, it looks like $GNULIB_SRCDIR is empty 
for us. So, the change doesn't seem to make a difference. Executing bootstrap 
after a revision bump still fails if the bootstrap script was already run 
before.
I like your idea of fetching if the checkout failed though.

The attached diff seems to be working for our use case. Would it be possible to 
apply something like that in gnulib?

Markus
diff -r 0fbf06e4b460 bootstrap-funclib.sh
--- a/bootstrap-funclib.sh  Sat Apr 27 17:33:00 2024 +0200
+++ b/bootstrap-funclib.sh  Sun Apr 28 11:04:52 2024 +0200
@@ -462,7 +462,17 @@
   || die "Error: --gnulib-srcdir or \$GNULIB_SRCDIR is specified," \
  "but does not contain gnulib-tool"
 if test -n "$GNULIB_REVISION" && $use_git; then
-  (cd "$GNULIB_SRCDIR" && git checkout "$GNULIB_REVISION") || exit $?
+  # The 'git checkout "$GNULIB_REVISION"' command succeeds if the
+  # GNULIB_REVISION is a commit hash that exists locally, or if it is
+  # branch name that can be fetched from origin. It fails, however,
+  # if the GNULIB_REVISION is a commit hash that only exists in origin.
+  # In this case, we need a 'git fetch' and then retry
+  # 'git checkout "$GNULIB_REVISION"'.
+  (cd "$GNULIB_SRCDIR" \
+&& { git checkout "$GNULIB_REVISION" 2>/dev/null \
+  || { git fetch origin && git checkout "$GNULIB_REVISION"; }
+}
+  ) || exit $?
 fi
   else
 if ! $use_git; then
@@ -532,7 +542,17 @@
 # The subdirectory 'gnulib' already exists.
 if test -n "$GNULIB_REVISION"; then
   if test -d "$gnulib_path/.git"; then
-(cd "$gnulib_path" && git checkout "$GNULIB_REVISION") || exit 1
+# The 'git checkout "$GNULIB_REVISION"' command succeeds if the
+# GNULIB_REVISION is a commit hash that exists locally, or if it is
+# branch name that can be fetched from origin. It fails, however,
+# if the GNULIB_REVISION is a commit hash that only exists in 
origin.
+# In this case, we need a 'git fetch' and then retry
+# 'git checkout "$GNULIB_REVISION"'.
+(cd "$gnulib_path" \
+  && { git checkout "$GNULIB_REVISION" 2>/dev/null \
+   || { git fetch origin && git checkout "$GNULIB_REVISION"; }
+  }
+) || exit $?
   else
 die "Error: GNULIB_REVISION is specified in bootstrap.conf," \
 "but '$gnulib_path' contains no git history"

Re: test-sprintf-posix and test-snprintf-posix test failures

[Bruno Haible]

Collin Funk wrote:
> > If I understand correctly, sprintf should return 4 here correct? And
> > the buffer should have 5 NUL bytes. 4 from the arguments and 1
> > trailing, which is excluded from the return value.

Correct. I added a comment, because the situation in ISO C and POSIX has
changed, not too long ago.

> I have no other systems to check this on, so I will trust my findings
> there.
> 
> Reported here:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114876

Perfect! Thank you. You also found out in which corner of GCC the faulty
optimization lies!

Bruno

Re: test-sprintf-posix and test-snprintf-posix test failures

[Collin Funk]

On 4/28/24 12:38 AM, Collin Funk wrote:
> If I understand correctly, sprintf should return 4 here correct? And
> the buffer should have 5 NUL bytes. 4 from the arguments and 1
> trailing, which is excluded from the return value.

I have no other systems to check this on, so I will trust my findings
there.

Reported here:

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

Collin

Re: test-sprintf-posix and test-snprintf-posix test failures

[Collin Funk]

Hi Bruno,

On 4/27/24 2:02 AM, Bruno Haible wrote:
> At this point of the investigations, it's too early for a patch.
> First, one needs to determine whether it's a bug in Gnulib or a
> bug in some package we rely on (typically gcc or glibc). In the
> latter case, we report the bug and wait for the answer. Depending
> on the answer, we may fix the test (if they say that our test is
> wrong), or we may put in an officially recommended workaround,
> or a workaround of our liking, or disable that part of the unit test.

Sounds like a lot of things to learn.

> If some glibc-optimized expansions are active, you would also see some
> *_chk symbols in the 'nm' output.

Interesting, I didn't know that is how you could tell. Thanks!

> So, at this point it looks like a gcc problem. The next step is to prepare
> a trimmed-down test case :
>   1. Concentrate on the simpler test case. In this case, I would say, it's
>  sprintf, since it takes one argument less than snprintf.
>   2. Eliminate the -I, -L, -l options specific to gnulib. So that you have
>  a program that succeeds without -O2 and fails with -O2. This implies
>  inlining include files from gnulib (test-sprintf-posix.h, macros.h, 
> etc.).
>   3. Work on minimizing the compilation unit that succeeds without -O2 and
>  fails with -O2. Always keep -Wall enabled, to guard against mistakes.
>  Remove the test cases that pass, remove unnecessary includes, and so on.
>   4. When you can't minimize that further, preprocess it:
>  $ gcc OPTIONS -E foo.c > foo-pre.c
>  Verify that it still succeeds without -O2 and fails with -O2.
>   5. Continue minimizing, with -Wall enabled. Typically the end result will
>  be a small file, 10-20 lines of code.
> 
> If you got that far and can argue (by the standards) that it is a GCC bug,
> open a bug report. The GCC shipped by Fedora is not an upstream release;
> nevertheless Paul opens his bug reports about Fedora's gcc directly in the
> upstream bug tracker, e.g.
> . You can do the same.

Thanks for the information! I did a little expirementing with this program:


#include 
#include 

int
main (void)
{
  char buffer[5000];
  wint_t ch = (wint_t) L'\0';
  int result = sprintf (buffer, "%lc%lc%lc%lc", ch, ch, ch, ch);
  printf ("%d\n", result);
  return 0;
}


I think I did decent at narrowing it down:

$ make
gcc -Wall -g -O2 -fprintf-return-value -o a.out main.c
./a.out
0
$ make
gcc -Wall -g -O0 -o a.out main.c
./a.out
4
$ make
gcc -Wall -g -O1 -o a.out main.c
./a.out
0
$ make
gcc -Wall -g -O1 -fprintf-return-value -o a.out main.c
./a.out
0
$ make
gcc -Wall -g -O1 -fno-printf-return-value -o a.out main.c
./a.out
4

If I understand correctly, sprintf should return 4 here correct? And
the buffer should have 5 NUL bytes. 4 from the arguments and 1
trailing, which is excluded from the return value.

Collin

Pacify -Wmissing-variable-declarations in unit tests.

[Collin Funk]

When building Coreutils I noticed that GCC 14's
-Wmissing-variable-declarations option spams the test output.

Can I apply the attached patch which adds the 'static' specifier to
global variables in unit tests?

Or would it be better to just disable the warning in tests? I'm used
to marking things static even in small single file programs, but maybe
others will forget the convention.

I think a few of the unistr, etc. tests also trigger this warning. I
left them unchanged since IIRC most are auto-generated.

CollinFrom a81727a18045c64b56f2bfd2cebbf61bb072f529 Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Sat, 27 Apr 2024 23:37:38 -0700
Subject: [PATCH] Pacify -Wmissing-variable-declarations in unit tests.

* tests/jit/test-cache.c: Add a static specifier to global variables.
* tests/minus-zero.h: Likewise.
* tests/test-acos.c: Likewise.
* tests/test-acosf.c: Likewise.
* tests/test-acosl.c: Likewise.
* tests/test-alignasof.c: Likewise.
* tests/test-alignof.c: Likewise.
* tests/test-alloca-opt.c: Likewise.
* tests/test-argp-version-etc.c: Likewise.
* tests/test-argp.c: Likewise.
* tests/test-asin.c: Likewise.
* tests/test-asinf.c: Likewise.
* tests/test-asinl.c: Likewise.
* tests/test-asyncsafe-spin1.c: Likewise.
* tests/test-atan.c: Likewise.
* tests/test-atan2.c: Likewise.
* tests/test-atan2f.c: Likewise.
* tests/test-atanf.c: Likewise.
* tests/test-atanl.c: Likewise.
* tests/test-cbrt.h: Likewise.
* tests/test-copysign.c: Likewise.
* tests/test-copysignf.c: Likewise.
* tests/test-copysignl.c: Likewise.
* tests/test-cos.c: Likewise.
* tests/test-cosf.c: Likewise.
* tests/test-cosh.c: Likewise.
* tests/test-coshf.c: Likewise.
* tests/test-cosl.c: Likewise.
* tests/test-erf.c: Likewise.
* tests/test-erfc.c: Likewise.
* tests/test-errno.c: Likewise.
* tests/test-exclude.c: Likewise.
* tests/test-exp.h: Likewise.
* tests/test-exp2.h: Likewise.
* tests/test-expm1.h: Likewise.
* tests/test-fabs.h: Likewise.
* tests/test-fcntl-h.c: Likewise.
* tests/test-fenv.c: Likewise.
* tests/test-float.c: Likewise.
* tests/test-fma2.c: Likewise.
* tests/test-fmaf2.c: Likewise.
* tests/test-fmal2.c: Likewise.
* tests/test-fmod.h: Likewise.
* tests/test-fnmatch-h.c: Likewise.
* tests/test-gc-pbkdf2-sha1.c: Likewise.
* tests/test-gc-pbkdf2.c: Likewise.
* tests/test-get-rusage-as.c: Likewise.
* tests/test-get-rusage-data.c: Likewise.
* tests/test-getentropy.c: Likewise.
* tests/test-getrusage.c: Likewise.
* tests/test-glob-h.c: Likewise.
* tests/test-hypot.h: Likewise.
* tests/test-iconv-utf.c: Likewise.
* tests/test-ilogb.h: Likewise.
* tests/test-intprops.c: Likewise.
* tests/test-inttypes.c: Likewise.
* tests/test-isfinite.c: Likewise.
* tests/test-isinf.c: Likewise.
* tests/test-iswblank.c: Likewise.
* tests/test-j0.c: Likewise.
* tests/test-j1.c: Likewise.
* tests/test-jn.c: Likewise.
* tests/test-langinfo.c: Likewise.
* tests/test-lgamma.c: Likewise.
* tests/test-limits-h.c: Likewise.
* tests/test-locale.c: Likewise.
* tests/test-log.h: Likewise.
* tests/test-log10.h: Likewise.
* tests/test-log1p.h: Likewise.
* tests/test-log2.h: Likewise.
* tests/test-logb.h: Likewise.
* tests/test-malloca.c: Likewise.
* tests/test-modf.h: Likewise.
* tests/test-nan-1.c: Likewise.
* tests/test-nan-2.c: Likewise.
* tests/test-netdb.c: Likewise.
* tests/test-nextafter.c: Likewise.
* tests/test-noreturn.c: Likewise.
* tests/test-nullptr.c: Likewise.
* tests/test-pathmax.c: Likewise.
* tests/test-poll-h.c: Likewise.
* tests/test-pow.c: Likewise.
* tests/test-powf.c: Likewise.
* tests/test-pthread.c: Likewise.
* tests/test-remainder.h: Likewise.
* tests/test-sched.c: Likewise.
* tests/test-sethostname1.c: Likewise.
* tests/test-signal-h.c: Likewise.
* tests/test-signbit.c: Likewise.
* tests/test-sigpipe.c: Likewise.
* tests/test-sigsegv-catch-segv1.c: Likewise.
* tests/test-sigsegv-catch-segv2.c: Likewise.
* tests/test-sin.c: Likewise.
* tests/test-sinf.c: Likewise.
* tests/test-sinh.c: Likewise.
* tests/test-sinhf.c: Likewise.
* tests/test-sinl.c: Likewise.
* tests/test-snan-1.c: Likewise.
* tests/test-snan-2.c: Likewise.
* tests/test-spawn.c: Likewise.
* tests/test-sqrt.h: Likewise.
* tests/test-stdbool.c: Likewise.
* tests/test-stddef.c: Likewise.
* tests/test-stdint.c: Likewise.
* tests/test-stdio.c: Likewise.
* tests/test-string-buffer.c: Likewise.
* tests/test-sys_random.c: Likewise.
* tests/test-sys_resource.c: Likewise.
* tests/test-sys_select.c: Likewise.
* tests/test-sys_socket.c: Likewise.
* tests/test-sys_stat.c: Likewise.
* tests/test-sys_time.c: Likewise.
* tests/test-sys_types.c: Likewise.
* tests/test-sys_uio.c: Likewise.
* tests/test-sysexits.c: Likewise.
* tests/test-tan.c: Likewise.
* tests/test-tanf.c: Likewise.
* tests/test-tanh.c: Likewise.
* tests/test-tanhf.c: Likewise.
* tests/test-tanl.c: Likewise.
* tests/test-termios.c: Likewise.
* tests/test-thrd_current.c: Likewise.
* tests/test-thread_self.c: Likewise.
* tests/test-threads.c: Likewise.
* tests/test-time-h.c: Likewise.
* tests/test-uchar.c: Likewise.
*