Re: GNU gnulib: gnulib-tool has become much faster

[Tim Rühsen]

Bruno, Dmitry, Collin, thank you so much !

The runtime improvement is amazing :)

Regards, Tim

On 4/26/24 12:14, Bruno Haible wrote:

If you are developer on a package that uses GNU gnulib as part of its build
system:

gnulib-tool has been known for being slow for many years. We have listened to
your complaints. We have rewritten gnulib-tool in another programming language
(Python). It is between 8 times and 100 times faster than the previous
implementation.

Both implementations behave identically, that is, produce the same generated
files and the same output. Nothing changes in your way to use Gnulib; it's
only faster.

In order to reap the new speed:

1. Make sure you have Python (version 3.7 or newer) installed on your
machine.

2. Update your gnulib checkout. (For some packages, it comes as a git
submodule named 'gnulib'.) Like this:

   $ git checkout master
   $ git pull

   Set the environment variable GNULIB_SRCDIR, pointing to this checkout.

   If the package is using a git submodule named 'gnulib', it is also advisable
to do

   $ git commit -m 'build: Update gnulib submodule to latest.' gnulib

   (as a preparation for step 4, because the --no-git option does not work as
expected in all variants of 'bootstrap').

3. Clean the built files of your package:

   $ make -k distclean


4. Regenerate the fetched and generated files of your package. Depending on
the package, this may be a command such as

   $ ./bootstrap --no-git --gnulib-srcdir=$GNULIB_SRCDIR

   or

   $ export GNULIB_SRCDIR; ./autopull.sh; ./autogen.sh

   or, if no such script is available:

   $ $GNULIB_SRCDIR/gnulib-tool --update


5. Continue with

   $ ./configure
   $ make

   as usual.

Enjoy! The rewritten gnulib-tool was implemented by Dmitry Selyutin, Collin
Funk, and me.



___
Message sent via Savannah
https://savannah.gnu.org/




OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: Shallow git update in bootstrap

[Tim Rühsen]

On 8/6/23 00:25, Carles Pina i Estany wrote:

When I say "long time" (and data transmission) in my case it's 9
minutes:
-
carles@pinux:[master]~/git/wget2$ time git submodule update --init
Cloning into '/home/carles/git/wget2/gnulib'...
Submodule path 'gnulib': checked out '2ae6faf9c78384dc6a2674b62dd56ff153cd51f6'

real9m1,135s
user6m24,309s
sys 0m5,020s
-


Not answering your question, but may be helpful:

If you regularly build wget2 from git, you only have to do download 
gnulib once. When the gnulib submodule becomes updated by the project 
(happens from time to time), only the missing parts are downloaded by 
git, which should be fast even on slow network connections.


Another option is to git clone gnulib into a separate directory outside 
the project directory and set the env variable GNULIB_REFDIR to this 
directory (e.g. "export GNULIB_REFDIR=/home/carles/git/gnulib").
When needed (or eventually), use "git pull" from inside the gnulib 
directory to update it.


The `./bootstrap` script in the wget2 project then fetches the needed 
gnulib commits from from $GNULIB_REFDIR.


To speed things up in container CI environments:
If containers are only used once, git clone gnulib at image creation 
time and do "rmdir gnulib && mv /gnulib . && git submodule update 
gnulib" in the container. This is still experimental, just started using 
it yesterday without experiencing any downsides so far.


Regards, Tim


OpenPGP_signature
Description: OpenPGP digital signature

Re: Including config.h emits warnings from -Wundef

[Tim Rühsen]

On 8/3/23 20:56, Jeffrey Walton wrote:

My personal opinion is, there's nothing to fix.

That is exactly how macros are supposed to work. Anything undefined
evaluates to 0. It has been that way since the early days of C and
C++.

If there's a fix, then it is that you don't use -Wundef.


-Wundef helps to find typos if #ifdef constructs quickly.

Without it you easily end up debugging issues that the compiler could 
have caught at compile time. This actually happened to me in the past, 
I'd like to take this kind of issue out of the game once and for all :).


Regards, Tim


OpenPGP_signature
Description: OpenPGP digital signature

Re: Including config.h emits warnings from -Wundef

[Tim Rühsen]

Awesome, thank you Bruno !

Tested and your patch fixes the issue(s) for me.

Tim

On 8/3/23 18:11, Bruno Haible wrote:

On Donnerstag, 3. August 2023 17:52:29 CEST Tim Rühsen wrote:

Hi,

I just updated gnulib and recognized these warnings with every '#include
'. Before the update there were no warnings.

../config.h:3317:34: warning: "__cplusplus" is not defined, evaluates to
0 [-Wundef]
   3317 | #if !defined HAVE_C_ALIGNASOF && __cplusplus < 201103 &&
!defined alignof
|  ^~~
../config.h:3318:6: warning: "HAVE_STDALIGN_H" is not defined, evaluates
to 0 [-Wundef]
   3318 | # if HAVE_STDALIGN_H
|  ^~~
../config.h:3415:7: warning: "HAVE_STDALIGN_H" is not defined, evaluates
to 0 [-Wundef]
   3415 | # if !HAVE_STDALIGN_H
|   ^~~
../config.h:3424:6: warning: "_GL_STDALIGN_NEEDS_STDDEF" is not defined,
evaluates to 0 [-Wundef]
   3424 | # if _GL_STDALIGN_NEEDS_STDDEF
|  ^

Disabling -Wundef for the whole project is not my preferred solution =).
Using a #pragma around the #include is ugly and tedious (too many files
to change).


Using a #pragma include config.h would also be ugly.


Can this be fixed in gnulib ?


Done:


2023-08-03  Bruno Haible  

alignasof, stdalign: Avoid some -Wundef warnings from config.h.
Reported by Tim Rühsen  in
<https://lists.gnu.org/archive/html/bug-gnulib/2023-08/msg00012.html>.
* m4/stdalign.m4 (gl_ALIGNASOF): Test whether __cplusplus is defined
before evaluating it. Assume HAVE_STDALIGN_H, _GL_STDALIGN_NEEDS_STDDEF
are never defined to 0.

diff --git a/m4/stdalign.m4 b/m4/stdalign.m4
index 1a236d66d2..6a39ffe756 100644
--- a/m4/stdalign.m4
+++ b/m4/stdalign.m4
@@ -68,8 +68,10 @@ AC_DEFUN([gl_ALIGNASOF]
dnl The "zz" puts this toward config.h's end, to avoid potential
dnl collisions with other definitions.
AH_VERBATIM([zzalignas],
-[#if !defined HAVE_C_ALIGNASOF && __cplusplus < 201103 && !defined alignof
-# if HAVE_STDALIGN_H
+[#if !defined HAVE_C_ALIGNASOF \
+&& !(defined __cplusplus && 201103 <= __cplusplus) \
+&& !defined alignof
+# if defined HAVE_STDALIGN_H
  #  include 
  # endif
  
@@ -166,7 +168,7 @@ AC_DEFUN([gl_ALIGNASOF]

  #   define _Alignas(a) __declspec (align (a))
  #  endif
  # endif
-# if !HAVE_STDALIGN_H
+# if !defined HAVE_STDALIGN_H
  #  if ((defined _Alignas \
  && !(defined __cplusplus \
   && (201103 <= __cplusplus || defined _MSC_VER))) \
@@ -175,7 +177,7 @@ AC_DEFUN([gl_ALIGNASOF]
  #  endif
  # endif
  
-# if _GL_STDALIGN_NEEDS_STDDEF

+# if defined _GL_STDALIGN_NEEDS_STDDEF
  #  include 
  # endif
  #endif])





OpenPGP_signature
Description: OpenPGP digital signature

Including config.h emits warnings from -Wundef

[Tim Rühsen]

Hi,

I just updated gnulib and recognized these warnings with every '#include 
'. Before the update there were no warnings.


../config.h:3317:34: warning: "__cplusplus" is not defined, evaluates to 
0 [-Wundef]
 3317 | #if !defined HAVE_C_ALIGNASOF && __cplusplus < 201103 && 
!defined alignof

  |  ^~~
../config.h:3318:6: warning: "HAVE_STDALIGN_H" is not defined, evaluates 
to 0 [-Wundef]

 3318 | # if HAVE_STDALIGN_H
  |  ^~~
../config.h:3415:7: warning: "HAVE_STDALIGN_H" is not defined, evaluates 
to 0 [-Wundef]

 3415 | # if !HAVE_STDALIGN_H
  |   ^~~
../config.h:3424:6: warning: "_GL_STDALIGN_NEEDS_STDDEF" is not defined, 
evaluates to 0 [-Wundef]

 3424 | # if _GL_STDALIGN_NEEDS_STDDEF
  |  ^

Disabling -Wundef for the whole project is not my preferred solution =).
Using a #pragma around the #include is ugly and tedious (too many files 
to change).


Can this be fixed in gnulib ?

Regards, Tim


OpenPGP_signature
Description: OpenPGP digital signature

Re: cnd_timedout returns immediately when built with MinGW

[Tim Rühsen]

On 06.08.22 19:40, Bruno Haible wrote:

Tim Rühsen wrote:

I read that even the MS C compilers / libraries support ISO C thread
nowadays,


I can't confirm this. If it were true, they would have documentation for it.
But a web search for "mtx_lock site:microsoft.com" returns no meaningul
results.


You are right. After diving deeper into it, I'd say I was blinded by 
some overly-

enthusiastic comments on social media.




Testing with wine, as I don't own a Windows license.
Have to carry a USB stick to a friend in order to test on real Windows
(trying to avoid that).


Yes, working like this is tedious.


'--enable-threads=windows' does not work with MinGW since 'threads.h' is
not available via gnulib then.


?? What do you mean? You have imported the 'threads' module, and it provides
a  file.

When I create a testdir for the 'threads' module and build it on mingw,
all tests pass, except for a hanging 'test-nanosleep'.

Maybe you are lacking a -I directive to the build directory where threads.h
has been generated?


Sorry, when using the shortcut of `autoconf -fi` after adding 
gl_AVOID_WINPTHREAD, lib/threads.h was gone.

A './boostrap' fixed it.

I will test '--enable-threads=windows' on real Windows in the next days, 
but TBH, it feels like the C11 threads experiment failed :-|


Maybe there is a (effective) way to rewrite the inter-thread 
communication avoiding cond_timedwait.


Regards, Tim



Bruno





OpenPGP_signature
Description: OpenPGP digital signature

Re: cnd_timedout returns immediately when built with MinGW

[Tim Rühsen]

Hi Bruno,

On 05.08.22 20:27, Bruno Haible wrote:

Hi Tim,


I am at switching wget2 multithreading from C99+glthreads to C11
(threads.h).


I hope that you have considered the Gnulib documentation on this topic:
https://www.gnu.org/software/gnulib/manual/html_node/Choosing-a-multithreading-API.html


Thanks for the pointer. I used the gnulib API in the past and created 
API wrappers for libwget,
so that apps didn't need to use gnulib directly. Since Wget2 is the only 
app I know that uses libwget,
I recently decided to go with ISO C (C11) in the future and drop the 
libwget thread API completely.


The rationale was
- gnulib is C11, so the libwget code is
- less code = less maintenance
- the library code just uses the basic thread, condition and mutex API

I read that even the MS C compilers / libraries support ISO C thread 
nowadays,

so ISO C seems OK to me.




The tests work well on Linux (amd64) (Debian unstable/testing/stable,
Fedora 35, Arch, Alpine/muslc) with native builds (gcc or clang).

But they completely fail when cross-building with MinGW64 (on Debian
bookworm).


And what's the execution environment? Is it Windows or is it wine? wine is
not supported; I've had enough work making it work on Windows.


Testing with wine, as I don't own a Windows license.
Have to carry a USB stick to a friend in order to test on real Windows 
(trying to avoid that).


And yeah, I can fully understand that wine isn't directly supported. It 
seems to be

a can of worms on it's own.


Also, are you configuring with --enable-threads=windows? If yes, then
it should work (on Windows). If not, then if the winpthreads library
is found, it will be used; but since this library is broken, some of the
tests will hang, crash, or fail. To avoid using this broken library,
add this macro invocation to your configure.ac:
   gl_AVOID_WINPTHREAD


'--enable-threads=windows' does not work with MinGW since 'threads.h' is 
not available via gnulib then.
MinGW also doesn't provide it. It's not 100% clear to me what this 
option actually does.


I'll use gl_AVOID_WINPTHREAD since there are reports from Windows users 
regarding multi-threading not working

as expected (using the gnulib API).

Thank you, I appreciate your help & hints !

Regards, Tim



Bruno






OpenPGP_signature
Description: OpenPGP digital signature

cnd_timedout returns immediately when built with MinGW

[Tim Rühsen]

Hi,

I am at switching wget2 multithreading from C99+glthreads to C11 
(threads.h).


The tests work well on Linux (amd64) (Debian unstable/testing/stable, 
Fedora 35, Arch, Alpine/muslc) with native builds (gcc or clang).


But they completely fail when cross-building with MinGW64 (on Debian 
bookworm).


It looks like cnd_timedwait() always immediately returns with 1, no 
matter what timeout is specified. The cond is initialized and the mutex 
is locked.


Using cnd_wait() instead of cnd_timedwait() works well, but I'd prefer 
having using a timeout to catch some corner cases.


Is this a known issue ?

Regards, Tim


OpenPGP_signature
Description: OpenPGP digital signature

Re: Parallelization of shell scripts for 'configure' etc.

[Tim Rühsen]

Hi all,

On 14.06.22 00:39, Paul Eggert wrote:
In many Gnu projects, the 'configure' script is the biggest barrier to 
building because it takes s long to run. Is there some way that we 
could improve its performance without completely reengineering it, by 
improving Bash so that it can parallelize 'configure' scripts?


A faster configure script execution indeed is something I'd love to see.
The title of this thread infers that we *only* want to discuss 
parallelization - maybe we can generalize this to "Making configure 
scripts run faster" ?


[A little side-note: the invocation of gnulib-tool is *far* slower than 
the running the configure scripts, for the projects that I work on.

But surely this is a problem on it own.]

I see two main setups when running configure scripts. How to speeding up 
the execution has several possible solutions for each of the setups (but 
with overlaps of course).


a) The maintainer/contributor/hacker setup
This is when you re-run configure relatively often for the same project(s).
I do this normally and and came up with 
https://gitlab.com/gnuwget/wget2/-/wikis/Developer-hints:-Increasing-speed-of-GNU-toolchain. 
It may be a bit outdated, but may help one or the other here.

Btw, I am down to 2.5s for a ./configure run from 25s originally.

b) The one-time build setup
This is people building + installing from tarball and automated build 
systems (e.g. CI) with regular OS updates. I also think of systems like 
Gentoo where you build everything from source.
As Alex Ameen pointed out, using a global configure cache across 
different projects may be insecure.

Also people often want to use optimization in this case.
Installing ccache is also not likely when people just want to 
build+install a single project.


I personally see a) as solved, at least for me.

b) is a problem because
1. People start to complain about the slow GNU build system (autotools), 
which drives new projects away from using autotools and possible it 
drives people away from GNU in general. Or in other words: let's not eat 
up people's precious time unnecessarily when building our software.


2. Building software in large scale eats tons of energy. If we could 
reduce the energy consumption, it gives us at least a better feeling.



What can we do to solve b)
I guess we first need to analyze/profile the configure execution.
For this I wrote a little tool some years ago: 
https://gitlab.com/rockdaboot/librusage.
It's simple to build and use and gives some number of which (external) 
commands are executed - fork+exec are pretty heavy.
[Configure for wget2 runs 'rm' and 'cat' each roughly 2000x - so I came 
up with enabling plugins for those two commands (had to write a plugin 
for 'rm', not sure if it never has been accepted by bash upstream).]
Maybe be we can create plugins for other highly used commands as well 
e.g. for sed !?


The output of the tool also roughly allows to see where the time goes - 
it's beyond my spare time to go deeper into this right now.

Please test yourself and share some numbers.

Another option is to group tests, e.g. if test 1 is X, we also know the 
results for tests 2,3,4,... Or we group several tests into a single C 
file, if possible. Just an idea (sounds a bit tedious, though).


Parallelism... can't we do that with &, at least for well-known / 
often-used tests ?


Family calls...

Regards, Tim


OpenPGP_signature
Description: OpenPGP digital signature

Re: Some generated header files are messed up on Alpine

[Tim Rühsen]

On 03.01.22 00:38, Bruno Haible wrote:

Paul Eggert wrote:

I'd like to try the approach a bit more (especially as this prompted me
to simplify it a bit :-). So I installed the first attached patch to
Gnulib, to work around the bug in BusyBox 'sed'.


Thanks. I confirm that it works on Alpine Linux. Unlike me, you succeeded
in keeping the GNU make optimization :-)


Just want to confirm that it works here as well.

Thank you for the quick fix !

Regards, Tim


OpenPGP_signature
Description: OpenPGP digital signature

Some generated header files are messed up on Alpine

[Tim Rühsen]

Hi,

I just updated gnulib to commit 2671376bc and have build issues on 
Alpine (using docker; FROM alpine:latest).

Not saying this commit is faulty, I didn't update gnulib since April 2021.

Some generated header files look like

bash-5.1# head -3 lib/uniwidth.h
/* DO NOT EDIT! GENERATED AUTOMATICALLY! */
 2001-2002, 2005, 2007, 2009-2021 Free Software Foundation,
   Inc.

This obviously won't compile.

Another example is:

bash-5.1# head -3 lib/unitypes.h
/* DO NOT EDIT! GENERATED AUTOMATICALLY! */
niString library.
   Copyright (C) 2002, 2005-2006, 2009-2021 Free Software Foundation, Inc.

I tested with ./bootstrap from gnulib/build-aux/.
This issue does not appear on Debian Linux (stable, testing, unstable).

If this is not reproducible or the issue is not obvious, please let me 
know so that I can provide a Dockerfile (or docker image) plus 
instructions on how to reproduce.


Regards, Tim


OpenPGP_signature
Description: OpenPGP digital signature

Re: implicit declaration of function 'utime' in trailing slashes test

[Tim Rühsen]

On 23.01.21 07:09, Ryan Schmidt wrote:

On Jan 22, 2021, at 16:47, Tim Rühsen wrote:


On 21.01.21 01:34, Ryan Schmidt wrote:

Hi, I'm the maintainer of wget in MacPorts.
In the version of clang included with Xcode 12 and later, implicit declaration 
of functions is an error.
During configure, wget 1.12.1 prints this:
checking whether utime handles trailing slashes on files... no
config.log contains this:
configure:49368: checking whether utime handles trailing slashes on files
configure:49414: ccache /usr/bin/clang -o conftest -DNDEBUG -pipe -Os 
-Werror=implicit-function-declaration -arch x86_64 -I/opt/local/include 
-L/opt/local/lib -Wl,-headerpad_max_install_names -arch x86_64 conftest.c  >&5
conftest.c:491:23: error: implicit declaration of function 'utime' is invalid 
in C99 [-Werror,-Wimplicit-function-declaration]
  if (!utime ("conftest.tmp/", NULL))
   ^
1 error generated.
configure:49414: $? = 1
configure: program exited with status 1
So the results of this test don't represent what you want them to.
This configure test uses #include . The problem goes away if I change that to 
#include . Then configure output is still:
checking whether utime handles trailing slashes on files... no
but config.log then contains:
configure:49368: checking whether utime handles trailing slashes on files
configure:49414: ccache /usr/bin/clang -o conftest -DNDEBUG -pipe -Os 
-Werror=implicit-function-declaration -arch x86_64 -I/opt/local/include 
-L/opt/local/lib -Wl,-headerpad_max_install_names -arch x86_64 conftest.c  >&5
configure:49414: $? = 0
configure:49414: ./conftest
configure:49414: $? = 2
configure: program exited with status 2
Now the results of the test are accurate.
I included this patch in the MacPorts wget 1.12.1 port:
https://github.com/macports/macports-ports/blob/13fd7facb9e1ea9e70b79c8c0b429058b9bb8698/net/wget/files/implicit.patch
Of course you'll want to patch m4/utime.m4 instead of configure.


./configure scripts are not made for using -Werror in CFLAGS.

This is documented somewhere but (sorry), I am just too tired to search for the 
URL right now.


Unfortunately, you no longer have much of a choice. Treating implicit 
declarations of functions as an error is the *default* behavior of the compiler 
included with Xcode 12 and later.

In the output I showed above I have specified 
-Werror=implicit-function-declaration manually in CFLAGS because I am using an 
earlier version of Xcode but I want to simulate the experience of an Xcode 12 
user so that I can identify and fix this type of problem.

Apple intentionally changed the default behavior of the compiler in order to be able to 
ship Macintosh computers with ARM processors ("Apple Silicon"). ARM processors 
have different calling conventions for variadic and non-variadic functions, so the 
compiler must know before you call a function what type of function it is, and the way it 
knows that is by the function declaration.

The C99 standard requires that you declare functions before you use them. It 
has been this way for over 2 decades. wget should do so in all of its code, 
regardless of whether the code is in a configure test or elsewhere.

This one configure test that I mentioned is the only place where I noticed a 
problem, so as far as I know only the one-byte change I proposed above needs to 
be made.


Thanks for the explanation, that makes it clearer now (and possibly my 
brain is not as mushed as during my last read/answer).


The code involved here comes directly from gnulib and is not part of the 
wget project. The issue affects not only wget and any patching here is 
mostly lost time/efford.


So I am adding the gnulib mailing list to the recipients as IMO this 
issue should be discussed/answered there. It will be read by people with 
more related expertise than I can provide.


Regards, Tim



OpenPGP_signature
Description: OpenPGP digital signature

Re: signal.h:682:13: error: size of array 'verify_NSIG_constraint' is negative

[Tim Rühsen]

On 02.08.20 15:41, Bruno Haible wrote:
> Tim Rühsen wrote:
>> The surrounding code in lib/signal.h is
>> #if 1
>> # if !0
> 
> Huh? HAVE_POSIX_SIGNALBLOCKING evaluates to 0 ?!
> 
> There was a change to m4/signalblocking.m4 recently, but for me it works.
> Can you take a look at the "checking for sigprocmask..." test in
> config.log?

Oh ouch, my bad.

Just recognized that i use a global configure cache that i didn't
remove. I assume that all the CI jobs suffer from the same.
This rarely happens - but updating gnulib is definitely worth removing
all configure caches.

I am really sorry to have wasted your time. The cache just didn't pop
into my mind :-(

Thank you very much !

Regards, Tim



signature.asc
Description: OpenPGP digital signature

signal.h:682:13: error: size of array 'verify_NSIG_constraint' is negative

[Tim Rühsen]

Hey,

after updating gnulib, I see this on all CI pipelines (Debian, Fedora,
Alpine, Arch, ...):

In file included from /usr/include/x86_64-linux-gnu/sys/param.h:28,
 from ./minmax.h:34,
 from md2.c:30:
./signal.h:682:13: error: size of array 'verify_NSIG_constraint' is negative
  682 | typedef int verify_NSIG_constraint[NSIG <= 32 ? 1 : -1];
  | ^~
make[3]: *** [Makefile:2261: md2.lo] Error 1


The surrounding code in lib/signal.h is
#if 1
# if !0

#  ifndef GNULIB_defined_signal_blocking
#   define GNULIB_defined_signal_blocking 1
#  endif

/* Maximum signal number + 1.  */
#  ifndef NSIG
#   define NSIG 32
#  endif

/* This code supports only 32 signals.  */
#  if !GNULIB_defined_verify_NSIG_constraint
typedef int verify_NSIG_constraint[NSIG <= 32 ? 1 : -1];
#   define GNULIB_defined_verify_NSIG_constraint 1
#  endif

# endif


Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: continuous integration

[Tim Rühsen]

Hi Ed,

On 11.06.20 21:23, Ed Maste wrote:
> On Thu, 26 Mar 2020 at 05:51, Tim Rühsen  wrote:
>>
>> We just need a mirror / fork on Github that we push to (sync) from time
>> to time. If someone cares for the initial Travis and/or Cirrus setup
>> with OSX / FreeBSD / Windows in mind, that would be great !
> 
> If such a fork/mirror is going to show up I'll add a .cirrus.yml to
> get at least FreeBSD coverage (and will add others if I manage to get
> them going easily).

You can mirror gnulib on your own Github account to add/test Cirrus CI.
I am not sure that gnulib will have an official account on Github, as
Github is very proprietary.

Another option is to get access to a FreeBSD machine and install the
Gitlab runner [1]. With that (plus a token from our Gitlab account), the
machine should appear on our list of CI runners and we can integrate it
into the .gitlab-ci.yml.

Regards, Tim

[1] https://docs.gitlab.com/runner/install/



signature.asc
Description: OpenPGP digital signature

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

Hi Bruno,

On 23.05.20 22:47, Bruno Haible wrote:
> Tim Rühsen wrote:
>> - clang-10 with unset CFLAGS (*.clang-10)
>>
>> And the same set with -fsanitize... (*.sanitize).
> 
> The other differences between the configure results without and with 
> sanitizers
> can be explained by the fact that these -fsanitize options have the effect 
> that
> the resulting executables are linked with
>   - libpthread,
>   - librt
>   - libm
>   - libdl
> by default.

...

> 
> So, all in all, it's more harmless that it looked at first sight. But
> it showed 9 bugs in gnulib, which are now all fixed.

Thank you so much for investigating and fixing the issues !

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: coreutils and GCC -fanalyzer

[Tim Rühsen]

On 23.05.20 18:08, Paul Eggert wrote:
> On 5/23/20 8:56 AM, Pádraig Brady wrote:
>> Note -Wanalyzer... flags are ineffective without -fanalyzer.
>> Also -Wanalyzer-too-complex (now auto enabled by gnulib) is currently quite
>> problematic with -fanalyzer.
> 
> Hmm, that's odd, since I recall getting more warnings with those flags 
> enabled.
> Perhaps not all of them are disabled when you don't use -fanalyzer? Or maybe 
> my
> memory is faulty.
> 
> Anyway, cleary the Gnulib manywarnings module needs to be fixed to do a better
> job here, and this reminds me of another problem that has been bothering me 
> for
> quite some time: ./configure spends way too much time testing for individual
> warnings when the manywarnings module is in use. So I am thinking of killing 
> two
> stones by doing the following.
> 
> 1. Test for -fanalyzer, -Wall, -Wextra.
> 
> 2. Test for flags not automatically enabled by -fanalyzer, -Wall, -Wextra but
> flags that we want anyway.
> 
> 3. Test for flags automatically enabled by -fanalyzer, -Wall, -Wextra that are
> also flags that we don't want.
> 
> This should make 'configure' much faster. It'll also fix the issue you
> mentioned. And we can make sure that -Wanalyzer-too-complex is not mentioned 
> in
> step (2) so that Gnulib won't generate it.

This has been done for Wget2
https://gitlab.com/gnuwget/wget2/-/blob/master/m4/wget_manywarnings.m4

Example usage at L123+:
https://gitlab.com/gnuwget/wget2/-/blob/master/configure.ac

To make it usable for other projects, it needs some merging with the
manywarnings.m4 from gnulib, IMO. I would support any volunteer as good
as i can.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

Hi Bruno,

On 21.05.20 21:31, Bruno Haible wrote:
> Hi Tim,
> 
> These comparisons are not so useful, because they not only come from different
> compilers (gcc vs. clang) but also from systems with different libcs: there
> are differences regarding libm, calloc, thrd_create, pthread_sigmask, and 
> other
> functions. It would be more useful to compare, on the _same_ system:
>   - gcc vs. clang,
>   - clag vs. clang -fsanitize=...

Well, all made on the _same_ system (my desktop) with just ~35 minutes
in between.

Since system updates are made manually, it is _very_ unlikely that I
made an update between the tests.

But yes, I made one test without $CC set (so likely with gcc) and the
other one with CC=clang and CFLAGS=-fsanitize...

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

Hi Bruno,

On 18.05.20 21:44, Bruno Haible wrote:
> Hi Tim,
> 
>>> The way to determine the answer is:
>>> 1. Create a test dir of all gnulib modules.
>>> 2. Configure it with --config-cache.
>>> 3. Configure it with --config-cache and your sanitizer options.
>>> 4. Compare the generated config.cache and config.status files.
>>
>> I am short on time and would like to prevent being distracted too much
> 
> Everyone here is probably in the same situation...
> 
>> If you could give me a quick instruction on how to do 1.
> 
> I typically use this command:
> 
>   rm -rf ../testdir-all; ./gnulib-tool --create-testdir --dir=../testdir-all 
> --single-configure
> 
>> I'll happily
>> go for steps 2-4. I already have a fresh cloned gnulib locally.
> 
> Thanks! We can then go through the findings one by one.

With my getdelim patch applied, this is the summary of sanitizer findings:

$ egrep 'SUMM|checking' config.log|grep -B1 SUMM
configure:14038: checking for working C stack overflow detection
SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
conftest.c:379:30 in
--
configure:36833: checking whether memmem works
SUMMARY: UndefinedBehaviorSanitizer: invalid-null-argument
conftest.c:513:32 in
--
configure:55268: checking whether fchownat works with AT_SYMLINK_NOFOLLOW
SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
conftest.c:812:50 in
configure:55326: checking whether fchownat works with an empty file name
SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change
conftest.c:817:37 in
--
configure:67322: checking whether glob lists broken symlinks
SUMMARY: AddressSanitizer: 37 byte(s) leaked in 2 allocation(s).
--
configure:94107: checking for working re_compile_pattern
SUMMARY: AddressSanitizer: 20072 byte(s) leaked in 116 allocation(s).


The lzip'ed tar archive with all 4 files is 137k. Is it ok to send it
here or do you better like via PM ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

Hi Bruno,

On 20.05.20 00:46, Bruno Haible wrote:
> Hi Tim,
> 
>>>   rm -rf ../testdir-all; ./gnulib-tool --create-testdir 
>>> --dir=../testdir-all --single-configure
>>
>> This results in:
>>
>> executing autopoint --force
>> autopoint: *** The AM_GNU_GETTEXT_VERSION declaration in your configure.ac
>>file requires the infrastructure from gettext-0.20 but
>> this version
>>is older. Please upgrade to gettext-0.20 or newer.
>> autopoint: *** Stop.
> 
> You may try to pass '--avoid=gettext' to overcome this.

./configure --config-cache ends (stops ?) with
The BISON_I18N macro is used without being preceded by AM_GNU_GETTEXT.

$ ls -la config.cache
-rw-r--r-- 1 tim tim 0 Mai 20 20:40 config.cache

Is that expected ?

And I see this output here as well:

gnulib-tool: warning: module euidaccess depends on a module with an
incompatible license: group-member

> 
>> As Debian unstable is at gettext 0.19.8.1, I tried to build gettext from
>> git master. This results in
>>
>> make[7]: Entering directory
>> '/home/tim/src/gettext/gettext-tools/examples/tmp-hello-pascal'
>> make[7]: warning: jobserver unavailable: using -j1.  Add '+' to parent
>> make rule.
>> LOCALEDIR='/usr/local/share/locale' /usr/bin/ppcx64 -o./hello ./hello.pas
>> Free Pascal Compiler version 3.0.4+dfsg-23 [2019/11/25] for x86_64
>> Copyright (c) 1993-2017 by Florian Klaempfl and others
>> Target OS: Linux for x86-64
>> Compiling ./hello.pas
>> hello.pas(9,6) Fatal: Can't find unit gettext used by hello
>> Fatal: Compilation aborted
>> make[7]: *** [Makefile:798: hello.rsj] Error 1
>>
>>
>> Is it really needed to fail the whole build just because a *PASCAL*
>> example doesn't compile ?
> 
> I may sound a bit old-fashioned when I recommend tarballs over building from
> git checkouts. But sometimes it has its advantages. gettext 0.20.2 is not
> that old. [1]
> 
> Bruno
> 
> [1] https://ftp.gnu.org/gnu/gettext/gettext-0.20.2.tar.gz

I will install that version and try again.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

On 18.05.20 21:44, Bruno Haible wrote:
> Hi Tim,
> 
>>> The way to determine the answer is:
>>> 1. Create a test dir of all gnulib modules.
>>> 2. Configure it with --config-cache.
>>> 3. Configure it with --config-cache and your sanitizer options.
>>> 4. Compare the generated config.cache and config.status files.
>>
>> I am short on time and would like to prevent being distracted too much
> 
> Everyone here is probably in the same situation...
> 
>> If you could give me a quick instruction on how to do 1.
> 
> I typically use this command:
> 
>   rm -rf ../testdir-all; ./gnulib-tool --create-testdir --dir=../testdir-all 
> --single-configure

This results in:

executing autopoint --force
autopoint: *** The AM_GNU_GETTEXT_VERSION declaration in your configure.ac
   file requires the infrastructure from gettext-0.20 but
this version
   is older. Please upgrade to gettext-0.20 or newer.
autopoint: *** Stop.


As Debian unstable is at gettext 0.19.8.1, I tried to build gettext from
git master. This results in

make[7]: Entering directory
'/home/tim/src/gettext/gettext-tools/examples/tmp-hello-pascal'
make[7]: warning: jobserver unavailable: using -j1.  Add '+' to parent
make rule.
LOCALEDIR='/usr/local/share/locale' /usr/bin/ppcx64 -o./hello ./hello.pas
Free Pascal Compiler version 3.0.4+dfsg-23 [2019/11/25] for x86_64
Copyright (c) 1993-2017 by Florian Klaempfl and others
Target OS: Linux for x86-64
Compiling ./hello.pas
hello.pas(9,6) Fatal: Can't find unit gettext used by hello
Fatal: Compilation aborted
make[7]: *** [Makefile:798: hello.rsj] Error 1


Is it really needed to fail the whole build just because a *PASCAL*
example doesn't compile ?
Wow, I used Pascal in the 1980s the last time, didn't know it is still
used except for fun projects :)

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Easy Accurate Reading and Writing of Floating-Point Numbers

[Tim Rühsen]

On 19.05.20 21:10, Marc Nieper-Wißkirchen wrote:
> Am Di., 19. Mai 2020 um 19:23 Uhr schrieb Tim Rühsen :
> 
>> "copyright" here means more "Nutzungsrecht" which you can transfer also
>> in Germany. You still stay the "Urheber".
> 
> If transferring the "Nutzungsrecht" is enough, everything should be fine.

The first step is to follow ./doc/Copyright/request-assign.future. That is

Please email the following information to ass...@gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
--
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]


[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]


[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]


[For the copyright registration, what country are you a citizen of?]


[What year were you born?]


[Please write your email address here.]


[Please write your postal address here.]





[Which files have you changed so far, and which new files have you written
so far?]




signature.asc
Description: OpenPGP digital signature

Re: Easy Accurate Reading and Writing of Floating-Point Numbers

[Tim Rühsen]

On 19.05.20 18:11, Marc Nieper-Wißkirchen wrote:
> Am Di., 19. Mai 2020 um 17:51 Uhr schrieb Paul Eggert :
>>
>> On 5/19/20 8:35 AM, Marc Nieper-Wißkirchen wrote:
 It is, however, locale-dependent, and there is no "c_dtoastr"
 version as there is a "c_strtod". (Could we get such a wrapper?)
>>> Or a version "c_dtoastr" that uses "c_snprintf" and "c_strtod" internally.
>>
>> Yes, that'd be good to have. Could you write that?
> 
> I should be able to. But please note that I haven't filled out an
> assignment form for copyright transfer yet (which is, by the copyright
> law of Germany, where I live, technically not possible, by the way). I
> hope this is not a major problem.
> 
> Marc

Hi Marc,

"copyright" here means more "Nutzungsrecht" which you can transfer also
in Germany. You still stay the "Urheber".

The german Urheberrecht has been changed a few years ago to be pretty
much in sync with international laws.

I am not an expert in this area and can't give you any advice here :-)

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Fix memleak in getdelim.m4

[Tim Rühsen]

Hi Bruno,

On 18.05.20 13:50, Bruno Haible wrote:
> Hi Tim,
> 
>> With leak sanitizer on, the test for getdelim fails due to a memory leak.
> 
> Is this the only configure test that fails this way?

The only one that fails when running GNU Poke ./configure.

> I mean, it's 3 years since we last looked for such configure test failures. 
> [1]
> 
> The way to determine the answer is:
> 1. Create a test dir of all gnulib modules.
> 2. Configure it with --config-cache.
> 3. Configure it with --config-cache and your sanitizer options.
> 4. Compare the generated config.cache and config.status files.

I am short on time and would like to prevent being distracted too much
from my (random) work at GNU Poke.

If you could give me a quick instruction on how to do 1., I'll happily
go for steps 2-4. I already have a fresh cloned gnulib locally.

Regards, Tim

> 
> Bruno
> 
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2017-05/msg00183.html
> 



signature.asc
Description: OpenPGP digital signature

Fix memleak in getdelim.m4

[Tim Rühsen]

With leak sanitizer on, the test for getdelim fails due to a memory leak.

The attached patch fixes it. (Please feel free to amend.)

Regards, Tim

Output from config.log:

configure:26259: checking for working getdelim function
configure:26325: gcc-10 -o conftest -O1 -g -fno-omit-frame-pointer
-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize
=undefined,bool,alignment,null,enum,address,leak,nonnull-attribute
-fno-sanitize-recover=all -fsanitize-address-use-afte
r-scope   conftest.c  >&5
configure:26325: $? = 0
configure:26325: ./conftest

=
==551573==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 120 byte(s) in 1 object(s) allocated from:
#0 0x7fb818f56e8f in malloc
(/usr/lib/x86_64-linux-gnu/libasan.so.6+0xa9e8f)
#1 0x7fb8183f51bf in _IO_getdelim
/build/glibc-WZtAaN/glibc-2.30/libio/iogetdelim.c:62
#2 0x7fb8198dd72f  ()

SUMMARY: AddressSanitizer: 120 byte(s) leaked in 1 allocation(s).
configure:26325: $? = 1
configure: program exited with status 1
From 0b9451c7c8919121df67b71fdd2f993605c1abc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20R=C3=BChsen?= 
Date: Mon, 18 May 2020 12:36:16 +0200
Subject: [PATCH] Fix memleak in getdelim C code to pacify leak sanitizer

---
 ChangeLog  | 4 
 m4/getdelim.m4 | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 9551d9934..2f7d6d4bb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2020-05-18  Tim Rühsen  
+
+	* m4/getdelim.m4: Fix memleak.
+
 2020-05-17  Bruno Haible  
 
 	Clarify intended usage of the license file modules.
diff --git a/m4/getdelim.m4 b/m4/getdelim.m4
index 9f4c7f6e9..ac3917b11 100644
--- a/m4/getdelim.m4
+++ b/m4/getdelim.m4
@@ -1,4 +1,4 @@
-# getdelim.m4 serial 14
+# getdelim.m4 serial 15
 
 dnl Copyright (C) 2005-2007, 2009-2020 Free Software Foundation, Inc.
 dnl
@@ -42,6 +42,7 @@ AC_DEFUN([gl_FUNC_GETDELIM],
 int len = getdelim (, , '\n', in);
 if (!(len == 4 && line && strcmp (line, "foo\n") == 0))
   { free (line); fclose (in); return 2; }
+free (line);
   }
   {
 /* Test result for a NULL buffer and a non-zero size.
-- 
2.26.2



signature.asc
Description: OpenPGP digital signature

Re: reclaiming memory before exit, take 2

[Tim Rühsen]

Hi,

thanks for your excellent write-up, Bruno.

Just want to add my thoughts :)

When talking about finding memory leaks, like in your wiki page, you
should also mention static analyzers and their ability to find memory
leaks (clang static analyzer, gcc-10 -fanalyzer, Coverity, cppcheck, ...).

These are able to find leaks (and much more) even in code that is seldom
used during runtime (and perhaps never during testing).
To find all those leaks with runtime tools like valgrind, you need  a
very high code coverage in your test suite. Even with a good coverage
you  miss *code paths* during your tests. So you better use coverage
guided fuzzing *and* static analysis in combination.

Another point is: a library exit / cleanup call before exiting the
application can easily free all reachable memory and by that disguise or
hide internal memory leaks that lead to OOM in a long-running
application (daemon or e.g. wget -r).
In these cases it may be helpful to *not* free memory and not call exit
library functions before exiting the application. Only then could
valgrind show the memory hog (maybe the --xtree-memory option is for
exactly that - I didn't investigate it yet).

Hmmm, while writing the above I was interrupted *five* times here... so
please let me know if it sounds a bit weird :).

Regards, Tim

On 16.05.20 16:04, Bruno Haible wrote:
> Hi all,
> 
> Let me try to bring some structure into this discussion.
> 
> 1) The memory-leak tools
> 2) The developer's perspective
> 3) The QA automation perspective
> 
> 
> 1) The memory-leak tools
> 
> 
> Paul and I apparently use the tools differently [1]. So, I've written
> a wiki page
>   https://gitlab.com/ghwiki/gnow-how/-/wikis/Finding_memory_leaks
> that describes what memory leaks are, what tools exist to find them,
> what are advantages and drawbacks. Please read it.
> 
> (If you want to contribute to this wiki, drop me a note, and I'll give
> you write access.)
> 
> In particular, all three tools by default omit memory leaks caused by
> global and static variables.
> 
> Paul writes in [1]:
>> In any event, it seems to me to be a deficiency in the detection if it
>> reports allocated memory which is still referenced to by global
>> variables, or even static variables, as memory leaks.
> 
> On the contrary, reporting leaks through global and static variables is
> a feature. *Hiding* them, which is what the tools do by default, is a
> *misfeature*.
> 
> Why? Because caches that grow without bounds are serious memory leaks
> as well. It is pointless to eliminate serious memory leaks that were
> caused by forgetting free(), while at the same not noticing serious memory
> leaks that were caused by bad design of caches. BOTH have same adverse
> effects.
> 
> This means, when looking for memory leaks, you should use valgrind with
> '--show-reachable=yes'.
> 
> 
> 2) The developer's perspective
> ==
> 
> A developer will want to eliminate all serious memory leaks and not spend
> much time on the not-serious ones. However:
> 
>   The tools cannot distinguish serious from not serious memory leaks.
> 
> Therefore the developer will typically want to silence the not serious
> memory leaks (like they also want to silence "gcc -Wall" warnings when
> they are pointless).
> 
> I can see three good and one bad ways of doing so:
> 
> * [good] Enhance the suppressions file(s).
> 
> * [good] In unit tests, add free() statements right before exit(). Like
>   Paul said in [2]:
> "tests should not contain those types of memory leaks and if someone
>  comes along with a fix, it should be applied."
> 
> * [acceptable] In production binaries, add free() statements guarded by an
>   environment variable test:
> 
> if (getenv ("CLEANUP_BEFORE_EXIT") != NULL)
>   free (data);
> exit (0);
> 
>   Removing the getenv test would, however, violate the GNU standards [3].
> 
> * [bad] Omit the valgrind option '--show-reachable=yes'.
>   This is bad because, as explained above, it will make you blind against
>   some types of serious memory leaks.
> 
> 
> 3) The QA automation perspective
> 
> 
> For QA automation, a multitude of program executions are done with a
> memory leak checker enabled. Since it needs to be automated, the usual
> policy will be that a valgrind or leak sanitizer report is considered
> a failure.
> 
> The QA automation needs to rely on the suppression files created by the
> developers, since it's not a QA engineer's job to evaluate whether a
> particular memory leak is serious or not.
> 
> A good QA automation will thus use 'valgrind --show-reachable=yes'
> and the developer's suppression files.
> 
> 
> Bruno
> 
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00154.html
> [2] https://lists.gnu.org/archive/html/bug-gnulib/2020-05/msg00149.html
> [3] https://www.gnu.org/prep/standards/html_node/Memory-Usage.html
> 
> 



signature.asc
Description: OpenPGP 

Re: reclaiming memory before exit, take 2

[Tim Rühsen]

On 16.05.20 17:33, Jeffrey Walton wrote:
> On Sat, May 16, 2020 at 10:05 AM Bruno Haible  wrote:
>> 3) The QA automation perspective
>> 
>>
>> For QA automation, a multitude of program executions are done with a
>> memory leak checker enabled. Since it needs to be automated, the usual
>> policy will be that a valgrind or leak sanitizer report is considered
>> a failure.
>>
>> The QA automation needs to rely on the suppression files created by the
>> developers, since it's not a QA engineer's job to evaluate whether a
>> particular memory leak is serious or not.
>>
>> A good QA automation will thus use 'valgrind --show-reachable=yes'
>> and the developer's suppression files.
>>
>>
> 
> GNU values choice for the user.
> 
> Perhaps GNU should provide a configuration option
> --enable-memory-leaks for those who want them. The rest of the world
> can enjoy a clean module that can pass through code quality and
> security gates.

Free'ing memory can cost *a lot* of CPU. In the best case this is
completely superfluous, in most cases this leads to a slowdown of
software, to increased energy consumption, to higher costs, and produces
more CO2 than needed.

So by default, applications should not free memory before exit.

But I agree that people who want to test for memory leaks (and most
developers want this for their own projects) should have an easy way to
do this.

At GNU wget we have conditional cleanup functions. That is compilation
with -DDEBUG_MALLOC in $CFLAGS will add those cleanup functions and they
are called before wget exits. Handy for testing, but you have to build
an extra executable.

If the GNU project could agree on a standard regarding such a flag or
even a command line option - I wouldn't be against it.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: portability of fopen and 'e' (O_CLOEXEC) flag

[Tim Rühsen]

Hi Bruno,

On 11.05.20 18:37, Bruno Haible wrote:
> Hi Tim,
> 
>> i would like to ask for your expert knowledge.
>>
>> How to prevent file descriptor leaks in a multi-threaded application
>> that fork+exec. Quick answer is surely "use O_CLOEXEC" to close those
>> file descriptors on exec.
>>
>> But how does this work with fopen in a portable way ?
>> GNU libc has the 'e' flag for exactly this.
> 
> Yes [1].
> 
>> How about other non-GNU OSes / alternative C libraries ?
> 
> POSIX [2][3], macOS [4], FreeBSD [5], Solaris [6] don't support this 'e' flag.
> 
> How about using open() with O_CLOEXEC, and then fdopen()?

Thanks. Sure, this is possible. Doing so at dozens of places (or more)
asks for an implementation in gnulib :)

Several projects (most library code) could benefit. I currently think
about GnuTLS where we have to fix this in one or the other way. Updating
gnulib and adding 'e' to the fopen modes would be straight forward.

> 
> Bruno
> 
> [1] http://man7.org/linux/man-pages/man3/fopen.3.html
> [2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html
> [3] http://man7.org/linux/man-pages/man3/fopen.3p.html
> [4] 
> https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/fopen.3.html
> [5] 
> https://www.freebsd.org/cgi/man.cgi?query=fopen=3=0=freebsd
> [6] https://docs.oracle.com/cd/E36784_01/html/E36874/fopen-3c.html
> 
> 



signature.asc
Description: OpenPGP digital signature

portability of fopen and 'e' (O_CLOEXEC) flag

[Tim Rühsen]

Hi,

i would like to ask for your expert knowledge.

How to prevent file descriptor leaks in a multi-threaded application
that fork+exec. Quick answer is surely "use O_CLOEXEC" to close those
file descriptors on exec.

But how does this work with fopen in a portable way ?
GNU libc has the 'e' flag for exactly this.

How about other non-GNU OSes / alternative C libraries ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Wget bootstrapping problem

[Tim Rühsen]

On 06.05.20 18:37, Bruno Haible wrote:
>> We could also check for an existing wget in bootstrap.conf and set
>> SKIP_PO=1 if not found. While it 'just works' it also disguises the real
>> problem and the user might get something unexpected
>> (non-internationalized wget).
> 
> Alternatively, you could change the build system so that
>   - 'make' recurses into the 'po/' directory only after 'src/',
>   - Fetching the PO files is done through po/Makefile, not bootstrap,
>   - po/Makefile uses src/wget, if not cross-compiling, instead of wget from
> $PATH.

I possibly don't understand - 'po' already appears after 'src' in
SUBDIRS. Since 'po/Makefile' is auto-generated, everything should work
out (if I understood you correctly). So why has Darshit a problem at all ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Wget bootstrapping problem

[Tim Rühsen]

On 05.05.20 03:14, Bruno Haible wrote:
> Paul Eggert wrote:
>>> We could switch the order such that Wget is the default and rsync is used 
>>> as a
>>> fallback
>>
>> That sounds better than reverting, no? Perhaps you could propose a patch.
> 
> No. From the point of security, "wget as default and rsync as fallback" is
> just as bad as "rsync always". Why? [1] Look at the SSLv3 / TLSv1.0 history.
> People believed that "SSLv3 is insecure, but since it's only used as a
> fallback, it doesn't matter". Until someone discovered a way to trick the
> fallback to be activated always [2]...
> 
> rsync is not secure. We should not enable it again.
> 
> Regarding the bootstrapping problem, why not build wget in two steps:
>   1. Bootstrap with no PO files. This produces a non-internationalized wget
>  binary.
>   2. Bootstrap again, using the wget binary from step 1 to fetch the PO files.
> 
> The 'bootstrap' script has an option '--skip-po'. The gnulib-tool script
> should behave the same way if you don't pass the --po-base=... option to it.
> 
> If necessary, we can add another option to gnulib-tool to avoid fetching PO
> files and/or to avoid the use of wget.

I fully agree with Bruno.

We could also check for an existing wget in bootstrap.conf and set
SKIP_PO=1 if not found. While it 'just works' it also disguises the real
problem and the user might get something unexpected
(non-internationalized wget).

Regards, Tim

> 
> Bruno
> 
> [1] https://en.wikipedia.org/wiki/Downgrade_attack
> [2] https://en.wikipedia.org/wiki/POODLE
> 
> 



signature.asc
Description: OpenPGP digital signature

Re: filename.h and dosname.h define identical macros

[Tim Rühsen]

Hi Bruno,

thank you so much for reorganizing the code.

I updated both, Wget and Wget2, to the latest gnulib and it looks good
for me :-)

Regards, Tim

On 3/28/20 2:07 PM, Bruno Haible wrote:

Hi Tim,


dosname.h (included by dirname.h) and filename.h overlap in macro
definition. This currently results in an error here:
...
[-Werror,-Wmacro-redefined]


Please don't say that it's an error, when in fact it's a warning, and
only the build system of your package (or your own way to invoke configure)
turned it into an error.

I process reports about errors with high priority. Reports about warnings
have a lower priority, 1. because you have the option to just ignore the
error, 2. because we cannot avoid warnings on all platforms.


But maybe it's time to consolidate the code, as we have e.g.
_IS_DRIVE_LETTER(C) and HAS_DEVICE(P) doing the same with slightly
different  implementations ?


Yes, you're right. While 'filename' is a better module name than 'dosname'
(because nowadays we care more about Windows than about DOS), some details
in dosname.h are better done than in filename.h. So, let me do it in two steps:
   1. Copy all interesting stuff from dosname.h to filename.h.
   2. Redirect from dosname.h to filename.h.

Part 1:


2020-03-28  Bruno Haible  

filename: Copy some definitions from module 'dosname'.
* lib/filename.h: Include , for IS_FILE_NAME_WITH_DIR.
(HAS_DEVICE): Document macro.
(FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE): New macro.
(IS_ABSOLUTE_FILE_NAME): Consider
FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE.
(IS_RELATIVE_FILE_NAME, IS_FILE_NAME_WITH_DIR): New macros.
(IS_ABSOLUTE_PATH, IS_PATH_WITH_DIR): Define as deprecated aliases.
* lib/relocatable.c (IS_FILE_NAME_WITH_DIR): Renamed from
IS_PATH_WITH_DIR.
(DllMain): Update.
* lib/progreloc.c (IS_FILE_NAME_WITH_DIR): Renamed from
IS_PATH_WITH_DIR.
(find_executable): Update.
* NEWS: Document the deprecations.

diff --git a/NEWS b/NEWS
index 4b9a983..3ec49f3 100644
--- a/NEWS
+++ b/NEWS
@@ -58,6 +58,11 @@ User visible incompatible changes

  DateModules Changes

+2020-03-28  filenameThe macro IS_ABSOLUTE_PATH is deprecated. Use
+IS_ABSOLUTE_FILE_NAME instead.
+The macro IS_PATH_WITH_DIR is deprecated. Use
+IS_FILE_NAME_WITH_DIR instead.
+
  2020-02-22  fchownatThis module no longer defines the functions
  'chownat' and 'lchownat'.  Program that need these
  functions should add the module 'chownat' to the
diff --git a/lib/filename.h b/lib/filename.h
index d4c7020..4598fb1 100644
--- a/lib/filename.h
+++ b/lib/filename.h
@@ -14,38 +14,94 @@
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see .  */

+/* From Paul Eggert and Jim Meyering.  */
+
  #ifndef _FILENAME_H
  #define _FILENAME_H

+#include 
+
  #ifdef __cplusplus
  extern "C" {
  #endif


-/* Pathname support.
-   ISSLASH(C)   tests whether C is a directory separator character.
-   IS_ABSOLUTE_PATH(P)  tests whether P is an absolute path.  If it is not,
-it may be concatenated to a directory pathname.
-   IS_PATH_WITH_DIR(P)  tests whether P contains a directory specification.
+/* Filename support.
+   ISSLASH(C)  tests whether C is a directory separator
+   character.
+   HAS_DEVICE(Filename)tests whether Filename contains a device
+   specification.
+   FILE_SYSTEM_PREFIX_LEN(Filename)  length of the device specification
+ at the beginning of Filename,
+ index of the part consisting of
+ alternating components and slashes.
+   FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE
+   1 when a non-empty device specification
+   can be followed by an empty or relative
+   part,
+   0 when a non-empty device specification
+   must be followed by a slash,
+   0 when device specification don't exist.
+   IS_ABSOLUTE_FILE_NAME(Filename)
+   tests whether Filename is independent of
+   any notion of "current directory".
+   IS_RELATIVE_FILE_NAME(Filename)
+   tests whether Filename may be concatenated
+   to a directory filename.
+   Note: On native Windows, OS/2, DOS, "c:" is neither an absolute nor a
+   relative file name!
+   IS_FILE_NAME_WITH_DIR(Filename)  tests whether Filename contains a device
+ 

Re: continuous integration

[Tim Rühsen]

On 26.03.20 00:46, Bruno Haible wrote:
> Jeffrey Walton wrote:
>> CI tests should be catching these mistakes. (And problems like
>> _NoReturn on OS X).
> 
> Yes, CI can catch some mistakes. Like, just last week, this one: [1].
> 
> Tim and I maintain a continuous integration for gnulib at [2].
> 
> More effort could be put in, in two directions:
> 
> * Like Paul says, instead of only building testdirs, it could build
>   some packages that use gnulib. I would estimate that this would catch
>   3x as many bugs as the current CI with just testdirs.
> 
> * Like you suggest, it would also be useful to test macOS, FreeBSD,
>   Cygwin, and mingw builds.
> 
>> Is there any reasons services like Travis or Cirrus are not being used
>> to proactively detect problems on Linux, OS X and FreeBSD?
> 
> For my part:
> 
> * I have only limited time to work on this; that's why I limit
>   myself to CI integrations for a couple of packages on gitlab.

Same here. I really wish we could had more time to put into CI runners.

I would like to point out that debugging using a CI like Travis is
absolutely tedious and might take a lot of time. Docker-based CI (Linux
only :-| ) are so much easier to debug as you can run the test
environment (images + build scripts) locally.

So while some errors are obvious and easy to fix, others are a nightmare
as you can't 'log in' and just use a debugger. At least I don't have VMs
with OSX or Windows for this purpose.

Did anyone think about using the gcc build platform for automated
testing ? I made up some scripts a while ago for Wget but then lost
focus... if someone likes to take that up.

> * I had not heard of Cirrus CI. Coverage of FreeBSD, additionally to
>   Windows and macOS, sounds interesting. [3]
> 
> * Travis and Cirrus CI are most easily used on Github [4][5]. I don't
>   much like to work on Github, because it tends to become a closed
>   environment. E.g.
> - You can fork someone else's repository only if you stay on Github.
> - Many developers' email addresses are not published, which prevents
>   you from reporting issues by email. You have to use Github "issues"
>   instead.

We just need a mirror / fork on Github that we push to (sync) from time
to time. If someone cares for the initial Travis and/or Cirrus setup
with OSX / FreeBSD / Windows in mind, that would be great !

> But if someone wants to set it up and maintain it, I'm all for it!
> 
> Bruno
> 
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2020-03/msg00041.html
> [2] https://gitlab.com/gnulib/gnulib-ci
> [3] https://cirrus-ci.org/features/#comparison-with-popular-ciaas
> [4] https://en.wikipedia.org/wiki/Travis_CI
> [5] https://cirrus-ci.org/faq/#only-github-support
> 
> 

Regards, Tim



signature.asc
Description: OpenPGP digital signature

rpl_unlink sets errno to EPERM on malloc failure

[Tim Rühsen]

Is this EPERM intended ? If yes, there should be a comment.

lib/unlink.c (rpl_unlink):
  /* Trailing NUL will overwrite the trailing slash.  */
  char *short_name = malloc (len);
  if (!short_name)
{
  errno = EPERM;
  return -1;
}

Regards, Tim



signature.asc
Description: OpenPGP digital signature

filename.h and dosname.h define identical macros

[Tim Rühsen]

Hi,

dosname.h (included by dirname.h) and filename.h overlap in macro
definition. This currently results in an error here:

In file included from wget.c:61:
In file included from ../lib/dirname.h:24:
../lib/dosname.h:36:10: error: 'FILE_SYSTEM_PREFIX_LEN' macro redefined
[-Werror,-Wmacro-redefined]
# define FILE_SYSTEM_PREFIX_LEN(Filename) 0
 ^
../lib/filename.h:46:10: note: previous definition is here
# define FILE_SYSTEM_PREFIX_LEN(P) 0
 ^
1 error generated.
make[2]: *** [Makefile:1737: wget.o] Error 1


We can easily work-around or even ignore this on our side.
But maybe it's time to consolidate the code, as we have e.g.
_IS_DRIVE_LETTER(C) and HAS_DEVICE(P) doing the same with slightly
different  implementations ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

-Wmissing-prototypes triggers in uninorm/decompose-internal.c

[Tim Rühsen]

There seems to be a prototype missing in uninorm/decompose-internal.h:

uninorm/decompose-internal.c:26:27: warning: no previous prototype for
'gl_uninorm_decompose_merge_sort_fromto' [-Wmissing-prototypes]
   26 | #define merge_sort_fromto gl_uninorm_decompose_merge_sort_fromto
  |   ^~
uninorm/decompose-internal.c:26:27: note: in definition of macro
'merge_sort_fromto'
   26 | #define merge_sort_fromto gl_uninorm_decompose_merge_sort_fromto


Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: restrict

[Tim Rühsen]

On 2/17/20 4:53 AM, Bruno Haible wrote:
> Paul Eggert wrote:
>> if GCC generated warnings for that sort of thing, the warnings would be false
>> alarms.
> 
> Yes, and this in turn means that the ability to produce useful warnings via
> 'restrict' is limited. In this example:
> ===
> #include 
> extern void memmcpy (void *restrict, const void *restrict, size_t);
> 
> void shuffle (char array[10])
> {
>   memmcpy (array + 2, array, 8);
>   memcpy (array + 2, array, 8);
> }
> ===
> gcc gives no warning about 'memmcpy' - because it does not know
> how many elements the function will access. gcc does give a warning
> about 'memcpy' - apparently due to custom logic in the compiler.

The following gives you a warning, with -O2 / -O3 and -Wall:

===
#include 
void memmcpy (void *restrict d, void *restrict s, size_t n)
{
  memcpy(d, s, n);
}

void shuffle ()
{
  char array[] = "abcdefg", *array2 = array + 2;

  memmcpy (array, array2 - 2, 8);
}
===

$ gcc-8 -O2 -Wall x.c
In function ‘memmcpy’,
inlined from ‘shuffle’ at x.c:11:3:
x.c:4:3: warning: ‘memcpy’ accessing 8 bytes at offsets 0 and 0 overlaps
8 bytes at offset 0 [-Wrestrict]
   memcpy(d, s, n);
   ^~~

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: restrict

[Tim Rühsen]

On 2/10/20 4:02 AM, Bruno Haible wrote:
> Tim Rühsen wrote:
>> gcc -Wall tells you so (gcc 8 and upwards):
>> $ gcc -Wall msg.c -o msg
>> msg.c: In function ‘main’:
>> msg.c:11:13: warning: passing argument 1 to restrict-qualified parameter
>> aliases with argument 4 [-Wrestrict]
>>11 |   snprintf (msg, sizeof (msg), "%s%s", msg, WRONG_MESSAGE);
>>   | ^~~~~~
>> msg.c:11:35: warning: ‘%s’ directive output may be truncated writing 4
>> bytes into a region of size between 1 and 11 [-Wformat-truncation=]
>>11 |   snprintf (msg, sizeof (msg), "%s%s", msg, WRONG_MESSAGE);
>>   |   ^~
>> msg.c:11:3: note: ‘snprintf’ output between 5 and 15 bytes into a
>> destination of size 11
>>11 |   snprintf (msg, sizeof (msg), "%s%s", msg, WRONG_MESSAGE);
>>   |   ^~~~
>>
> 
> Oh, the 'restrict' keyword is actually useful (other than for micro-
> optimizations)! Thanks for this pointer, Tim.
> 
> So, it would make sense to use this keyword in function declarations in
> public .h files in gnulib.
> 
> For internal .h files in gnulib, there's no point - this code is assumed
> correct, no need to try to get GCC to produce warnings here.
> 
> What does 'restrict' precisely mean? ISO C 99 § 6.7.3.(7):
>   "An object that is accessed through a restrict-qualified pointer has a
>special association with that pointer. This association, defined in
>6.7.3.1 below, requires that all accesses to that object use, directly
>or indirectly, the value of that particular pointer.115) The intended
>use of the restrict qualifier (like the register storage class) is to
>promote optimization, and deleting all instances of the qualifier from
>all preprocessing translation units composing a conforming program
>does not change its meaning (i.e., observable behavior)."
> 
> Which function declarations could therefore profit from the 'restrict'
> keyword?
> 
>   - Only parameters of pointer types, excluding function pointer types,
> matter.
> 
>   - If all pointer parameters are 'const SOMETHING *', the function
> is not writing into them, therefore 'restrict' is redundant.
> Example: int strcmp (const char *, const char *);
> 
>   - If all pointer parameters point to different types, excluding 'void *',
> ISO C forbids aliasing anyway, therefore 'restrict' is redundant.
> Example: void dfacomp (char const *, ptrdiff_t, struct dfa *, bool);
> and https://pubs.opengroup.org/onlinepubs/9699919799/functions/glob.html
> 
>   - If, among the parameters, there are N parameters of type
>   [const] TYPE *
> adding the 'restrict' keyword to N-1 of them is equivalent to adding
> the 'restrict' keyword to all N of them.
> See ISO C 99 § 6.7.3.1.(7) [Example 1].
> 
> GCC's warning facility understands this, regarding the non-const pointer.
> Test case:
> 
> #include 
> extern int smprintf (char *restrict, size_t, const char *, ...)
>  __attribute__ ((__format__ (__printf__, 3, 4)));
> int main ()
> {
>   char msg[sizeof ("try a fool")] = "try a ";
>   smprintf (msg, sizeof (msg), "%s%s", msg, "fool");
> }
> 
> 
>   - If a function has an input parameter of type
>   const TYPE *
> and an output parameter of type
>   TYPE *
> then the 'restrict' keyword means that the implementation may write
> the output before having finished reading all the input.
> Example:
>   void arctwo_encrypt (arctwo_context *context, const char *inbuf,
>char * restrict outbuf, size_t length);

Doesn't this need 'restrict' for the 'inbuf' parameter as well ?
If not, why is glibc applying restrict to both parameters of strcpy ?

> Whereas the absence of the 'restrict' keyword means that the
> implementation will read all the input _before_ starting to store
> the output.

Can we say
"Whereas the absence of the 'restrict' keyword means that the
implementation guarantees documented operation for any aliasing of input
and output"
?

> Example:
>   int hmac_md5 (const void *key, size_t keylen,
> const void *buffer, size_t buflen, void *resbuf);
> 
>   - 'restrict' should NOT be used for multiple output parameters of the
> same type, when only a single word is written through each parameter.
&

Re: Possible testing case of snprintf.

[Tim Rühsen]

On 09.02.20 15:44, Mats Erik Andersson wrote:
> Hello there!
> 
> This note has its origin in a report received at bug-inetutils.
> The following test code for snprintf() is a simplyfied detection
> I have implemented as a warning-only test in Gnu Inetutils.
> My point is that Linux/glibc and kfreebsd/glibc triggers this
> warning, but OpenSolaris, OpenIndiana, FreeBSD, OpenBSD, NetBSD,
> and DragonflyBSD do not! Reading the replacement code for the
> Gnulib module snprintf, neither would your function, should it
> undergo the test. In conclusion, this is a case where the native
> glibc function snprintf() behaves worse than does your replacement.
> 
>   #define MESSAGE   "try a fool"
>   #define WRONG_MESSAGE "fool"
> 
>   char msg[sizeof (MESSAGE)] = "try a ";
> 
>   snprintf (msg, sizeof (msg), "%s%s", msg, WRONG_MESSAGE);
> 
>   if (!strcmp (msg, WRONG_MESSAGE))
> printf ("Warning! snprintf got confused!\n");
> 
> Observe that `msg' is target, as well as source. POSIX mentions
> nothing about such a use case, but glibc will produce "fool",
> whereas all BSD unices as well as OpenSolaris descendants will
> produce "try a fool". Tacitly, POSIX would probably cry out
> a statement like "Undefined"!

s(n)printf declaration uses the restrict keyword. That basically means
that each of the pointers in the arguments points to the same block of
memory.

gcc -Wall tells you so (gcc 8 and upwards):
$ gcc -Wall msg.c -o msg
msg.c: In function ‘main’:
msg.c:11:13: warning: passing argument 1 to restrict-qualified parameter
aliases with argument 4 [-Wrestrict]
   11 |   snprintf (msg, sizeof (msg), "%s%s", msg, WRONG_MESSAGE);
  | ^~~~~~
msg.c:11:35: warning: ‘%s’ directive output may be truncated writing 4
bytes into a region of size between 1 and 11 [-Wformat-truncation=]
   11 |   snprintf (msg, sizeof (msg), "%s%s", msg, WRONG_MESSAGE);
  |   ^~
msg.c:11:3: note: ‘snprintf’ output between 5 and 15 bytes into a
destination of size 11
   11 |   snprintf (msg, sizeof (msg), "%s%s", msg, WRONG_MESSAGE);
  |   ^~~~

Except in rare cases, compiler warnings indicate that the programmer is
wrong. Turn them all (well, almost all) on !

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: clang-10 warning in hash.c

[Tim Rühsen]

Hi Bruno,

On 1/27/20 2:09 PM, Bruno Haible wrote:
>> Not sure if the compiler is correct here, but maybe worth a look:
>>
>> hash.c:549:11: error: implicit conversion from 'unsigned long' to
>> 'float' changes value from 18446744073709551615 to 18446744073709551616
>> [-Werror,-Wimplicit-int-float-conversion]
>>   if (SIZE_MAX <= new_candidate)
>>   ^~~~ ~~
>> /usr/include/stdint.h:227:22: note: expanded from macro 'SIZE_MAX'
>> #  define SIZE_MAX  (18446744073709551615UL)
>>  ^~
> 
> This warning is pointless, because
>   - Since the next float below 18446744073709551616 = 0x1
> would be   18446742974197923840 =  0xFF00
> the comparison result is the same for the two values ...615 and ...616.
>   - The compiler inserts the implicit conversion only because of the '<='
> operator.
> IMO you should file a ticket with the clang people.
> 
> Inserting a cast to 'double'
> 
>   if ((double) SIZE_MAX <= (double) new_candidate)
> 
> would not help, because
> the next double-float below 18446744073709551616 = 0x1
> would be18446744073709549568 =  0xF800

Thanks for the understandable explanation !

Sadly, bugs.llvm.org disabled self-registration. So they won't get a bug
report from me (looks like they want to encapsulate from the rest of the
world). Instead -Wno-implicit-int-float-conversion will be added to the
clang options.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: FreeBSD 11.2: glthread build failure

[Tim Rühsen]

On 1/20/20 7:04 PM, Bruno Haible wrote:
>   CCLD wget2
>>>
>>> Please make it a habit to use "make V=1" when submitting a report.
>>> I don't want to see "CCLD wget2"; I want to see the actual link
>>> command line.
>>
>> I don't have direct console/shell access - the issue is/was on a FreeBSD
>> instance running somewhere that we use for testing via Gitlab CI.
> 
> You could, instead of running
>make
> run
>make || { echo 'Retrying with verbosity'; make V=1; exit 1; }
> This way, the overall log is compact, but it is verbose at the place
> where we need it.

Simple and brilliant !

Thanks, Tim



signature.asc
Description: OpenPGP digital signature

clang-10 warning in hash.c

[Tim Rühsen]

Not sure if the compiler is correct here, but maybe worth a look:

hash.c:549:11: error: implicit conversion from 'unsigned long' to
'float' changes value from 18446744073709551615 to 18446744073709551616
[-Werror,-Wimplicit-int-float-conversion]
  if (SIZE_MAX <= new_candidate)
  ^~~~ ~~
/usr/include/stdint.h:227:22: note: expanded from macro 'SIZE_MAX'
#  define SIZE_MAX  (18446744073709551615UL)
 ^~
hash.c:1079:15: error: implicit conversion from 'unsigned long' to
'float' changes value from 18446744073709551615 to 18446744073709551616
[-Werror,-Wimplicit-int-float-conversion]
  if (SIZE_MAX <= candidate)
  ^~~~ ~~
/usr/include/stdint.h:227:22: note: expanded from macro 'SIZE_MAX'
#  define SIZE_MAX  (18446744073709551615UL)
 ^~
2 errors generated.


Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: FreeBSD 11.2: glthread build failure

[Tim Rühsen]

Hi Bruno,

thanks for your work !

I can confirm that your patch fixes the issue here.

On 1/20/20 4:01 AM, Bruno Haible wrote:
>>> FreeBSD 11.2-RELEASE-p4 (with clang 6.0):
>>>
>>>   CCLD wget2
> 
> Please make it a habit to use "make V=1" when submitting a report.
> I don't want to see "CCLD wget2"; I want to see the actual link
> command line.

I don't have direct console/shell access - the issue is/was on a FreeBSD
instance running somewhere that we use for testing via Gitlab CI.
Despite from just having the output from that CI runner, it wasn't
obvious in that moment that 'make V=1' would be helpful.

Please let me know if something is missing in a report before you waste
time and i try to get that piece of info somehow.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: heap-use-after-free in rpl_glob

[Tim Rühsen]

Hi Bruno,

I can confirm that your patch doesn't trigger asan any more.

Thank you !

Regards, Tim

On 17.01.20 18:00, Bruno Haible wrote:
> Hi Tim,
> 
>> The continuous fuzzer at OSS-Fuzz today reported an issue in rpl_glob.
>>
>> To reproduce with attached C code (on Debian unstable here, same result
>> on Ubuntu 16.04.6 docker container with clang 10):
>>
>> export CC=gcc
>> export CFLAGS="-O1 -g -fno-omit-frame-pointer -fsanitize=address
>> -fsanitize-address-use-after-scope"
>> # ... build gnulib ...
>> $CC $CFLAGS -I. -Ilib glob_crash2.c -o glob_crash2 lib/.libs/libgnu.a
>> ./glob_crash2
>>
>> =
>> ==1671628==ERROR: AddressSanitizer: heap-use-after-free on address
>> 0x60400013 at pc 0x55fa90a36ecd bp 0x7ffe68412980 sp 0x7ffe68412978
>> READ of size 44 at 0x60400013 thread T0
>> #0 0x55fa90a36ecc in rpl_glob /home/tim/src/wget2/lib/glob.c:868
>> #1 0x55fa90a334eb in main /home/tim/src/wget2/glob_crash2.c:35
>> #2 0x7fdafafabbba in __libc_start_main ../csu/libc-start.c:308
>> #3 0x55fa90a332f9 in _start (/home/tim/src/wget2/glob_crash2+0x22f9)
>>
>> 0x60400013 is located 3 bytes inside of 48-byte region
>> [0x60400010,0x60400040)
>> freed by thread T0 here:
>> #0 0x7fdafb24c277 in __interceptor_free
>> (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107277)
>> #1 0x55fa90a36e31 in rpl_glob /home/tim/src/wget2/lib/glob.c:849
>> #2 0x55fa90a334eb in main /home/tim/src/wget2/glob_crash2.c:35
>> #3 0x7fdafafabbba in __libc_start_main ../csu/libc-start.c:308
>>
>> previously allocated by thread T0 here:
>> #0 0x7fdafb24c628 in malloc
>> (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
>> #1 0x55fa90a35311 in rpl_glob /home/tim/src/wget2/lib/glob.c:565
>> #2 0x55fa90a334eb in main /home/tim/src/wget2/glob_crash2.c:35
>> #3 0x7fdafafabbba in __libc_start_main ../csu/libc-start.c:308
> 
> I can't reproduce the crashes. But the line numbers (565, 849, 868)
> from the output above are clearly indicating the problem:
>   - end_name is part of dirname,
>   - dirname is freed,
>   - after dirname is freed, the code still accesses end_name.
> 
> Can you please test this patch?
> 
> Thank you very much for this report! I expect that the fix will also need
> to go into glibc.
> 
> 
> 2020-01-17  Bruno Haible  
> 
>   glob: Fix use-after-free bug.
>   Reported by Tim Rühsen  in
>   <https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00102.html>.
>   * lib/glob.c (__glob): Delay freeing dirname until after the use of
>   end_name.
> 
> diff --git a/lib/glob.c b/lib/glob.c
> index a67cbb6..5b34939 100644
> --- a/lib/glob.c
> +++ b/lib/glob.c
> @@ -843,10 +843,11 @@ __glob (const char *pattern, int flags, int (*errfunc) 
> (const char *, int),
>{
>  size_t home_len = strlen (p->pw_dir);
>  size_t rest_len = end_name == NULL ? 0 : strlen (end_name);
> +/* dirname contains end_name; we can't free it now.  */
> +char *prev_dirname =
> +  (__glibc_unlikely (malloc_dirname) ? dirname : NULL);
>  char *d;
>  
> -if (__glibc_unlikely (malloc_dirname))
> -  free (dirname);
>  malloc_dirname = 0;
>  
>  if (glob_use_alloca (alloca_used, home_len + rest_len + 1))
> @@ -868,6 +869,8 @@ __glob (const char *pattern, int flags, int (*errfunc) 
> (const char *, int),
>d = mempcpy (d, end_name, rest_len);
>  *d = '\0';
>  
> +free (prev_dirname);
> +
>  dirlen = home_len + rest_len;
>  dirname_modified = 1;
>}
> 



signature.asc
Description: OpenPGP digital signature

heap-use-after-free in rpl_glob

[Tim Rühsen]

Hi,

I recently updated wget2 to gnulib commit
a7903da07d3d18c23314aa0815adbb4058fd7cec.

The continuous fuzzer at OSS-Fuzz today reported an issue in rpl_glob.

To reproduce with attached C code (on Debian unstable here, same result
on Ubuntu 16.04.6 docker container with clang 10):

export CC=gcc
export CFLAGS="-O1 -g -fno-omit-frame-pointer -fsanitize=address
-fsanitize-address-use-after-scope"
# ... build gnulib ...
$CC $CFLAGS -I. -Ilib glob_crash2.c -o glob_crash2 lib/.libs/libgnu.a
./glob_crash2

=
==1671628==ERROR: AddressSanitizer: heap-use-after-free on address
0x60400013 at pc 0x55fa90a36ecd bp 0x7ffe68412980 sp 0x7ffe68412978
READ of size 44 at 0x60400013 thread T0
#0 0x55fa90a36ecc in rpl_glob /home/tim/src/wget2/lib/glob.c:868
#1 0x55fa90a334eb in main /home/tim/src/wget2/glob_crash2.c:35
#2 0x7fdafafabbba in __libc_start_main ../csu/libc-start.c:308
#3 0x55fa90a332f9 in _start (/home/tim/src/wget2/glob_crash2+0x22f9)

0x60400013 is located 3 bytes inside of 48-byte region
[0x60400010,0x60400040)
freed by thread T0 here:
#0 0x7fdafb24c277 in __interceptor_free
(/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107277)
#1 0x55fa90a36e31 in rpl_glob /home/tim/src/wget2/lib/glob.c:849
#2 0x55fa90a334eb in main /home/tim/src/wget2/glob_crash2.c:35
#3 0x7fdafafabbba in __libc_start_main ../csu/libc-start.c:308

previously allocated by thread T0 here:
#0 0x7fdafb24c628 in malloc
(/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
#1 0x55fa90a35311 in rpl_glob /home/tim/src/wget2/lib/glob.c:565
#2 0x55fa90a334eb in main /home/tim/src/wget2/glob_crash2.c:35
#3 0x7fdafafabbba in __libc_start_main ../csu/libc-start.c:308

SUMMARY: AddressSanitizer: heap-use-after-free
/home/tim/src/wget2/lib/glob.c:868 in rpl_glob
Shadow bytes around the buggy address:
  0x0c087fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c087fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c087fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c087fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c087fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c087fff8000: fa fa[fd]fd fd fd fd fd fa fa 00 00 00 00 00 01
  0x0c087fff8010: fa fa 00 00 00 00 00 01 fa fa 00 00 00 00 06 fa
  0x0c087fff8020: fa fa 00 00 00 00 06 fa fa fa 00 00 00 00 02 fa
  0x0c087fff8030: fa fa 00 00 00 00 02 fa fa fa 00 00 00 00 00 fa
  0x0c087fff8040: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 00 fa
  0x0c087fff8050: fa fa 00 00 00 00 00 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
  Shadow gap:  cc
==1671628==ABORTING


Maybe someone who knows glob better than me could have a look. It seems
to be a regression.

Regards, Tim
/*
 * Created 17.01.2019 by Tim Rühsen
 *
 * Call glob() using data from fuzzer crash file
 *
 * Build and execute with instrumented gnulib (amend -I paths as needed):
 *
 * clang build (spills out WRITE heap buffer overflow)
 * export CC=clang-6.0
 * export CFLAGS="-O1 -g -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope"
 * $CC $CFLAGS -I. -Ilib glob_crash2.c -o glob_crash2 lib/.libs/libgnu.a
 * ./glob_crash2
 *
 * gcc build (spills out READ heap buffer overflow):
 * export CC=gcc
 * export CFLAGS="-O1 -g -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope"
 * $CC $CFLAGS -I. -Ilib glob_crash2.c -o glob_crash2 lib/.libs/libgnu.a
 * ./glob_crash2
 */

#include 
#include 

int main(int argc, char **argv)
{
static const unsigned char data[] = {
  0x7e,0x6c,0x70,0x2f,0x83,0x6d,0x65,0x1d,0x75,0xef,0xcc,0xf0,0x74,0x1b,0x03,0x02,0x43,
  0x94,0x05,0x33,0x83,0x1a,0xd4,0x4c,0x9f,0xbb,0x62,0xe6,0xb5,0x99,0x75,0x9f,0x26,0x69,
  0xc0,0x49,0xb0,0x4b,0x38,0xe8,0x74,0x0c,0xc2,0xd1,0x81,0x46,0x77,0x2f,0x89,0xf1,0xc8,
  0x73,0xb3,0x8f,0xf7,0x60,0x63,0xba,0xa5,0x59,0xaa,0xd1,0xa8,0xfc,0xf8,0x20,0xd8,0x12,
  0x58,0x61,0x12,0xc6,0x21,0x5b,0xf5,0x93,0x5a,0x7c,0x79,0x34,0xa5,0x01, 0x00
};

	glob_t pglob = { .gl_pathc = 0 };
	if (glob((const char *) data, GLOB_MARK | GLOB_TILDE, NULL, ) == 0)
		globfree();

	return 0;
}


signature.asc
Description: OpenPGP digital signature

Re: Unicode support in poke

[Tim Rühsen]

Hi Bruno,

On 1/13/20 12:01 PM, Bruno Haible wrote:
> Hi Tim,
> 
>> you could look at libidn2 as an example how to use system libunistring
>> if there (or if new enough) and fallback to gnulib unistring.
>> (BTW, libunistring is made of the gnulib unistring modules)
>>
>> It creates a separate dir / library for gnulib unistring functions,
>> *BUT* only uses it when a system libunistring can't be found.
>>
>> bootstrap.conf: Call gnulib-tool in bootstrap_post_import_hook() only
>> for the needed unistring modules.
>>
>> configure.ac: Check for system libunistring (set a conditional
>> HAVE_LIBUNISTRING
>>
>> Makefile.am: if !HAVE_LIBUNISTRING -> add unistring/ to SUBDIR
>>
>> */Makefile.am: if !HAVE_LIBUNISTRING -> add unistring/ to includes in
>> AM_CPPFLAGS
> 
> A simpler way to achieve the same thing is to include the gnulib module
> 'libunistring-optional'. It will use the system libunistring if it
> exists and is new enough, and otherwise compile the respective modules
> from source.

Ah, I didn't know that, thanks.

Don't that pull in all the unistring modules (and code) ? At least for
building the project.

In libidn2 we just use a small subset of the vast functionality of
gnulib unistring. In order to keep build overhead small, isn't that the
way to go ? I only know that building libunistring takes a while...

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Unicode support in poke

[Tim Rühsen]

Hi José,

you could look at libidn2 as an example how to use system libunistring
if there (or if new enough) and fallback to gnulib unistring.
(BTW, libunistring is made of the gnulib unistring modules)

It creates a separate dir / library for gnulib unistring functions,
*BUT* only uses it when a system libunistring can't be found.

bootstrap.conf: Call gnulib-tool in bootstrap_post_import_hook() only
for the needed unistring modules.

configure.ac: Check for system libunistring (set a conditional
HAVE_LIBUNISTRING

Makefile.am: if !HAVE_LIBUNISTRING -> add unistring/ to SUBDIR

*/Makefile.am: if !HAVE_LIBUNISTRING -> add unistring/ to includes in
AM_CPPFLAGS

I think, that's all.

Regards, Tim

On 1/13/20 11:33 AM, Bruno Haible wrote:
> Hi José,
> 
> Yesterday, you identified a set of functions from GNU libunistring that would
> be useful to use in GNU poke. Since you will need only a few such functions,
> which sums up to little code and only one big table, you can take the
> respective modules from gnulib - a regular use of gnulib-tool. All the source
> code of libunistring is in gnulib, distributed across ca. 350 modules.
> 
> The list of modules is as follows. The relation between function name and
> module name is obvious.
> 
> unitypes
> unistr/base
> unistr/u8-check
> unistr/u8-chr
> unistr/u8-cmp
> unistr/u8-cmp2
> unistr/u8-cpy
> unistr/u8-cpy-alloc
> unistr/u8-endswith
> unistr/u8-mblen
> unistr/u8-mbsnlen
> unistr/u8-mbtouc
> unistr/u8-mbtoucr
> unistr/u8-mbtouc-unsafe
> unistr/u8-move
> unistr/u8-next
> unistr/u8-prev
> unistr/u8-set
> unistr/u8-startswith
> unistr/u8-stpcpy
> unistr/u8-stpncpy
> unistr/u8-strcat
> unistr/u8-strchr
> unistr/u8-strcmp
> unistr/u8-strcoll
> unistr/u8-strcpy
> unistr/u8-strcspn
> unistr/u8-strdup
> unistr/u8-strlen
> unistr/u8-strmblen
> unistr/u8-strmbtouc
> unistr/u8-strncat
> unistr/u8-strncmp
> unistr/u8-strncpy
> unistr/u8-strnlen
> unistr/u8-strpbrk
> unistr/u8-strrchr
> unistr/u8-strspn
> unistr/u8-strstr
> unistr/u8-strtok
> unistr/u8-to-u16
> unistr/u8-to-u32
> unistr/u8-uctomb
> unistr/u16-check
> unistr/u16-chr
> unistr/u16-cmp
> unistr/u16-cmp2
> unistr/u16-cpy
> unistr/u16-cpy-alloc
> unistr/u16-endswith
> unistr/u16-mblen
> unistr/u16-mbsnlen
> unistr/u16-mbtouc
> unistr/u16-mbtoucr
> unistr/u16-mbtouc-unsafe
> unistr/u16-move
> unistr/u16-next
> unistr/u16-prev
> unistr/u16-set
> unistr/u16-startswith
> unistr/u16-stpcpy
> unistr/u16-stpncpy
> unistr/u16-strcat
> unistr/u16-strchr
> unistr/u16-strcmp
> unistr/u16-strcoll
> unistr/u16-strcpy
> unistr/u16-strcspn
> unistr/u16-strdup
> unistr/u16-strlen
> unistr/u16-strmblen
> unistr/u16-strmbtouc
> unistr/u16-strncat
> unistr/u16-strncmp
> unistr/u16-strncpy
> unistr/u16-strnlen
> unistr/u16-strpbrk
> unistr/u16-strrchr
> unistr/u16-strspn
> unistr/u16-strstr
> unistr/u16-strtok
> unistr/u16-to-u32
> unistr/u16-to-u8
> unistr/u16-uctomb
> unistr/u32-check
> unistr/u32-chr
> unistr/u32-cmp
> unistr/u32-cmp2
> unistr/u32-cpy
> unistr/u32-cpy-alloc
> unistr/u32-endswith
> unistr/u32-mblen
> unistr/u32-mbsnlen
> unistr/u32-mbtouc
> unistr/u32-mbtoucr
> unistr/u32-mbtouc-unsafe
> unistr/u32-move
> unistr/u32-next
> unistr/u32-prev
> unistr/u32-set
> unistr/u32-startswith
> unistr/u32-stpcpy
> unistr/u32-stpncpy
> unistr/u32-strcat
> unistr/u32-strchr
> unistr/u32-strcmp
> unistr/u32-strcoll
> unistr/u32-strcpy
> unistr/u32-strcspn
> unistr/u32-strdup
> unistr/u32-strlen
> unistr/u32-strmblen
> unistr/u32-strmbtouc
> unistr/u32-strncat
> unistr/u32-strncmp
> unistr/u32-strncpy
> unistr/u32-strnlen
> unistr/u32-strpbrk
> unistr/u32-strrchr
> unistr/u32-strspn
> unistr/u32-strstr
> unistr/u32-strtok
> unistr/u32-to-u16
> unistr/u32-to-u8
> unistr/u32-uctomb
> uniconv/base
> uniconv/u8-conv-from-enc
> uniconv/u8-conv-to-enc
> uniconv/u8-strconv-from-enc
> uniconv/u8-strconv-from-locale
> uniconv/u8-strconv-to-enc
> uniconv/u8-strconv-to-locale
> uniconv/u16-conv-from-enc
> uniconv/u16-conv-to-enc
> uniconv/u16-strconv-from-enc
> uniconv/u16-strconv-from-locale
> uniconv/u16-strconv-to-enc
> uniconv/u16-strconv-to-locale
> uniconv/u32-conv-from-enc
> uniconv/u32-conv-to-enc
> uniconv/u32-strconv-from-enc
> uniconv/u32-strconv-from-locale
> uniconv/u32-strconv-to-enc
> uniconv/u32-strconv-to-locale
> unistdio/base
> unistdio/u8-asnprintf
> unistdio/u8-asprintf
> unistdio/u8-snprintf
> unistdio/u8-sprintf
> 

Re: FreeBSD 11.2: glthread build failure

[Tim Rühsen]

FYI, the issue below also happens with --disable-threads.

Using --enable-threads=isoc fixes it (found in an earlier post on this ML).

Regards, Tim

On 1/7/20 4:03 PM, Tim Rühsen wrote:
> Hi,
> 
> with the latest gnulib (a7903da07d3d18c23314aa0815adbb4058fd7cec) on
> FreeBSD 11.2-RELEASE-p4 (with clang 6.0):
> 
>   CCLD wget2
> /usr/bin/ld: undefined reference to symbol
> `pthread_mutexattr_gettype@@FBSD_1.0' (try adding -lthr)
> //lib/libthr.so.3: could not read symbols: Bad value
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
> gmake[2]: *** [Makefile:1697: wget2] Error 1
> 
> I have no direct access to that machine or to any other FreeBSD
> installation.
> 
> LIBMULTITHREAD=''
> LIBPMULTITHREAD='-lpthread'
> LIBPTHREAD=''
> LIBSTDTHREAD=''
> LIBTHREAD=''
> LIB_PTHREAD_SIGMASK=''
> 
> gnulib-tool tells me to add $(LIBMULTITHREAD), $(LIBTHREAD) and
> $(LIB_PTHREAD_SIGMASK) to LDADD.
> 
> config.log is at
> https://gitlab.com/gnuwget/wget2/-/jobs/395869770/artifacts/file/config.log
> 



signature.asc
Description: OpenPGP digital signature

Re: Debian Stretch: strndup.c build failure

[Tim Rühsen]

Hi Bruno,

On 07.01.20 19:00, Bruno Haible wrote:
>> with the latest gnulib (a7903da07d3d18c23314aa0815adbb4058fd7cec) on
>> Debian Stretch:
>>
>> In file included from /usr/include/string.h:630:0,
>>  from ./string.h:41,
>>  from strndup.c:21:
>> strndup.c:26:1: error: expected identifier or '(' before '__extension__'
>>  strndup (char const *s, size_t n)
>>  ^
>>
>> config.log is at
>> https://gitlab.com/gnuwget/wget2/-/jobs/395706872/artifacts/file/config.log.
> 
> An invocation of AC_CHECK_FUNC is malfunctioning because of -Werror:
> 
> For this reason, -Werror is not supported at configuration-time in gnulib [1].
> 
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2019-07/msg00081.html

Thanks for clarification :-)

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Debian Stretch: strndup.c build failure

[Tim Rühsen]

On 1/7/20 3:24 PM, Tim Rühsen wrote:
> Hi,
> 
> with the latest gnulib (a7903da07d3d18c23314aa0815adbb4058fd7cec) on
> Debian Stretch:
> 
> In file included from /usr/include/string.h:630:0,
>  from ./string.h:41,
>  from strndup.c:21:
> strndup.c:26:1: error: expected identifier or '(' before '__extension__'
>  strndup (char const *s, size_t n)
>  ^
> 
> config.log is at
> https://gitlab.com/gnuwget/wget2/-/jobs/395706872/artifacts/file/config.log.

The issue is that the ./configure step is run with CFLAGS="-Werror"
(here with CC=clang):

$ grep strndup config.log
configure:8814: checking for strndup
conftest.c:152:6: error: incompatible redeclaration of library function
'strndup' [-Werror,-Wincompatible-library-redeclaration]
char strndup ();
conftest.c:152:6: note: 'strndup' is a builtin with type 'char *(const
char *, unsigned long)'
| /* Define strndup to an innocuous variant, in case  declares
strndup.
| #define strndup innocuous_strndup
| which can conflict with char strndup (); below.
| #undef strndup
| char strndup ();
| #if defined __stub_strndup || defined __stub___strndup
| return strndup ();
configure:20326: checking whether strndup is declared
ac_cv_func_strndup=no
ac_cv_have_decl_strndup=yes


Though it always worked (for my projects), I can imagine that
./configure tests may stumble when -Werror is set. So, is it good
practice not to use -Werror with ./configure ?


Without -Werror:
$ grep strndup config.log
configure:8814: checking for strndup
conftest.c:154:6: warning: conflicting types for built-in function 'strndup'
 char strndup ();
configure:20326: checking whether strndup is declared
configure:38601: checking for working strndup
ac_cv_func_strndup=yes
ac_cv_have_decl_strndup=yes
gl_cv_func_strndup_works=yes


Regards, Tim



signature.asc
Description: OpenPGP digital signature

FreeBSD 11.2: glthread build failure

[Tim Rühsen]

Hi,

with the latest gnulib (a7903da07d3d18c23314aa0815adbb4058fd7cec) on
FreeBSD 11.2-RELEASE-p4 (with clang 6.0):

  CCLD wget2
/usr/bin/ld: undefined reference to symbol
`pthread_mutexattr_gettype@@FBSD_1.0' (try adding -lthr)
//lib/libthr.so.3: could not read symbols: Bad value
clang: error: linker command failed with exit code 1 (use -v to see
invocation)
gmake[2]: *** [Makefile:1697: wget2] Error 1

I have no direct access to that machine or to any other FreeBSD
installation.

LIBMULTITHREAD=''
LIBPMULTITHREAD='-lpthread'
LIBPTHREAD=''
LIBSTDTHREAD=''
LIBTHREAD=''
LIB_PTHREAD_SIGMASK=''

gnulib-tool tells me to add $(LIBMULTITHREAD), $(LIBTHREAD) and
$(LIB_PTHREAD_SIGMASK) to LDADD.

config.log is at
https://gitlab.com/gnuwget/wget2/-/jobs/395869770/artifacts/file/config.log



signature.asc
Description: OpenPGP digital signature

Debian Stretch: strndup.c build failure

[Tim Rühsen]

Hi,

with the latest gnulib (a7903da07d3d18c23314aa0815adbb4058fd7cec) on
Debian Stretch:

In file included from /usr/include/string.h:630:0,
 from ./string.h:41,
 from strndup.c:21:
strndup.c:26:1: error: expected identifier or '(' before '__extension__'
 strndup (char const *s, size_t n)
 ^

config.log is at
https://gitlab.com/gnuwget/wget2/-/jobs/395706872/artifacts/file/config.log.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: string types

[Tim Rühsen]

On 1/6/20 5:08 PM, Tim Rühsen wrote:
>>   * What about reusing the complete vasnprintf.c for (B), rather than
>> adding another, limited printf-like implementation?
> 
> Yes, would be nice. At least for appending we need an extra
> malloc/malloc/memcpy/free.
> 
> vasnprintf reallocates RESULTBUF that means we can't use a stack
> allocated buffer - thus we lose the performance advantage. Or should we
> try snprintf first and fallback to vasnprintf in case of truncation ?
> 
> We want another module e.g. buffer-printf to not pull in vasnprintf when
> not needing printf-like buffer functions.
> 
> Once the buffer module is done, we could think of amending vasnprintf to
> better play with the buffer type.

Just made a speed comparison between vasnprintf and wget_buffer_printf,
10m times executed, within a stack-only szenario (no reallocations), gcc
9.2.1, -O1 -g.

asnprintf(sbuf, , "%d", i);
takes 0m2.727s

wget_buffer_printf(, "%d", i);
takes 0m0.226s

char s[]="123";

asnprintf(sbuf, , "%s", s);
takes 0m2.282s

wget_buffer_printf(, "%s", s);
takes 0m0.212s

It tells me that vasnprintf has a huge startup overhead. Perhaps we can
tweak that a little bit.


the vasnprintf program that I run for %d:

#include 

void main(void)
{
  char sbuf[256];
  size_t size = sizeof(sbuf);

  for (int i=0;i<100;i++) {
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
asnprintf(sbuf, , "%d", i);
  }
}



signature.asc
Description: OpenPGP digital signature

Re: string types

[Tim Rühsen]

Hi Bruno,

On 1/6/20 1:46 PM, Bruno Haible wrote:
>   - providing primitives for string allocation reduces the amount of 
> buffer
> overflow bugs that otherwise occur in this area. [1]
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2019-09/msg00031.html
>>> ...
>> We created a 'catch them all' string/buffer type plus API. It is a good
>> compromise for all kinds of situations, works like a memory buffer but
>> is guaranteed 0-terminated, allows custom stack buffers with fallback to
>> heap if to small.
>>
>> https://gitlab.com/gnuwget/wget2/blob/master/libwget/buffer.c
>>
>>
>> There also is a sprintf functionality (glibc compatible) using these
>> buffers - and the operation is well faster than glibc's sprintf-like
>> functions for all format strings tested (tested back a few years). The
>> code is also much smaller (380 C code lines), the return values are
>> size_t. It doesn't support float/double.
>>
>> https://gitlab.com/gnuwget/wget2/blob/master/libwget/buffer_printf.c
>>
>> If there is serious interest, I could prepare modules for gnulib.
> 
> It is interesting that your solution does not only cover the simple cases
> (string concatenation, etc.), but also the more complex one, possibly
> with if()s in the generation logic, and all this without blatant potential
> for buffer overflow bugs.
> 
> So, the solution would consists of the following parts:
>   (A) A growable buffer type, with up to N (128 or 1024 or so) bytes on
>   the stack.

Preferable, the initial size and if starting with heap or stack buffer
should be (runtime) configurable.
- initial size because it allows fine-tuning to better avoid reallocations
- initial stack if used as local / temporary buffer
- initial heap when you already know that the resulting string has to
persist the function return

Currently there is are two init functions (I leave away the wget namespace):
int buffer_init(buffer *buf, char *data, size_t size);
buffer *buffer_alloc(size_t size);

buffer_alloc creates a buffer instance on the heap and initializes it
with a heap buffer of size.

buffer_init(buf, data, date_size) initializes 'buf' with the given data
and data_size. data will not be free'd, so stack data can be used here.

buffer_init(buf, NULL, date_size) initializes 'buf' with freshly
allocated heap data of size 'data_size'.

buffer_init(buf, NULL, 0) initializes 'buf' with freshly allocated heap
data of size 128. We could leave this out - it's a currently unused
special case to avoid error handling.

Then there is
int buffer_ensure_capacity(buffer *buf, size_t size);


>   (B) A set of functions for appending to such a growable buffer.

To copy a number of bytes to the beginning (effectively dropping the
previous content):
size_t buffer_memcpy(buffer *buf, const void *data, size_t length);

To append a number of bytes:
size_t buffer_memcat(buffer *buf, const void *data, size_t length);

To copy a string to the beginning (effectively dropping the previous
content):
size_t buffer_strcpy(buffer *buf, const char *s);

To append a string:
size_t buffer_strcat(buffer *buf, const char *s);

To set a number of bytes at the beginning (effectively dropping the
previous content):
size_t buffer_memset(buffer *buf, char c, size_t length);

To append a number of the same bytes:
size_t buffer_memset_append(buffer *buf, char c, size_t length);


>   (C) A function for creating a heap-allocated 'char *' from a growable
>   buffer.

Currently we do:
buffer buf;
buffer_init(, NULL, date_size); // allocate buf.data on heap
... add stuff to buf ...
mydata = buf.data; buf.data = NULL;
buffer_deinit();

We could make up a (static inline) for this, named
void *buffer_deinit_transfer(buffer *buf);

This function could also call realloc() to shrink 'data' to it's
occupied length.

>   (D) Short-hand functions for the simple cases (like string concatenation).

See above, e.g.
buffer_strcpy(buf, scheme);
buffer_strcat(buf, "://");
buffer_strcat(buf, domain);
buffer_memcat(buf, ":", 1);
buffer_strcat(buf, port_s);
buffer_memcat(buf, "/", 1);
buffer_strcat(buf, path);

But I prefer the slightly slower but better readable form
buffer_printf("%s://%s:%d/%s", scheme, domain, port, path);

Since our printf-like functions directly write into a buffer, there is
no overhead for copying data.

> It would be good to have all these well integrated (in terms of function
> names and calling conventions). So far, in gnulib, we have only pieces of
> it:
>   - Module 'scratch_buffer' is (A) without (B), (C), (D).
>   - Modules 'vasnprintf', 'asprintf' are (B), (C), (D) but without (A).
> 
> Before you start writing the code, it's worth looking at the following
> questions:
>   * Should the module 'scratch_buffer' be reused for (A)? Or is this
> not possible? If not, can it still have a memory-leak prevention
> like described in lib/malloc/scratch_buffer.h?

I don't see the advantage of the described memory-leak prevention. On
memory error 

Re: string types

[Tim Rühsen]

On 12/31/19 10:53 AM, Bruno Haible wrote:
> Hi Tim,
> 
>>>   - providing primitives for string allocation reduces the amount of buffer
>>> overflow bugs that otherwise occur in this area. [1]
>>> [1] https://lists.gnu.org/archive/html/bug-gnulib/2019-09/msg00031.html
> 
>> here is a string concatenation function without ellipsis, analogue to
>> writev() and struct iovec - just a suggestion. Instead of 'struct
>> strvec' a new string_t type would be handy.
>>
>> #include 
>> #include 
>> #include 
>> #include 
>>
>> struct strvec {
>>   char *strv_base;
>>   size_t strv_len;
>> };
>>
>> __attribute__ ((nonnull (1)))
>> char *concat_stringv(const struct strvec *strv)
>> {
>>   const struct strvec *str;
>>   size_t len = 0;
>>   char *buf;
>>
>>   for (str = strv; str->strv_base; str++)
>> len += str->strv_len;
>>
>>   if (!(buf = malloc(len + 1)))
>> return buf;
>>
>>   len = 0;
>>   for (str = strv; str->strv_base; len += str->strv_len, str++)
>> memcpy(buf + len, str->strv_base, str->strv_len);
>>
>>   buf[len] = 0;
>>
>>   return buf;
>> }
>>
>> void main(void)
>> {
>>   char *s = concat_stringv((struct strvec []) {
>> { "a", 1 },
>> { "b", 1 },
>> { NULL }
>>   });
> 
> This looks good. It brings us one step closer to the stated goal [1].
> 
> Would you like to contribute such a 'string-alloc' module that, together with
> 'strdup' and 'asprintf', removes most needs to create a string's contents
> "by hand"?

When time allows, I would like to make up a module.

Though IMO the design of the function doesn't allow to reuse an existing
buffer (e.g. a scratch buffer on the stack). Since malloc() etc are
pretty costly, you often want to avoid it as much as possible.

Like e.g.

/* Use given stack buffer, fallback to malloc() if too short */
char sbuf[256];
char *s = concat_stringv_stack(sbuf, sizeof (sbuf), (struct strvec []) {
{ "a", 1 },
{ "b", 1 },
{ NULL }
  });

... do things with s ...

if (s != sbuf)
  free (s);

Sometimes you want to reuse an existing malloc'ed buffer:

/* Use existing heap buffer, use realloc() if too short */
char *buf = malloc(N);
char *buf = concat_stringv_reuse(buf, N, (struct strvec []) {
{ "a", 1 },
{ "b", 1 },
{ NULL }
  });

... do things with s ...

free (buf);

You might also be interested in the size of the created string to avoid
a superfluous strlen(). So the need for more specialized functions makes
it all more and more complex.

During the development of Libwget/Wget2 we needed all of the above (and
more) and finally came up with a good compromise (well, good for us).

We created a 'catch them all' string/buffer type plus API. It is a good
compromise for all kinds of situations, works like a memory buffer but
is guaranteed 0-terminated, allows custom stack buffers with fallback to
heap if to small.

$ cloc buffer.c
Language files  blankcomment   code
---
C1 49327195

https://gitlab.com/gnuwget/wget2/blob/master/libwget/buffer.c


There also is a sprintf functionality (glibc compatible) using these
buffers - and the operation is well faster than glibc's sprintf-like
functions for all format strings tested (tested back a few years). The
code is also much smaller (380 C code lines), the return values are
size_t. It doesn't support float/double.

$ cloc buffer_printf.c
Language files  blankcomment   code
---
C1 74120380

https://gitlab.com/gnuwget/wget2/blob/master/libwget/buffer_printf.c

If there is serious interest, I could prepare modules for gnulib.


Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: string types

[Tim Rühsen]

On 27.12.19 11:51, Bruno Haible wrote:
> Aga wrote:
>> I do not know if
>> you can (or if it is possible, how it can be done), extract with a way a 
>> specific
>> a functionality from gnulib, with the absolute necessary code and only that.
> 
> gnulib-tool does this. With its --avoid option, the developer can even 
> customize
> their notion of "absolutely necessary".
> 
>> In a myriad of codebases a string type is implemented at least as:
>>   size_t mem_size;
>>   size_t num_bytes;
>>   char *bytes;
> 
> This is actually a string-buffer type. A string type does not need two size_t
> members. Long-term experience has shown that using different types for string
> and string-buffer is a win, because
>   - a string can be put in a read-only virtual memory area, thus enforcing
> immutability (-> reducing multithread problems),
>   - providing primitives for string allocation reduces the amount of buffer
> overflow bugs that otherwise occur in this area. [1]
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2019-09/msg00031.html
>

Just FYI,

here is a string concatenation function without ellipsis, analogue to
writev() and struct iovec - just a suggestion. Instead of 'struct
strvec' a new string_t type would be handy.

#include 
#include 
#include 
#include 

struct strvec {
  char *strv_base;
  size_t strv_len;
};

__attribute__ ((nonnull (1)))
char *concat_stringv(const struct strvec *strv)
{
  const struct strvec *str;
  size_t len = 0;
  char *buf;

  for (str = strv; str->strv_base; str++)
len += str->strv_len;

  if (!(buf = malloc(len + 1)))
return buf;

  len = 0;
  for (str = strv; str->strv_base; len += str->strv_len, str++)
memcpy(buf + len, str->strv_base, str->strv_len);

  buf[len] = 0;

  return buf;
}

void main(void)
{
  char *s = concat_stringv((struct strvec []) {
{ "a", 1 },
{ "b", 1 },
{ NULL }
  });

  puts(s);

  free(s);
}


In GNU Wget2 we already have type similar to string_t. Just used in
cases where we need pointer + len of URLs inside const HTML/XML/CSS data.

typedef struct {
const char
*p; //!< pointer to memory region
size_t
len; //!< length of memory region
} wget_string;


So maybe we need a string_t and a const_string_t type !? (to avoid
casting from const char *)

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: immutable string type

[Tim Rühsen]

On 29.12.19 19:49, Paul Eggert wrote:
> On 12/29/19 4:07 AM, Tim Rühsen wrote:
>> Introducing a stronger 'const' could be helpful in some situations
> 
> D has 'immutable' for that. It doesn't work as well as one might think.

It all depends on the implementation / definition of 'immutable' and how
it is used. Not sure what the actual problems are within D, though.

A quick web search didn't give me any complaints about immutable in D.
Do you have some examples or can you elaborate a bit ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: immutable string type

[Tim Rühsen]

On 29.12.19 10:45, Bruno Haible wrote:
> Tim Rühsen wrote:
>> the use cases are mostly in the testing area (especially fuzzing).
> 
> Indeed. During fuzzing, you want to check against any kind of buggy/undefined
> behaviour, and writing into arbitrary memory is one of these kinds.
> 
> This brings up the question: Should such as facility be in a Sanitizer and
> not in a library? I think the answer is "no", because
> 
>   - Writing into a string is not invalid in C. Even casting a 'const char *'
> to 'char *' and then writing into it is valid. The reason is that the
> C standard only makes statements about a program as a whole and therefore
> cannot express constraints such as "function A is allowed to write into
> the memory object M but function B is not".

I agree that such a thing doesn't make sense in a sanitizer, as only the
application/programmer knows about the semantics.

> 
>   - Integer overflow checking, for example, is available in both the 
> Sanitizers
> and library code. Apparently it is useful enough that some applications
> want to have it enabled in production code. I believe the same will be
> true for immutables string or memory regions.

Makes sense, especially as lot's of code is never being fuzzed to a full
degree.

> 
>> As a more general approach, a function that switches already allocated
>> memory into read-only memory would be handy. Like in
>>  - m = malloc()
>>  - initialize m with some data
>>  - if in debug mode: call memmap_readonly(m) - from this point on 'm' is
>> read-only and a write leads to a segmentation fault.
>>  - ...
>>  - free(m)
> 
> Hardware has write barriers only on the page level. You can't easily request
> a write barrier for a requence of, say, 30 bytes. To accomodate this, the
> API needs to have a certain shape. Paul wrote:

True, and it means that immalloc() always allocate multiples of the page
size (page is 4096 bytes on x86_64 ?). How do you plan to optimize
memory usage here ?

> 
>>  p = immalloc (sizeof *p);
>>  p->x = whatever; p->y = something; ...
>>  imfreeze (p, sizeof *p);
>>  [no changes to *p allowed here]
>>  imfree (p);
> 
> The third line needs to be something like
> 
>p = imfreeze (p, sizeof *p);
> 
> because the "writable p" and the "read-only p" will be at different virtual
> addresses.

Ah, now I get it - you have two virtual memory addresses pointing to the
same physical memory area.


Just throwing in another thought as an addition to immalloc etc:

Introducing a stronger 'const' could be helpful in some situations - the
compiler could find possible violations during the build phase. Stronger
in the means of "not allowed to be cast to non-const, but allowed to be
cast to const". This can be achieved by either extending the C standard,
by adding a new option to gcc or by adding a new __attribute__ for gcc.

Example:
#define imconst __attribute__ ((immutable))
#define transfer_ptr(dst) ({ typeof(dst) _t=(dst); (dst)=NULL; _t; })

char *tmp = malloc()
... initialize tmp...
imconst char *m = transfer_ptr(tmp);
// now m is save against writing if architecture has a MMU

char *m2 = strdup(m); // allowed
memcpy(m, m2, ...); // compiler error (implicit cast 'imconst char *' to
'void *' not allowed)
memcpy((void *) m, m2, ...); // same compiler error (without 'implicit')

Maybe we can think this through to either drop it or make a suggestion
to the gcc folks.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: immutable string type

[Tim Rühsen]

Hi Bruno,

On 28.12.19 12:17, Bruno Haible wrote:
> Would you find it useful to have an immutable string type in gnulib?

The idea is good in fact had similar thoughts/needs a while ago. IMO,
the use cases are mostly in the testing area (especially fuzzing).

As a more general approach, a function that switches already allocated
memory into read-only memory would be handy. Like in
 - m = malloc()
 - initialize m with some data
 - if in debug mode: call memmap_readonly(m) - from this point on 'm' is
read-only and a write leads to a segmentation fault.
 - ...
 - free(m)

Maybe it would best be integrated into glibc ?

Functions like iasprintf could then be built around existing functions
as needed, e.g. as static inline or as macro.

> In the simplest case, this would a 'const char *' where the 'const' is
> actually checked by the hardware. You allocate it through
> 
>const char *str = iasprintf (...);
> 
> You use it like any 'const char *'.
> 
> You free it through
> 
>ifree (str);
> 
> not free (str). And when you attempt to write into it:
> 
>((char *) str)[0] = 'x';
> 
> it crashes.
> 
> The benefits I imagine:
>   - no worry about security flaws through multithreaded accesses,
>   - in large applications: verification that no part of the application
> is doing side effects that it shouldn't.
> 
> The implementation uses mmap() to create a read-only and a read-write
> view of the same memory area. The contents of the string is filled through
> the read-write view. All other operations are done through the read-only
> view, because the address os the string is the one of the read-only view.
> 
> This won't work on all platforms, e.g. HP-UX. But it will work on glibc
> systems, BSD, and Solaris, at least.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: hard-locale: make multithread-safe

[Tim Rühsen]

Hi Bruno, hi gnulib developers,

it's a joy to follow the posts on this list - you (all) surprise,
impress and inspire me with your code but even more with your detailed
explanations / documentations.

Thank you so much for your ongoing work !!!

[E.g. this post made me check my code for mbtowc/mbrtowc (and also to
read the man pages again). In fact I found a call to mbtowc in MT code
(protected by a mutex) without resetting the internal state.]

Regards, Tim

On 12/17/19 2:45 PM, Bruno Haible wrote:
> Hi Paul,
> 
> Here is a proposed patch to make the hard_locale() function multithread-safe.
> This is needed because our mbrtowc() override relies on hard_locale, and
> mbrtowc obviously must be multi-thread safe (that's one of its main features,
> compared to mbtowc).
> 
> The previous hard_locale code tries to guess whether a locale is in fact
> a "C"/"POSIX" locale, although it is not apparent from its name. This was
> a case to worry about between 1995 and 2000, when many systems did not
> have working locales. This has changed: Nowadays nearly all platforms
> honour the locale names with some localized behaviour, except OpenBSD,
> Minix, and Android. It's not my priority to optimize for these three
> systems. But if you want to keep optimizations for these platforms,
> we could add #ifs for these platforms.



signature.asc
Description: OpenPGP digital signature

Re: GNUlib unicode encoding causes smart quotes to be displayed in program's output

[Tim Rühsen]

On 12/5/19 4:12 PM, Wes Hurd wrote:
> Hi,
> 
> It seems GNUlib quote encoding goes to Unicode smart quotes, which causes
> command-line program output to be in smart quotes.
> Smart quotes are dangerous for programmers and technical users, and should
> be avoided in program output.
> 
> Originally noticed with wget -
> https://savannah.gnu.org/bugs/index.php?57356#comment1
> 
> Can you change it to use only regular typed " quotes , at least with STDOUT
> / STDERR ?

All the GNU tools support localization. And there seems to be some kind
of inconsistency.


$ LANGUAGE=de cp
cp: Fehlender Dateioperand
„cp --help“ liefert weitere Informationen.


$ LANGUAGE=de wget
wget: URL fehlt
Aufruf: wget [OPTION]... [URL] …

»wget --help« gibt weitere Informationen.


Which one is correct ?

Maybe someone here can bring light into this or give us a pointer where
to get clarification.

Wes, maybe you can elaborate why you think that smart quotes are
*dangerous* ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Space separator missing in gnulib when building wget2

[Tim Rühsen]

Hi Dago,

On 30.11.19 18:33, Dagobert Michelsen wrote:
> Am 30.11.2019 um 16:28 schrieb Tim Rühsen :
>> On 30.11.19 15:15, Bruno Haible wrote:
>>>> There is one LF at the end of the diff file (taken from the email I
>>>> received via mailing list, also cross-checked with bug-gnulib archive).
>>>
>>> Indeed, the one in bug-gnulib archive and even the one I get by using
>>> the web access to my IMAP folder have the trailing newline. So it must
>>> be 'kmail' (my MUA) which removes the trailing newline. Apparently it
>>> does so only when processing signed emails...
>>>
>>> Sorry for having put the blame on your environment. It is in my environment.
>>
>> I didn't understood that as "blame" :-)
>>
>> Just another interesting detail/bug worth looking at...
> 
> Thanks for fixing the issue so quickly! I can verify that I can now build
> wget2 cleanly. I’ll see that the wget2 CI on Solaris is usable again :-)

:-)

Thanks for reporting with all the details that were needed to track this
down.

The Wget2 CI on Gitlab is currently struggling with "502 Bad Gateway"
from Savannah. That made me finally create a gnulib mirror on Gitlab -
we often saw CI failures due to Savannah not being stable.

Anyways, as soon as the CI is successful with the gnulib update, the
master branch will be updated.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Space separator missing in gnulib when building wget2

[Tim Rühsen]

Hi Bruno,

On 30.11.19 15:15, Bruno Haible wrote:
>> There is one LF at the end of the diff file (taken from the email I
>> received via mailing list, also cross-checked with bug-gnulib archive).
> 
> Indeed, the one in bug-gnulib archive and even the one I get by using
> the web access to my IMAP folder have the trailing newline. So it must
> be 'kmail' (my MUA) which removes the trailing newline. Apparently it
> does so only when processing signed emails...
> 
> Sorry for having put the blame on your environment. It is in my environment.

I didn't understood that as "blame" :-)

Just another interesting detail/bug worth looking at...

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Space separator missing in gnulib when building wget2

[Tim Rühsen]

Hi Bruno,

On 30.11.19 13:06, Bruno Haible wrote:
>> Please review the patch - surely some of my python constructs could be
>> done more elegant.
> 
> Review:
> - GLModuleSystem.getLink: Looks good. This method corresponds to
>   func_get_link_directive and therefore should not do any joining or so,
>   just return a list of strings.
> - GLEmiter.lib_Makefile_am: Looks good.
> - GLImport.execute: Looks good as well.
> 
> It corrects the issue reported by Dagobert. Therefore I'm pushing it in
> your name. (I don't remember if you already have direct write access to
> the gnulib repository, sorry.)

Thanks for reviewing and pushing !
I don't have write access, but that's fine as I just seldom contribute :-)


> By the way, something in your environment apparently stripped the trailing
> newline of the patch. As a consequence, I got this warning:
> $ patch -p1 < .../pygnulib.diff 
> patching file pygnulib/GLEmiter.py
> patching file pygnulib/GLImport.py
> patching file pygnulib/GLModuleSystem.py
> patch unexpectedly ends in middle of line
> Hunk #2 succeeded at 806 with fuzz 1.

There is one LF at the end of the diff file (taken from the email I
received via mailing list, also cross-checked with bug-gnulib archive).

Does patch expect 2 LF in this case ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Space separator missing in gnulib when building wget2

[Tim Rühsen]

On 11/28/19 11:11 PM, Bruno Haible wrote:
> Hi Dagobert,
> 
>  lib/gnulib.mk there is a suspicious addition to libgnu_la_LDFLAGS without 
> space:
>>
>> libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)$(LTLIBINTL)
>> ...
>> libgnu_la_LDFLAGS += $(LTLIBICONV)
>> libgnu_la_LDFLAGS += $(LTLIBINTL)
>> libgnu_la_LDFLAGS += $(LTLIBMULTITHREAD)
>> ```
>>
>> The file `lib/gnulib.mk` is generated during bootstrap. 
>>
>> What is actually executed during bootstrap is
>>
>> gnulib/gnulib-tool.py --no-changelog --aux-dir=build-aux --doc-base=doc 
>> --lib=libgnu --m4-base=m4/ --source-base=lib/ --tests-base=lib/tests 
>> --local-dir=gl --makefile-name=gnulib.mk --libtool --import accept access 
>> arpa_inet atoll bind c-strcase c-strcasestr c-ctype calloc-posix 
>> canonicalize-lgpl clock-time close closedir cond connect crypto/md2 
>> crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dirname dup dup2 errno 
>> fclose fcntl fdopen fflush flock fnmatch-gnu fopen freopen fstat fsync 
>> ftruncate futimens getaddrinfo getpass getsockname gettext-h gettime 
>> gitlog-to-changelog glob iconv inet_pton inline inttypes ioctl isatty 
>> lib-symbol-visibility limits-h link listen lock maintainer-makefile 
>> malloc-posix memchr mkdir mkstemp msvc-nothrow nanosleep netdb netinet_in 
>> nl_langinfo open opendir pclose pipe-posix progname popen poll posix_spawn 
>> pwrite qsort_r random_r read readdir realloc-posix recv recvfrom regex 
>> rename safe-read safe-write select send sendto servent setlocale setsockopt 
>> socket sockets socklen spawn-pipe stdarg stdbool stddef stdint stat strcase 
>> strchrnul strdup-posix strerror strndup strpbrk strstr strtoll sys_file 
>> sys_socket sys_stat sys_time sys_types sys_uio thread time_r unistd unlink 
>> update-copyright warnings wcwidth write xgethostname
>>
>> and this command reproduces the error in `lib/gnulib.mk`
> 
> Thanks for the test case. When I run this command
>   - once with gnulib-tool (shell script),
>   - once with gnulib-tool.py,
> I see these differences in the generated lib/gnulib.mk:
> 
> @@ -168,7 +35,7 @@
>  EXTRA_libgnu_la_SOURCES =
>  libgnu_la_LDFLAGS = $(AM_LDFLAGS)
>  libgnu_la_LDFLAGS += -no-undefined
> -libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)
> +libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)$(LTLIBINTL)
>  libgnu_la_LDFLAGS += $(GETHOSTNAME_LIB)
>  libgnu_la_LDFLAGS += $(HOSTENT_LIB)
>  libgnu_la_LDFLAGS += $(INET_NTOP_LIB)
> @@ -187,7 +54,7 @@
>  libgnu_la_LDFLAGS += $(LTLIBMULTITHREAD)
>  libgnu_la_LDFLAGS += $(LTLIBTHREAD)
>  libgnu_la_LDFLAGS += $(SERVENT_LIB)
> -libgnu_la_LDFLAGS += @INTL_MACOSX_LIBS@
> +libgnu_la_LDFLAGS += @INTL_MACOSX_LIBS@$(LTLIBTHREAD)
>  
>  ## begin gnulib module absolute-header
>  
> Lines 
> libgnu_la_LDFLAGS += $(LTLIBINTL)
> and
> libgnu_la_LDFLAGS += $(LTLIBTHREAD)
> are already part of the output. Therefore the right fix is not to insert a
> space, but to have gnulib-tool.py generate the same lines as the shell script.
> 
> FYI, in the shell script it is the line gnulib-tool:3856.

Attached is a diff for review that fixes the issue.

Indeed the 'space fix' above only worked for
Link:
$(LIB1)
$(LIB2) optional comment

The diff fixes this to also allow (even if not found in modules yet)
Link:
$(LIB1) optional comment 1
$(LIB2) optional comment 2

Then, the double $([LT]LIB...) entries are removed and sorted as
gnulib-tool does it by 'sort -u'.

Please review the patch - surely some of my python constructs could be
done more elegant.

Regards, Tim
diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py
index a350ba753..f0746f827 100644
--- a/pygnulib/GLEmiter.py
+++ b/pygnulib/GLEmiter.py
@@ -853,17 +853,18 @@ AC_DEFUN([%V1%_LIBSOURCES], [
 emit += '%s_%s_LDFLAGS += -no-undefined\n' % (libname, libext)
 # Synthesize an ${libname}_${libext}_LDFLAGS augmentation by combining
 # the link dependencies of all modules.
-listing = list()
 links = [module.getLink()
  for module in modules if not module.isTests()]
+ulinks = list()
 for link in links:
-link = constants.nlremove(link)
-position = link.find(' when linking with libtool')
-if position != -1:
-link = link[:position]
-listing += [link]
-listing = sorted(set([link for link in listing if link != '']))
-for link in listing:
+for lib in link:
+lib = constants.nlremove(lib)
+position = lib.find(' when linking with libtool')
+if position != -1:
+lib = lib[:position]
+ulinks += [lib]
+ulinks = sorted(set(ulinks))
+for link in ulinks:
 emit += '%s_%s_LDFLAGS += %s\n' % (libname, libext, link)
 emit += '\n'
 if pobase:
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 19ade5e8d..27a01be43 100644
--- 

Re: Space separator missing in gnulib when building wget2

[Tim Rühsen]

On 11/28/19 11:11 PM, Bruno Haible wrote:
> Hi Dagobert,
> 
>  lib/gnulib.mk there is a suspicious addition to libgnu_la_LDFLAGS without 
> space:
>>
>> libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)$(LTLIBINTL)
>> ...
>> libgnu_la_LDFLAGS += $(LTLIBICONV)
>> libgnu_la_LDFLAGS += $(LTLIBINTL)
>> libgnu_la_LDFLAGS += $(LTLIBMULTITHREAD)
>> ```
>>
>> The file `lib/gnulib.mk` is generated during bootstrap. 
>>
>> What is actually executed during bootstrap is
>>
>> gnulib/gnulib-tool.py --no-changelog --aux-dir=build-aux --doc-base=doc 
>> --lib=libgnu --m4-base=m4/ --source-base=lib/ --tests-base=lib/tests 
>> --local-dir=gl --makefile-name=gnulib.mk --libtool --import accept access 
>> arpa_inet atoll bind c-strcase c-strcasestr c-ctype calloc-posix 
>> canonicalize-lgpl clock-time close closedir cond connect crypto/md2 
>> crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dirname dup dup2 errno 
>> fclose fcntl fdopen fflush flock fnmatch-gnu fopen freopen fstat fsync 
>> ftruncate futimens getaddrinfo getpass getsockname gettext-h gettime 
>> gitlog-to-changelog glob iconv inet_pton inline inttypes ioctl isatty 
>> lib-symbol-visibility limits-h link listen lock maintainer-makefile 
>> malloc-posix memchr mkdir mkstemp msvc-nothrow nanosleep netdb netinet_in 
>> nl_langinfo open opendir pclose pipe-posix progname popen poll posix_spawn 
>> pwrite qsort_r random_r read readdir realloc-posix recv recvfrom regex 
>> rename safe-read safe-write select send sendto servent setlocale setsockopt 
>> socket sockets socklen spawn-pipe stdarg stdbool stddef stdint stat strcase 
>> strchrnul strdup-posix strerror strndup strpbrk strstr strtoll sys_file 
>> sys_socket sys_stat sys_time sys_types sys_uio thread time_r unistd unlink 
>> update-copyright warnings wcwidth write xgethostname
>>
>> and this command reproduces the error in `lib/gnulib.mk`
> 
> Thanks for the test case. When I run this command
>   - once with gnulib-tool (shell script),
>   - once with gnulib-tool.py,
> I see these differences in the generated lib/gnulib.mk:
> 
> @@ -168,7 +35,7 @@
>  EXTRA_libgnu_la_SOURCES =
>  libgnu_la_LDFLAGS = $(AM_LDFLAGS)
>  libgnu_la_LDFLAGS += -no-undefined
> -libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)
> +libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)$(LTLIBINTL)
>  libgnu_la_LDFLAGS += $(GETHOSTNAME_LIB)
>  libgnu_la_LDFLAGS += $(HOSTENT_LIB)
>  libgnu_la_LDFLAGS += $(INET_NTOP_LIB)
> @@ -187,7 +54,7 @@
>  libgnu_la_LDFLAGS += $(LTLIBMULTITHREAD)
>  libgnu_la_LDFLAGS += $(LTLIBTHREAD)
>  libgnu_la_LDFLAGS += $(SERVENT_LIB)
> -libgnu_la_LDFLAGS += @INTL_MACOSX_LIBS@
> +libgnu_la_LDFLAGS += @INTL_MACOSX_LIBS@$(LTLIBTHREAD)
>  
>  ## begin gnulib module absolute-header
>  
> Lines 
> libgnu_la_LDFLAGS += $(LTLIBINTL)
> and
> libgnu_la_LDFLAGS += $(LTLIBTHREAD)
> are already part of the output. Therefore the right fix is not to insert a
> space, but to have gnulib-tool.py generate the same lines as the shell script.
> 
> FYI, in the shell script it is the line gnulib-tool:3856.
> 
> Any volunteer for doing this?

As far as it keeps calm today, let me give it a try.
Need to polish my almost zero python experience ;-)

But first let's get the algorithm straight.
In modules/getaddrinfo (and some other modules) are 2 'Link' libraries
given:

Link:
$(GETADDRINFO_LIB)
$(LTLIBINTL) when linking with libtool, $(LIBINTL) otherwise

So "libgnu_la_LDFLAGS += $(GETADDRINFO_LIB) $(LTLIBINTL)" would be right
in the first place. But we don't want to add any of those libraries in
case libgnu_la_LDFLAGS already contain them (avoiding duplicates) !?

Is that right ?

And what about the order of libraries ? AFAIR, gcc/ld 9+ rely on the
correct order of libraries while older gcc/ld version doesn't.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: [libmicrohttpd] CI Pipeline fails (doc / etex fails)

[Tim Rühsen]

@Paul

The error says
"/home/oms/src/libmicrohttpd/doc/gpl-2.0.texi:286: This command can
appear only outside of any environment, not in environment @enumerate."


Line 286 is
@heading NO WARRANTY

$ grep -n enumerate gpl-2.0.texi
67:@enumerate 0
104:@enumerate a
126:@end enumerate
153:@enumerate a
173:@end enumerate
314:@end enumerate

As you can see L67-L314 is an enumerate, where L286 (@heading) is enclosed.

So it looks like a problem in gpl-2.0.texi (or in the texi2dvi command
chain). Any ideas ?

Regards, Tim


On 10/18/19 9:54 AM, Tim Rühsen wrote:
> Hi Christian,
> 
> I can even reproduce on Debian unstable:
> 
> ./bootstrap
> ./configure
> make
> cd doc
> make dvi
> 
> Regards, Tim
> 
> On 10/17/19 3:11 PM, Christian Grothoff wrote:
>> I downloaded an Ubuntu 18.04.3 LTS VM image, installed the required
>> packages (including texlive 2017 and texinfo 6.5) and it _still_ works
>> for me. I did try the texi2dvi command and others.
>>
>> Tim: So sorry, but I cannot reproduce the problem.
>>
>> Paul: I also completely fail to comprehend the error message and how it
>> would point to an issue in libmicrohttpd.texi. But if it doesn't reflect
>> an issue in gpl-2.0.texi, I guess you're off the hook ;-).
>>
>> Happy hacking!
>>
>> Christian
>>
>> On 10/13/19 9:14 PM, Paul Eggert wrote:
>>> On 10/13/19 9:01 AM, Christian Grothoff wrote:
>>>> I suspect what you discovered could be a bug in gnulib when
>>>> the license file is used against some particular (maybe very recent)
>>>> texinfo installation/version.
>>>
>>> I see the problem on Ubuntu 18.04.3 LTS which has GNU texinfo 6.5 and
>>> TeX Live 2017/Debian. It's due to a bug in libmicrohttpd.texi that is
>>> unrelated to Gnulib's gpl-2.0.texi. If I install the attached patch,
>>> "make dvi" outputs the following diagnostics. I suggest getting an
>>> appropriate texinfo and TeX Live to debug this.
>>>
>>> make[2]: Entering directory '/home/eggert/junk/d/libmicrohttpd/doc'
>>> TEXINPUTS="../build-aux:$TEXINPUTS" \
>>> MAKEINFO='/bin/bash /home/eggert/junk/d/libmicrohttpd/build-aux/missing
>>> makeinfo   -I .' \
>>> texi2dvi  --build-dir=libmicrohttpd.t2d -o libmicrohttpd.dvi  \
>>> libmicrohttpd.texi
>>> This is pdfTeX, Version 3.14159265-2.6-1.40.18 (TeX Live 2017/Debian)
>>> (preloaded format=etex)
>>>  restricted \write18 enabled.
>>> entering extended mode
>>> (../../../.././libmicrohttpd.texi
>>> (/home/eggert/junk/d/libmicrohttpd/build-aux/texinfo.tex
>>> Loading texinfo [version 2013-02-01.11]: pdf, fonts, markup, glyphs,
>>> page headings, tables, conditionals, indexing, sectioning, toc,
>>> environments,
>>> defuns, macros, cross references, insertions,
>>> (/usr/share/texlive/texmf-dist/tex/generic/epsf/epsf.tex
>>> This is `epsf.tex' v2.7.4 <14 February 2011>
>>> ) localization, formatting, and turning on texinfo input format.)
>>> (./libmicrohttpd.aux)
>>> Runaway argument?
>>> {microhttpd-optio
>>> ../../../.././libmicrohttpd.texi:2: File ended while scanning use of
>>> @xrdef.
>>> 
>>>     @par
>>> @readdatafile ...etupdatafile @input @jobname .#1
>>>   @endgroup
>>> @tryauxfile ... @ifeof 1 @else @readdatafile {aux}
>>>   @global @havexrefstrue
>>> @fi...
>>>
>>> @setfilename ->@fixbackslash @iflinks @tryauxfile
>>>   @immediate @openout
>>> @auxfi...
>>> l.2 @setfilename
>>>  libmicrohttpd.info
>>> ?
>>
> 



signature.asc
Description: OpenPGP digital signature

Re: [libmicrohttpd] CI Pipeline fails (doc / etex fails)

[Tim Rühsen]

Hi Christian,

I can even reproduce on Debian unstable:

./bootstrap
./configure
make
cd doc
make dvi

Regards, Tim

On 10/17/19 3:11 PM, Christian Grothoff wrote:
> I downloaded an Ubuntu 18.04.3 LTS VM image, installed the required
> packages (including texlive 2017 and texinfo 6.5) and it _still_ works
> for me. I did try the texi2dvi command and others.
> 
> Tim: So sorry, but I cannot reproduce the problem.
> 
> Paul: I also completely fail to comprehend the error message and how it
> would point to an issue in libmicrohttpd.texi. But if it doesn't reflect
> an issue in gpl-2.0.texi, I guess you're off the hook ;-).
> 
> Happy hacking!
> 
> Christian
> 
> On 10/13/19 9:14 PM, Paul Eggert wrote:
>> On 10/13/19 9:01 AM, Christian Grothoff wrote:
>>> I suspect what you discovered could be a bug in gnulib when
>>> the license file is used against some particular (maybe very recent)
>>> texinfo installation/version.
>>
>> I see the problem on Ubuntu 18.04.3 LTS which has GNU texinfo 6.5 and
>> TeX Live 2017/Debian. It's due to a bug in libmicrohttpd.texi that is
>> unrelated to Gnulib's gpl-2.0.texi. If I install the attached patch,
>> "make dvi" outputs the following diagnostics. I suggest getting an
>> appropriate texinfo and TeX Live to debug this.
>>
>> make[2]: Entering directory '/home/eggert/junk/d/libmicrohttpd/doc'
>> TEXINPUTS="../build-aux:$TEXINPUTS" \
>> MAKEINFO='/bin/bash /home/eggert/junk/d/libmicrohttpd/build-aux/missing
>> makeinfo   -I .' \
>> texi2dvi  --build-dir=libmicrohttpd.t2d -o libmicrohttpd.dvi  \
>> libmicrohttpd.texi
>> This is pdfTeX, Version 3.14159265-2.6-1.40.18 (TeX Live 2017/Debian)
>> (preloaded format=etex)
>>  restricted \write18 enabled.
>> entering extended mode
>> (../../../.././libmicrohttpd.texi
>> (/home/eggert/junk/d/libmicrohttpd/build-aux/texinfo.tex
>> Loading texinfo [version 2013-02-01.11]: pdf, fonts, markup, glyphs,
>> page headings, tables, conditionals, indexing, sectioning, toc,
>> environments,
>> defuns, macros, cross references, insertions,
>> (/usr/share/texlive/texmf-dist/tex/generic/epsf/epsf.tex
>> This is `epsf.tex' v2.7.4 <14 February 2011>
>> ) localization, formatting, and turning on texinfo input format.)
>> (./libmicrohttpd.aux)
>> Runaway argument?
>> {microhttpd-optio
>> ../../../.././libmicrohttpd.texi:2: File ended while scanning use of
>> @xrdef.
>> 
>>     @par
>> @readdatafile ...etupdatafile @input @jobname .#1
>>   @endgroup
>> @tryauxfile ... @ifeof 1 @else @readdatafile {aux}
>>   @global @havexrefstrue
>> @fi...
>>
>> @setfilename ->@fixbackslash @iflinks @tryauxfile
>>   @immediate @openout
>> @auxfi...
>> l.2 @setfilename
>>  libmicrohttpd.info
>> ?
> 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Allow error output to be printed

[Tim Rühsen]

Hi Bernhard,

it's already in gnulib since yesterday. I didn't expect you to come up
with a patch, else I would have informed you on the bug tracker.

Please excuse my lack of communication here.

Regards, Tim

On 10/7/19 11:13 AM, Bernhard M. Wiedemann wrote:
> e.g. when running in a build environment without CA certificates,
> wget would silently fail without giving a hint to the user
> about the cause.
>
> For background see
> https://gitlab.com/gnuwget/wget2/merge_requests/450#note_226179055
>
> It seems, -nv still shows some non-error output.
>
> wget -q was introduced in commit feaf4df70588cf1ee30b4879a1048cc143135a67
>
> * build-aux/bootstrap: allow wget error output to be printed
> ---
>  build-aux/bootstrap | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/build-aux/bootstrap b/build-aux/bootstrap
> index 5b08e7e2d..e273ea732 100755
> --- a/build-aux/bootstrap
> +++ b/build-aux/bootstrap
> @@ -166,7 +166,7 @@ bootstrap_epilogue() { :; }
>  # specified directory.  Fill in the first %s with the destination
>  # directory and the second with the domain name.
>  po_download_command_format=\
> -"wget --mirror --level=1 -nd -q -A.po -P '%s' \
> +"wget --mirror --level=1 -nd -nv -A.po -P '%s' \
>   https://translationproject.org/latest/%s/;
>
>  # Prefer a non-empty tarname (4th argument of AC_INIT if given), else
>

[PATCH] users.txt: Fix URL for libidn2

[Tim Rühsen]

From 3bde5ad266986687ad38a8742244099970ed7cc7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20R=C3=BChsen?= 
Date: Sun, 6 Oct 2019 16:52:23 +0200
Subject: [PATCH] users.txt: Fix URL for libidn2

---
 users.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/users.txt b/users.txt
index 84863c449..004875bad 100644
--- a/users.txt
+++ b/users.txt
@@ -50,7 +50,7 @@ The following packages appear to be using gnulib and gnulib-tool:
   libguestfs  https://github.com/libguestfs/libguestfs
   libiconvhttps://git.sv.gnu.org/gitweb/?p=libiconv.git
   libidn  https://git.sv.gnu.org/gitweb/?p=libidn.git
-  libidn2 https://josefsson.org/libidn2/
+  libidn2 https://gitlab.com/libidn/libidn2
   libksba https://www.gnupg.org/related_software/libksba/index.html
   libntlm https://git.savannah.nongnu.org/gitweb/?p=libntlm.git;a=tree
   libprelude  https://trac.prelude-ids.org/browser/trunk/libprelude/
-- 
2.23.0



signature.asc
Description: OpenPGP digital signature

Re: wget -q in bootstrap is too silent

[Tim Rühsen]

Thank you, Paul !

On 06.10.19 06:38, Paul Eggert wrote:
> On 10/4/19 8:34 AM, Tim Rühsen wrote:
>> Instead of -q we could use -nv to avoid disguising the real error.
>> But there will be some diagnostic noice...
> 
> At this point the bootstrap script is so noisy that a bit more won't
> hurt, especially if it's in a good cause. Thanks for reporting the
> problem; I installed the attached.



signature.asc
Description: OpenPGP digital signature

wget -q in bootstrap is too silent

[Tim Rühsen]

Hi,

we just had an issue with the bootstrap script because wget -q
suppresses error output.

E.g. instead of
"
ERROR: cannot verify translationproject.org's certificate, issued by
'CN=Let\'s Encrypt Authority X3,O=Let\'s Encrypt,C=US':
  Unable to locally verify the issuer's authority.
"
there is no output.

This lead later to the confusing error
"
automake: error: cannot open < lib/gnulib.mk: No such file or directory
"

Instead of -q we could use -nv to avoid disguising the real error.
But there will be some diagnostic noice...

WDYT ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: FreeBSD: Warnings about c-ctype macros used but marked unused

[Tim Rühsen]

Hi Bruno,

On 28.08.19 17:28, Bruno Haible wrote:
>> The option -Wused-but-marked-unused is indirectly activated by
>> -Weverything
> 
> -Weverything is not something we can support in gnulib. For the
> meaning of this option, see
> https://quuxplusone.github.io/blog/2018/12/06/dont-use-weverything/

I can't agree 100% with the statements in this article.

We use -Weverything + some -Wno- options successfully for our sources
since years. Though, building the gnulib sources has a different set of
options of course.

>> which is set during the ./configure run (kind of a
>> manywarnings module)
> 
> Why do we have a 'manywarnings' module, not an 'allwarnings' module?
> Look into the list of warnings that we don't add through 'manywarnings':
>   build-aux/gcc-warning.spec
>   build-aux/g++-warning.spec

Thanks for the pointers.

>> And I wonder why do the c_ macros are marked
>> UNUSED at all (I assume that gnulib does it for some reason) ?
> 
> 'c-ctype' uses the module 'extern-inline'. In extern-inline.m4 you can
> see that on platforms where 'extern inline' cannot properly be supported,
> we let _GL_INLINE expand to 'static _GL_UNUSED'. Without '_GL_UNUSED',
> gcc (with some appropriate, useful warning options) would complain that
> the functions are not used.
> 
> The ability to use this warning option - which allows developers to
> detect dead code - is more important than the option -Wused-but-marked-unused.

That's reasonable. I'll put that option on the exclude list for the
gnulib sources. Thanks for the explanation, I didn't see that in the
first place (the FreeBSD is a CI image, not controlled by me).

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: FreeBSD: Warnings about c-ctype macros used but marked unused

[Tim Rühsen]

Hi Bruno,

On 8/28/19 4:09 PM, Bruno Haible wrote:
> Hi Tim,
> 
>> Compiling on FreeBSD 12 gives a warning per use of c_ macros, for example
>>
>> http_parse.c:187:10: warning: 'c_isblank' was marked unused but was used
>> [-Wused-but-marked-unused]
>> while (c_isblank(*s)) s++;
>>
>>
>> This is true also for c_isdigit, c_isspace, etc.
> 
> A bit more details, please:
>   - Which version of gcc or clang is this?
>   - Where does the option -Wused-but-marked-unused come from? Is it part
> of -Wall, or did you or your package add it explicitly?

From config.log:

FreeBSD clang version 6.0.0 (tags/RELEASE_600/final 326565) (based on
LLVM 6.0.0)
Target: x86_64-unknown-freebsd11.2
Thread model: posix
InstalledDir: /usr/bin

The option -Wused-but-marked-unused is indirectly activated by
-Weverything, which is set during the ./configure run (kind of a
manywarnings module). So it's not part of $CFLAGS or $CPPFLAGS but later
used as part of AM_CFLAGS.

I do not see that warning on Linux, with any version of clang (3.8, ...
9) or gcc (4.3, ..., 9.2). And I wonder why do the c_ macros are marked
UNUSED at all (I assume that gnulib does it for some reason) ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

FreeBSD: Warnings about c-ctype macros used but marked unused

[Tim Rühsen]

Compiling on FreeBSD 12 gives a warning per use of c_ macros, for example

http_parse.c:187:10: warning: 'c_isblank' was marked unused but was used
[-Wused-but-marked-unused]
while (c_isblank(*s)) s++;


This is true also for c_isdigit, c_isspace, etc.

Is that expected behavior and if so, why ?

Regards, Tim





signature.asc
Description: OpenPGP digital signature

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

[Tim Rühsen]

Hi Bruno,

sorry for the delay. This somewhat dropped off the tip of my list, but
lurks at the edge of my consciousness, coming up from time to time.

On 12/5/18 12:43 AM, Bruno Haible wrote:
> Hi Tim,
> 
>> Currently we have
>> - positive integer (N): resize by old value * N
>> - negative integer (N): resize by old value + (-N)
> 
> It's currently the opposite:
> - positive integer (N): resize to old size + N
> - negative integer (N): resize to old size * (-N)
> 
>>>   - The ability to set a growth policy > 0. This leads to quadratic
>>> run time behaviour. I mean, you make such an effort to have O(1)
>>> access on average, and then the fixed-size increment of the size
>>> turns the insertion operation into an average-O(n) operation.
>>
>> In Wget2 we use reasonable default sizes for the hashmap to make
>> resizing more or less a corner case.
> 
> The problem is that when you guessed wrong about the "corner case",
> the rehash will consume 99.9% of your program's run time.
> 
>> How can we do it better ?
> 
> 1. Remove the ability to "resize to old size + N",
> 2. (optional) When "resize to old size * FACTOR", allow the factor
>to be 1.5 or 1.3 or something like that.

That's what I did meanwhile, FACTOR is now a float and that's it.


>>>   - Some functions take keysize and valuesize arguments, some don't.
>>> I'm confused.
>>
>> The "noalloc" functions just store the given pointers for key and value,
>> that means without allocation. The hashmap takes ownership of the
>> objects and has to release them when being removed/destroyed (or use a
>> NULL destructor when the values are static). This can be used to
>> generate a search index for an existing bunch of values (objects /
>> structures). Like a "unique index" for a field in a database table.
>> Example function: wget_hashmap_put_noalloc()
>>
>> Then we have the functions that do (shallow) allocations/clones, like
>> wget_hashmap_put(). This is for using the hashmap as a container, the
>> original objects have to be (shallow) released outside of the container.
> 
> Hmm. What you describe here is whether ownership is passed to the container
> or kept outside the container. My question was about the keysize and
> valuesize arguments, that is, about the nature of the key and value objects
> (a. a pointer, pointing to anything in memory, or
>  b. a region of memory, with a start address and an end address).
> 
> I think both questions are orthogonal:
>   - In either a or b, you can keep the ownership outside the container
> by using a NULL destructor function.
>   - For container-owned objects:
> In case a, you need a clone or copy function indeed,
> In case b, the container needs to clone precisely the given region of
> memory.
> 
> But I haven't thought long about it. What do you think?

I thought a while about this and finally decided to leave memory
allocation outside the container, in the responsibility of the caller.

That reduces the number of functions, makes things easier to understand,
reduce overall code. That moves the API usage more into the direction of
a no-brainer. The additional code complexity in the application (wget2)
was much less than I feared.

Thank you for your hints / thoughts and please excuse me for taking so
long to answer.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Eliminate dead assignment in regex_internal.c/re_node_set_insert()

[Tim Rühsen]

Please consider applying this

diff --git a/lib/regex_internal.c b/lib/regex_internal.c
index 9004ce809..15f38a755 100644
--- a/lib/regex_internal.c
+++ b/lib/regex_internal.c
@@ -1311,7 +1311,6 @@ re_node_set_insert (re_node_set *set, Idx elem)
  first element separately to skip a check in the inner loop.  */
   if (elem < set->elems[0])
 {
-  idx = 0;
   for (idx = set->nelem; idx > 0; idx--)
set->elems[idx] = set->elems[idx - 1];
 }



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

On 7/10/19 9:32 PM, Akim Demaille wrote:
> I'm using 4.2.1, and it does what I meant: -p prints the rules,
> and -q (which is --question, not --quiet) avoids that we
> fired a rule (i.e., "make -q" does not run "make all").
> 
> So I'm just clueless here.  I don't know what to do to address
> your issue.

I can *NOT* reproduce the issue on Debian stretch (gawk 4.1.4, make 4.1).
Also, here the .deps/ directory is not being generated by make -qp.


Back to Debian unstable / Arch / Fedora 30.

I tested with good old wget and see the same issue.
The command sequence is
  git clone https://gitlab.com/gnuwget/wget.git
  cd wget
  ./bootstrap
  cd gnulib
  git checkout master
  git pull
  cd ..
  git commit -m "update gnulib" gnulib
  ./bootstrap
  ./configure
  make syntax-check

This also creates the .deps/ directory. So maybe make 4.2.1 has a bug as
it creates files though -q is given. But then, why can't you reproduce
with the same version of make ?

Earlier in this thread I attached a Dockerfile that makes it pretty easy
to reproduce. If you don't use docker, I could send you the complete
make -qp output.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

Hi Akim,

On 7/10/19 9:32 PM, Akim Demaille wrote:
> Hi Tim,
> 
> Sorry I dropped the ball...

NP, I dropped it as well ;-)

>> Reproducible everywhere (needs gawk being installed, else the
>> sc_prohibit_gnu_make_extensions is a no-op).
> 
> Which is what I meant.  So are you saying it work as (I) expected?

It works as expected for mawk which effectively means the test is skipped.

>> Akim, at least with GNU make 4.2.1 the combination of -q and -p doesn't
>> do what you expect. From the make man page, I would say that both
>> options contradict. -q: don't print anything; -p: print the database
> 
> I'm using 4.2.1, and it does what I meant: -p prints the rules,
> and -q (which is --question, not --quiet) avoids that we
> fired a rule (i.e., "make -q" does not run "make all").

I just summed up the man page and picked up the 'Do not ... print anything':
-q, --question
 ``Question mode''.  Do not run any commands, or print anything;
just return an exit status that  is  zero  if  the
 specified targets are already up to date, nonzero otherwise.


> So I'm just clueless here.  I don't know what to do to address
> your issue.

It looks like this is not *my* issue. The issue shows up on different
installations which have gawk and make 4.2.1 in common.

I will check if the issue pops up with older make versions or with other
projects as well.

> Are you running "make syntax-check" on a configured builddir?

No sure what you mean. The command sequence to trigger the issue is
straight forward:
./bootstrap
./configure
make
make check
make syntax-check <- bang

What (C) project + environment are you using successfully ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

On 7/10/19 11:38 AM, Tim Rühsen wrote:
> 
> 
> On 7/10/19 11:07 AM, Tim Rühsen wrote:
>> On 6/25/19 3:24 PM, Tim Rühsen wrote:
>>> Hi Akim,
>>>
>>> the command expands to
>>>
>>> if gawk --version | grep GNU >/dev/null 2>&1; then  \
>>>   (cd . && autoconf --trace AC_CONFIG_FILES:'$1') | \
>>> tr ' ' '\n' |   \
>>> /usr/bin/sed -ne '/Makefile/{s/\.in$//;p;}' |   \
>>> while read m; do\
>>>   make -qp -f $m .DUMMY-TARGET 2>/dev/null |\
>>> gawk -v file=$m -e 'BEGIN { RS = "\n\n"; in_rules = 0; } /^#
>>> Files/ { in_rules = 1; } /\$>> \.\w+(\.\w+)?:/ { print "Error: " file ": $< in a non implicit rule\n"
>>> $0; status = 1; } END { exit status; }' || exit 1;\
>>> done;   \
>>> fi
>>>
>>> # make --version
>>> GNU Make 4.2.1
>>> Built for x86_64-redhat-linux-gnu
>>>
>>> # gawk --version
>>> GNU Awk 4.2.1, API: 2.0 (GNU MPFR 3.1.6-p2, GNU MP 6.1.2)
>>>
>>> # sed --version
>>> sed (GNU sed) 4.5
>>>
>>> # tr --version
>>> tr (GNU coreutils) 8.31
>>>
>>> # autoconf --version
>>> autoconf (GNU Autoconf) 2.69
>>
>> Testing the above directly in bash works.
>> So after some testing I found a work-around:
>>
>> LC_ALL=en_US.UTF-8 make syntax-check
>>
>> The issue is seen on Fedora 30 and Arch Linux.
>>
>> Arch `locale` output:
>> LANG=en_US.UTF-8
>> LC_CTYPE="en_US.UTF-8"
>> LC_NUMERIC="en_US.UTF-8"
>> LC_TIME="en_US.UTF-8"
>> LC_COLLATE="en_US.UTF-8"
>> LC_MONETARY="en_US.UTF-8"
>> LC_MESSAGES="en_US.UTF-8"
>> LC_PAPER="en_US.UTF-8"
>> LC_NAME="en_US.UTF-8"
>> LC_ADDRESS="en_US.UTF-8"
>> LC_TELEPHONE="en_US.UTF-8"
>> LC_MEASUREMENT="en_US.UTF-8"
>> LC_IDENTIFICATION="en_US.UTF-8"
>> LC_ALL=
>>
>> The difference to a 'working' environment is that LC_ALL isn't set.
>>
>> So the outcome of 'make syntax-check' (concrete:
>> sc_prohibit_gnu_make_extensions rule) is locale dependent.
>>
>> Is that wanted behavior ?
>>
>> Fun fact: Once you run `LC_ALL=en_US.UTF-8 make syntax-check`, all
>> following 'make syntax-check' succeed while LC_ALL still is empty. This
>> is on Arch.
>>
>> I have no idea how to fix this properly.
> 
> Sorry, I have to backpaddle. After more tests with a fresh docker
> container it seems not LC_ALL related. Instead after several 'make
> syntax-check' those suddenly succeed.
> 
> All that changes my project directory is the files in '.deps/'. Removing
> that directory, I am back at failing 'make syntax-check' (after several
> invocations it then succeeds again).
> 
> It looks like the 'make' in the sc_prohibit_gnu_make_extensions rule
> creates .Plo and .Po files. One group of files for each SUBDIR/Makefile
> per 'make check'. If all files for all SUBDIRs have been created, make
> 'syntax-check' succeeds.
> 
> Knowing this, the issue is reproducible on Debian (unstable) as well.
> 
> I report more once tracked down.

Reproducible everywhere (needs gawk being installed, else the
sc_prohibit_gnu_make_extensions is a no-op).

Akim, at least with GNU make 4.2.1 the combination of -q and -p doesn't
do what you expect. From the make man page, I would say that both
options contradict. -q: don't print anything; -p: print the database

For now, I have to disable sc_prohibit_gnu_make_extensions in my projects.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

On 7/10/19 11:07 AM, Tim Rühsen wrote:
> On 6/25/19 3:24 PM, Tim Rühsen wrote:
>> Hi Akim,
>>
>> the command expands to
>>
>> if gawk --version | grep GNU >/dev/null 2>&1; then  \
>>   (cd . && autoconf --trace AC_CONFIG_FILES:'$1') | \
>> tr ' ' '\n' |   \
>> /usr/bin/sed -ne '/Makefile/{s/\.in$//;p;}' |   \
>> while read m; do\
>>   make -qp -f $m .DUMMY-TARGET 2>/dev/null |\
>> gawk -v file=$m -e 'BEGIN { RS = "\n\n"; in_rules = 0; } /^#
>> Files/ { in_rules = 1; } /\$> \.\w+(\.\w+)?:/ { print "Error: " file ": $< in a non implicit rule\n"
>> $0; status = 1; } END { exit status; }' || exit 1;\
>> done;   \
>> fi
>>
>> # make --version
>> GNU Make 4.2.1
>> Built for x86_64-redhat-linux-gnu
>>
>> # gawk --version
>> GNU Awk 4.2.1, API: 2.0 (GNU MPFR 3.1.6-p2, GNU MP 6.1.2)
>>
>> # sed --version
>> sed (GNU sed) 4.5
>>
>> # tr --version
>> tr (GNU coreutils) 8.31
>>
>> # autoconf --version
>> autoconf (GNU Autoconf) 2.69
> 
> Testing the above directly in bash works.
> So after some testing I found a work-around:
> 
> LC_ALL=en_US.UTF-8 make syntax-check
> 
> The issue is seen on Fedora 30 and Arch Linux.
> 
> Arch `locale` output:
> LANG=en_US.UTF-8
> LC_CTYPE="en_US.UTF-8"
> LC_NUMERIC="en_US.UTF-8"
> LC_TIME="en_US.UTF-8"
> LC_COLLATE="en_US.UTF-8"
> LC_MONETARY="en_US.UTF-8"
> LC_MESSAGES="en_US.UTF-8"
> LC_PAPER="en_US.UTF-8"
> LC_NAME="en_US.UTF-8"
> LC_ADDRESS="en_US.UTF-8"
> LC_TELEPHONE="en_US.UTF-8"
> LC_MEASUREMENT="en_US.UTF-8"
> LC_IDENTIFICATION="en_US.UTF-8"
> LC_ALL=
> 
> The difference to a 'working' environment is that LC_ALL isn't set.
> 
> So the outcome of 'make syntax-check' (concrete:
> sc_prohibit_gnu_make_extensions rule) is locale dependent.
> 
> Is that wanted behavior ?
> 
> Fun fact: Once you run `LC_ALL=en_US.UTF-8 make syntax-check`, all
> following 'make syntax-check' succeed while LC_ALL still is empty. This
> is on Arch.
> 
> I have no idea how to fix this properly.

Sorry, I have to backpaddle. After more tests with a fresh docker
container it seems not LC_ALL related. Instead after several 'make
syntax-check' those suddenly succeed.

All that changes my project directory is the files in '.deps/'. Removing
that directory, I am back at failing 'make syntax-check' (after several
invocations it then succeeds again).

It looks like the 'make' in the sc_prohibit_gnu_make_extensions rule
creates .Plo and .Po files. One group of files for each SUBDIR/Makefile
per 'make check'. If all files for all SUBDIRs have been created, make
'syntax-check' succeeds.

Knowing this, the issue is reproducible on Debian (unstable) as well.

I report more once tracked down.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

On 6/25/19 3:24 PM, Tim Rühsen wrote:
> Hi Akim,
> 
> the command expands to
> 
> if gawk --version | grep GNU >/dev/null 2>&1; then  \
>   (cd . && autoconf --trace AC_CONFIG_FILES:'$1') | \
> tr ' ' '\n' |   \
> /usr/bin/sed -ne '/Makefile/{s/\.in$//;p;}' |   \
> while read m; do\
>   make -qp -f $m .DUMMY-TARGET 2>/dev/null |\
> gawk -v file=$m -e 'BEGIN { RS = "\n\n"; in_rules = 0; } /^#
> Files/ { in_rules = 1; } /\$ \.\w+(\.\w+)?:/ { print "Error: " file ": $< in a non implicit rule\n"
> $0; status = 1; } END { exit status; }' || exit 1;\
> done;   \
> fi
> 
> # make --version
> GNU Make 4.2.1
> Built for x86_64-redhat-linux-gnu
> 
> # gawk --version
> GNU Awk 4.2.1, API: 2.0 (GNU MPFR 3.1.6-p2, GNU MP 6.1.2)
> 
> # sed --version
> sed (GNU sed) 4.5
> 
> # tr --version
> tr (GNU coreutils) 8.31
> 
> # autoconf --version
> autoconf (GNU Autoconf) 2.69

Testing the above directly in bash works.
So after some testing I found a work-around:

LC_ALL=en_US.UTF-8 make syntax-check

The issue is seen on Fedora 30 and Arch Linux.

Arch `locale` output:
LANG=en_US.UTF-8
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_PAPER="en_US.UTF-8"
LC_NAME="en_US.UTF-8"
LC_ADDRESS="en_US.UTF-8"
LC_TELEPHONE="en_US.UTF-8"
LC_MEASUREMENT="en_US.UTF-8"
LC_IDENTIFICATION="en_US.UTF-8"
LC_ALL=

The difference to a 'working' environment is that LC_ALL isn't set.

So the outcome of 'make syntax-check' (concrete:
sc_prohibit_gnu_make_extensions rule) is locale dependent.

Is that wanted behavior ?

Fun fact: Once you run `LC_ALL=en_US.UTF-8 make syntax-check`, all
following 'make syntax-check' succeed while LC_ALL still is empty. This
is on Arch.

I have no idea how to fix this properly.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

Here is a Dockerfile for reproducing the issue on Fedora 30.

docker build ...
docker run ...

git clone https://gitlab.com/gnuwget/wget2.git
cd wget2
git checkout tmp-stuff
./bootstrap
./configure
make syntax-check


Regards, Tim
FROM fedora:latest

LABEL maintainer "Tim Rühsen "

WORKDIR /usr/local

RUN dnf update -y
RUN dnf install -y --allowerasing \
git \
make \
gcc \
coreutils \
autoconf \
libtool \
gettext-devel \
automake \
autogen \
python \
valgrind \
libunistring-devel \
flex \
gnutls-devel \
libpsl-devel \
libnghttp2-devel \
zlib-devel \
libidn2-devel \
bzip2-devel \
xz-devel \
libmicrohttpd-devel \
gperf \
lzip \
rsync \
pandoc \
texinfo \
perl-IO-Socket-SSL \
perl-HTTP-Daemon \
gpgme-devel \
ccache \
which

RUN git clone https://git.savannah.gnu.org/git/gnulib.git
ENV GNULIB_SRCDIR /usr/local/gnulib


signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

Hi Bruno,

sorry, it doesn't.

On 6/25/19 4:37 PM, Bruno Haible wrote:
> Hi Tim,
> 
>> the command expands to
>>
>> if gawk --version | grep GNU >/dev/null 2>&1; then  \
>>   (cd . && autoconf --trace AC_CONFIG_FILES:'$1') | \
>> tr ' ' '\n' |   \
>> /usr/bin/sed -ne '/Makefile/{s/\.in$//;p;}' |   \
>> while read m; do\
>>   make -qp -f $m .DUMMY-TARGET 2>/dev/null |\
>> gawk -v file=$m -e 'BEGIN { RS = "\n\n"; in_rules = 0; } /^#
>> Files/ { in_rules = 1; } /\$> \.\w+(\.\w+)?:/ { print "Error: " file ": $< in a non implicit rule\n"
> 
> Does the attached patch fix it?
> 
> Bruno
> 



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

Hi Akim,

the command expands to

if gawk --version | grep GNU >/dev/null 2>&1; then  \
  (cd . && autoconf --trace AC_CONFIG_FILES:'$1') | \
tr ' ' '\n' |   \
/usr/bin/sed -ne '/Makefile/{s/\.in$//;p;}' |   \
while read m; do\
  make -qp -f $m .DUMMY-TARGET 2>/dev/null |\
gawk -v file=$m -e 'BEGIN { RS = "\n\n"; in_rules = 0; } /^#
Files/ { in_rules = 1; } /\$

signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

Hi Akim,

On 6/24/19 5:56 PM, Akim Demaille wrote:
> Hi Tim,
> 
>> Le 24 juin 2019 à 11:42, Tim Rühsen  a écrit :
>>
>> Hi Akim,
>>
>> Your proposal works here on Debian after I switched to mawk via
>> 'update-alternatives --config awk', selecting mawk. It detects two
>> occurrences of $< in our (project) Makefiles correctly.
>>
>> Switching back to the old maint.mk, brings back the error message about
>> '-e'. I just did this to make sure.
> 
> Perfect, thanks.  Installed.

I updated gnulib to your commit
(47405621b3066c035b4b98d2db934d550aaed1ad) and - sorry to say - two CI
runners broke. That are Arch Linux and Fedora 30.

Output on Fedora 30 from 'make syntax-check':

prohibit_gnu_make_extensions
Error: lib/Makefile: $< in a non implicit rule
# makefile (from 'lib/Makefile', line 1310)
NEXT_STRING_H = 
# makefile (from 'lib/Makefile', line 1409)
REPLACE_LOCALECONV = 0
# makefile (from 'lib/Makefile', line 1231)
LTLIBICONV =
# makefile (from 'lib/Makefile', line 476)
ETAGS = etags
# environment
halt =
# makefile (from 'lib/Makefile', line 1450)
REPLACE_PTSNAME = 0
# automatic
?F = $(notdir $?)
# makefile (from 'lib/Makefile', line 784)
GNULIB_STRTOD = 0
# makefile (from 'lib/Makefile', line 1410)
REPLACE_LOCALTIME = 0

... (many more lines like these) ...

MAKE_VERSION := 4.2.1
# makefile (from 'lib/Makefile', line 540)
GNULIB_ATOLL = 0
# variable set hash-table stats:
# Load=1368/2048=67%, Rehash=1, Collisions=6067/4772=127%
Error: lib/Makefile: $< in a non implicit rule
%.lo: %.c
#  recipe to execute (from 'lib/Makefile', line 2150):
$(AM_V_CC)depbase=`echo $@ | sed
's|[^/]*$$|$(DEPDIR)/&|;s|\.lo$$||'`;\
$(LTCOMPILE) -MT $@ -MD -MP -MF $$depbase.Tpo -c -o $@ $< &&\
$(am__mv) $$depbase.Tpo $$depbase.Plo
Error: lib/Makefile: $< in a non implicit rule
%.o: %.c
#  recipe to execute (from 'lib/Makefile', line 2134):
$(AM_V_CC)depbase=`echo $@ | sed
's|[^/]*$$|$(DEPDIR)/&|;s|\.o$$||'`;\
$(COMPILE) -MT $@ -MD -MP -MF $$depbase.Tpo -c -o $@ $< &&\
$(am__mv) $$depbase.Tpo $$depbase.Po
Error: lib/Makefile: $< in a non implicit rule
%.obj: %.c
#  recipe to execute (from 'lib/Makefile', line 2142):
$(AM_V_CC)depbase=`echo $@ | sed
's|[^/]*$$|$(DEPDIR)/&|;s|\.obj$$||'`;\
$(COMPILE) -MT $@ -MD -MP -MF $$depbase.Tpo -c -o $@
`$(CYGPATH_W) '$<'` &&\
$(am__mv) $$depbase.Tpo $$depbase.Po
Error: lib/Makefile: $< in a non implicit rule
(%): %
#  recipe to execute (built-in):
$(AR) $(ARFLAGS) $@ $<
Error: lib/Makefile: $< in a non implicit rule
%.out: %
#  recipe to execute (built-in):
@rm -f $@
 cp $< $@
Error: lib/Makefile: $< in a non implicit rule
%:: s.%
#  recipe to execute (built-in):
$(GET) $(GFLAGS) $(SCCS_OUTPUT_OPTION) $<
Error: lib/Makefile: $< in a non implicit rule
%:: SCCS/s.%
#  recipe to execute (built-in):
$(GET) $(GFLAGS) $(SCCS_OUTPUT_OPTION) $<
make: *** [maint.mk:439: sc_prohibit_gnu_make_extensions] Error 1



[root@61f4306ca485 wget2]# cd gnulib
[root@61f4306ca485 gnulib]# git branch
* (HEAD detached at 47405621b)
  master
[root@61f4306ca485 gnulib]# git log
commit 47405621b3066c035b4b98d2db934d550aaed1ad (HEAD, origin/master,
origin/HEAD, master)
Author: Akim Demaille 
Date:   Tue Jun 25 08:11:34 2019 +0200

[root@61f4306ca485 gnulib]# awk --version
GNU Awk 4.2.1, API: 2.0 (GNU MPFR 3.1.6-p2, GNU MP 6.1.2)


Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

Hi Akim,

On 6/22/19 5:53 PM, Akim Demaille wrote:
> Hi Tim,
> 
>> Le 17 juin 2019 à 12:04, Tim Rühsen  a écrit :
>>
>> Hi Akim,
>>
>> On 6/17/19 11:59 AM, Akim Demaille wrote:
>>> Hi Tim,
>>>
>>>> Le 17 juin 2019 à 11:57, Tim Rühsen  a écrit :
>>>>
>>>> Hi Akim,
>>>>
>>>> The patch uses awk -e which is understood only by GNU awk. This breaks
>>>> all Debian CI tests here since Debian installs 'mawk' by default (I
>>>> wasn't aware of that before).
>>>>
>>>> It's not a big deal to install package 'gawk' everywhere, but I just
>>>> wanted to mention it. I can see no warning/hint in any of the patch's
>>>> comments.
>>>
>>> That was not my intention.
>>>
>>> I expect that maintainers have gawk installed, so I think this check
>>> should be skipped if awk is not gawk.  WDYT?
>>
>> Thanks, sounds reasonable to me.
> 
> Here is my proposal.  Worked properly with the non GNU awk sitting on my 
> machine.

Your proposal works here on Debian after I switched to mawk via
'update-alternatives --config awk', selecting mawk. It detects two
occurrences of $< in our (project) Makefiles correctly.

Switching back to the old maint.mk, brings back the error message about
'-e'. I just did this to make sure.

Thanks for working on it !

Regards, Tim

> 
> commit b70c97f3ecf45a5c98b7f32189c1a4ae72a60788
> Author: Akim Demaille 
> Date:   Sat Jun 22 17:52:16 2019 +0200
> 
> maintainer-makefile: restore portability to non-GNU awks
> 
> Reported by Tim Rühsen.
> * top/maint.mk (AWK): New variable.  Use it.
> (sc_prohibit_gnu_make_extensions): Skip if $(AWK) is not gawk.
> 
> diff --git a/ChangeLog b/ChangeLog
> index cdc2b3d7a..dc7e52c0a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,10 @@
> +2019-06-22  Akim Demaille  
> +
> + maintainer-makefile: restore portability to non-GNU awks
> + Reported by Tim Rühsen.
> + * top/maint.mk (AWK): New variable.  Use it.
> + (sc_prohibit_gnu_make_extensions): Skip if $(AWK) is not gawk.
> +
>  2019-06-22  Akim Demaille  
>  
>   argmatch: put all the docs member last.
> diff --git a/top/maint.mk b/top/maint.mk
> index 3dbe9c378..16e936022 100644
> --- a/top/maint.mk
> +++ b/top/maint.mk
> @@ -24,6 +24,7 @@ ME := maint.mk
>  # These variables ought to be defined through the configure.ac section
>  # of the module description. But some packages import this file directly,
>  # ignoring the module description.
> +AWK ?= awk
>  GREP ?= grep
>  SED ?= sed
>  
> @@ -190,7 +191,7 @@ $(sc_z_rules_): %.z: %
>   @end=$$(date +%s.%N);   \
>   start=$$(cat .sc-start-$*); \
>   rm -f .sc-start-$*; \
> - awk -v s=$$start -v e=$$end \
> + $(AWK) -v s=$$start -v e=$$end  \
> 'END {printf "%.2f $(patsubst sc_%,%,$*)\n", e - s}' < /dev/null
>  
>  # The patsubst here is to replace each sc_% rule with its sc_%.z wrapper
> @@ -435,13 +436,15 @@ sc_prohibit_gnu_make_extensions_awk_ =  
> \
>   exit status;\
>}
>  sc_prohibit_gnu_make_extensions:
> - (cd $(srcdir) && autoconf --trace AC_CONFIG_FILES:'$$1') |  \
> -   tr ' ' '\n' | \
> -   $(SED) -ne '/Makefile/{s/\.in$$//;p;}' |  \
> -   while read m; do  \
> - $(MAKE) -qp -f $$m .DUMMY-TARGET 2>/dev/null |  \
> -   awk -v file=$$m -e '$($@_awk_)' || exit 1;\
> -   done
> + @if $(AWK) --version | grep GNU >/dev/null 2>&1; then   \
> +   (cd $(srcdir) && autoconf --trace AC_CONFIG_FILES:'$$1') |\
> + tr ' ' '\n' |   \
> + $(SED) -ne '/Makefile/{s/\.in$$//;p;}' |\
> + while read m; do\
> +   $(MAKE) -qp -f $$m .DUMMY-TARGET 2>/dev/null |\
> + $(AWK) -v file=$$m -e '$($@_awk_)' || exit 1;   \
> + done;   \
> + fi
>  
>  # Using EXIT_SUCCESS as the first argument to error is misleading,
>  # since when that parameter is 0, error does not exit.  Use '0' instead

Re: Alpine: useless-if-before-free: Exec format error

[Tim Rühsen]

On 6/23/19 6:32 PM, Jim Meyering wrote:
> On Wed, Jun 19, 2019 at 3:12 AM Bruno Haible  wrote:
> ...
>> Done like this:
>>
>> 2019-06-19  Bruno Haible  
>>
>> Reorder pieces of header in perl scripts.
>> The desired order is
>> - Prologue part 1 (2 lines with #!)
>> - Program short description
>> - Copyright and license notice
>> - Written-by notice
>> - Program short description (optional)
>> - Program long description (optional)
>> - Prologue part 2
>> - Time stamp
>> - Code
>> Reported by Paul Eggert.
>> * build-aux/announce-gen: Reorder header.
>> * build-aux/gitlog-to-changelog: Likewise.
>> * build-aux/useless-if-before-free: Likewise.
>> * build-aux/prefix-gnulib-mk: Add copyright notice and short
>> description.
>> * build-aux/update-copyright: Likewise. Add short description. Bump
>> time-stamp-line-limit to 200.
> 
> Thank you all for improving those scripts!
> 

I didn't expect this to escalate so much - and can only repeat what Jim
says, thank you for doing all this work !

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

Hi Akim,

On 6/17/19 11:59 AM, Akim Demaille wrote:
> Hi Tim,
> 
>> Le 17 juin 2019 à 11:57, Tim Rühsen  a écrit :
>>
>> Hi Akim,
>>
>> The patch uses awk -e which is understood only by GNU awk. This breaks
>> all Debian CI tests here since Debian installs 'mawk' by default (I
>> wasn't aware of that before).
>>
>> It's not a big deal to install package 'gawk' everywhere, but I just
>> wanted to mention it. I can see no warning/hint in any of the patch's
>> comments.
> 
> That was not my intention.
> 
> I expect that maintainers have gawk installed, so I think this check
> should be skipped if awk is not gawk.  WDYT?

Thanks, sounds reasonable to me.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: maintainer-makefile: catch uses of $< in non-implicit rules

[Tim Rühsen]

Hi Akim,

On 5/19/19 7:42 AM, Akim Demaille wrote:
> 
>> Le 18 mai 2019 à 20:21, Paul Eggert  a écrit :
>>
>> Akim Demaille wrote:
>>
>>> How about this?
>>
>> Looks OK to me; thanks.
> 
> Installed.  Thanks!
> 

The patch uses awk -e which is understood only by GNU awk. This breaks
all Debian CI tests here since Debian installs 'mawk' by default (I
wasn't aware of that before).

It's not a big deal to install package 'gawk' everywhere, but I just
wanted to mention it. I can see no warning/hint in any of the patch's
comments.
Hmmm, this also implies documenting a new dependency in all projects
that use gnulib or disabling sc_prohibit_gnu_make_extensions.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

socket module and sc_prohibit_always_true_header_tests

[Tim Rühsen]

Hi,

at GNU Wget we are currently a bit puzzled about gnulib-tool's pro-tip

#if HAVE_SYS_SOCKET_H
# include 
#elif HAVE_WS2TCPIP_H
# include 
#endif

and 'make syntax-check' complaining about using HAVE_SYS_SOCKET_H:

maint.mk: do not test the above HAVE__H symbol(s);
  with the corresponding gnulib module, they are always true
make: *** [maint.mk:825: sc_prohibit_always_true_header_tests] Fehler 1

So far we dealt with it by disabling
sc_prohibit_always_true_header_tests in cfg.mk, but it feels wrong and
we want to use that check general.

Other project suffer from this as well, e.g. GnuTLS
(https://gitlab.com/gnutls/gnutls/commit/a75d97033c839b0a764b50280b88b8fc6e4baa9f).

What is your advise ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Alpine: useless-if-before-free: Exec format error

[Tim Rühsen]

On 14.06.19 19:50, Paul Eggert wrote:
> On 6/14/19 10:04 AM, Tim Rühsen wrote:
>> Adding a shebang to build-aux/useless-if-before-free fixes the issue.
> 
> What shebang should that be, exactly? The current style is used in
> several scripts (build-aux/announce-gen, build-aux/gitlog-to-changelog,
> build-aux/prefix-gnulib-mk, build-aux/update-copyright,
> build-aux/useless-if-before-free) and presumably works in a wide variety
> of systems. Adding a shebang might break one of those systems unless
> it's done carefully.

I am not an expert in this area, just mentioned the shebang to give a
hint. Alpine / busybox is a pretty common system and it would be nice to
see 'make syntax-check' work there as well.
Do you think it makes more sense to open a bug at busybox then ?

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Alpine: useless-if-before-free: Exec format error

[Tim Rühsen]

Hi,

on Alpine, with pretty much latest gnulib commit
5905d8ca9945f0d60ff40eb6cfa42afc0199ab8f, 'make syntax-check' throws out

xargs: ./build-aux/useless-if-before-free: Exec format error

Adding a shebang to build-aux/useless-if-before-free fixes the issue.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: VPATH distcheck build fails

[Tim Rühsen]

Hi Bruno,

On 05.05.19 02:24, Bruno Haible wrote:
> Tim Rühsen wrote:
>> Nothing documented found. After reading some code, it seems po/Makevars
>> is left alone when removing AM_GNU_GETTEXT_VERSION([0.17]) from
>> configure.ac.
> 
> The Makefile.in.in from version 0.17 did not contain the support for
> the PACKAGE_GNU customization. You need at least
>   AM_GNU_GETTEXT_VERSION([0.19])
> for that.
> 
>> The wget.po created in the vpath build still contains just 'wget'
>> instead of 'GNU Wget' due to
>>
>> if LC_ALL=C grep 'GNU @PACKAGE@' $(top_srcdir)/* 2>/dev/null |
>> grep -v 'libtool:' >/dev/null; then \
>>   package_gnu='GNU '; \
>> else \
>>   package_gnu=''; \
>> fi; \
>>
>> That is, package_gnu is empty due to not finding 'GNU wget' in the main
>> project/tarball dir.
> 
> You are repeating yourself. I told you already that this heuristic is
> not reliable, and the way to avoid the heuristic is to add a po/Makevars
> that contains
>   PACKAGE_GNU = yes

Right, but as I said that finally ends in a loop, that's why I was
repeating that fact. Sorry, it wasn't meant to annoy you (or as any kind
of rant).

To break the "loop" it is indeed necessary to increase the gettext
version. Thanks for pointing this out.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: VPATH distcheck build fails

[Tim Rühsen]

On 04.05.19 18:13, Bruno Haible wrote:
> Tim Rühsen wrote:
>> Both, 'po/Makevars' and 'po/Makevars.template' are overwritten by
>> './bootstrap' (the gnulib bootstrap) (adding to git doesn't matter).
> 
> po/Makevars is meant to contain *package-specific* customizations
> of the gettext integration.
> 
> If 'bootstrap' overwrites it, surely it has a way to override this
> behaviour (in bootstrap.conf)?

Nothing documented found. After reading some code, it seems po/Makevars
is left alone when removing AM_GNU_GETTEXT_VERSION([0.17]) from
configure.ac. Good, but now I am back at the original reported error :-(

The wget.po created in the vpath build still contains just 'wget'
instead of 'GNU Wget' due to

if LC_ALL=C grep 'GNU @PACKAGE@' $(top_srcdir)/* 2>/dev/null |
grep -v 'libtool:' >/dev/null; then \
  package_gnu='GNU '; \
else \
  package_gnu=''; \
fi; \

That is, package_gnu is empty due to not finding 'GNU wget' in the main
project/tarball dir.

Of course I could just generate a file with such a signature, but it
feels like a hack. And I wonder how other GNU projects do it. GNU Wget2
has the same issue. GNU Libidn2 names itself just 'libidn2' everywhere -
so the 'cmp' works (by random).

Thank you for your patience and time, Bruno.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: VPATH distcheck build fails

[Tim Rühsen]

On 04.05.19 16:26, Tim Rühsen wrote:
> On 04.05.19 16:25, Bruno Haible wrote:
>> Tim Rühsen wrote:
>>> $ diff wget.po ../../../po/wget.pot
>>> 3c3
>>> < # This file is distributed under the same license as the wget package.
>>> ---
>>>> # This file is distributed under the same license as the GNU wget package.
>>> 9c9
>>> < "Project-Id-Version: wget 1.20.3.13-ee7fe-dirty\n"
>>> ---
>>>> "Project-Id-Version: GNU wget 1.20.3.13-ee7fe-dirty\n"
>>
>> The determination whether a package is a GNU package or not is a bit
>> unreliable. To fix this, set the PACKAGE_GNU variable in your
>> po/Makevars:
>>
>> # This tells whether or not to prepend "GNU " prefix to the package
>> # name that gets inserted into the header of the $(DOMAIN).pot file.
>> # Possible values are "yes", "no", or empty.  If it is empty, try to
>> # detect it automatically by scanning the files in $(top_srcdir) for
>> # "GNU packagename" string.
>> PACKAGE_GNU = yes
> 
> Thanks, Bruno.
> 
> That seems much better than changing AC_INIT in configure.ac.

Both, 'po/Makevars' and 'po/Makevars.template' are overwritten by
'./bootstrap' (the gnulib bootstrap) (adding to git doesn't matter).

We use gnulib module 'gettext-h' but not 'gettext', in configure.ac there is
AM_GNU_GETTEXT([external],[need-ngettext])
AM_GNU_GETTEXT_VERSION([0.17])

Sorry, but I'm a bit puzzled here.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

VPATH distcheck build fails

[Tim Rühsen]

Hi,

I am possible at the wrong ML now, but see the braking shell code as
well in gnulib/build-aux/po/Makefile.in.in.

The make target '$(DOMAIN).pot-update:' contains this code:
  if test -f $(srcdir)/$(DOMAIN).pot; then \
sed -f remove-potcdate.sed < $(srcdir)/$(DOMAIN).pot >
$(DOMAIN).1po && \
sed -f remove-potcdate.sed < $(DOMAIN).po > $(DOMAIN).2po && \
if cmp $(DOMAIN).1po $(DOMAIN).2po >/dev/null 2>&1; then \
  rm -f $(DOMAIN).1po $(DOMAIN).2po $(DOMAIN).po; \
else \
  rm -f $(DOMAIN).1po $(DOMAIN).2po $(srcdir)/$(DOMAIN).pot && \
  mv $(DOMAIN).po $(srcdir)/$(DOMAIN).pot; \
fi; \
  else \
mv $(DOMAIN).po $(srcdir)/$(DOMAIN).pot; \
  fi; \

In a vpath build (using the configure script outside the main project
directory), the line
  rm -f $(DOMAIN).1po $(DOMAIN).2po $(srcdir)/$(DOMAIN).pot
is triggered and thus breaks 'make distcheck'.

This due to all files in $(srcdir) are read-only.


What is the general advice from the experts here how to deal with it ?

Projects like GNU Wget fail on the first run, others need a second run
to trigger the error:

rm: cannot remove '../../../po/wget.pot': Permission denied
make[6]: *** [Makefile:223: wget.pot-update] Error 1
make[6]: Leaving directory
'/home/tim/src/wget1.x/vpath/wget-1.20.3.13-ee7fe-dirty/_build/sub/po'

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Add new macro gl_WARN_ADD_MULTIPLE

[Tim Rühsen]

On 5/2/19 10:16 AM, Bruno Haible wrote:
> Hi Alex,
> 
>> GNU Emacs spends about ~6s out of ~28s in its configuration process to
>> determine supported C compiler warnings.
> 
> It's good that you tackle this problem. Thanks!
> 
>> This macro first checks if all the supplied warnings are
>> supported, and then only checks them individually (via gl_WARN_ADD) if
>> that check fails.
> 
> So, the speed improvement exists only for the newest compilers; people
> who use a compiler that was released 1 or 2 years before the Emacs release
> will see no improvement.
> 
> How about a modified algorithm?
>   1) In a first pass, use the GCC's --help=warnings output to determine which
>  warnings are likely supported. Unfortunately, for clang, I don't know
>  of such a help option.
>   2) When the first check (with all the options) fails, split that list into
>  2 (or 3, or 4) chunks of nearly equal size, and perform the check
>  on each sub-list. And so on, recurse.
>  This way, you would still get some performance benefit if the compiler
>  supports 80% of the requested warnings but not all of them.
> 
> Finally, since there is no semantic difference between gl_WARN_ADD and
> gl_WARN_ADD_MULTIPLE, except that the latter allows multiple options, how
> about extending the gl_WARN_ADD macro (to allow multiple options) instead
> of defining a different macro (gl_WARN_ADD_MULTIPLE)?

Mentioned 2 years ago (but being still too busy to make a gnulib module
from it); Wget2's implementation using --help=warnings is here and FSF
copyrighted:

https://gitlab.com/gnuwget/wget2/blob/master/m4/wget_manywarnings.m4

And how to use / fine-tune it can be seen in L106 of

https://gitlab.com/gnuwget/wget2/blob/master/configure.ac

There is gcc and clang support. The configure.ac creates two sets of
compiler flags, for wget2/libwget sources and for gnulib sources.

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Tests on MinGW: undef ref pthread_mutex_lock/unlock

[Tim Rühsen]

On 12.04.19 02:26, Bruno Haible wrote:
> Hi Tim,
> 
> The points to look at are:
>   - How does the configure script behave while running the expanded code
> that comes from threadlib.m4? Try "sh -x ./configure ..." for this
> exercise.
>   - Why does gl_lock_unlock expand to a call to pthread_mutex_unlock? Only
> if USE_POSIX_THREADS is defined. And where does USE_POSIX_THREADS come
> from on your system?

Well, stderr and stdout seems to mixed up pretty much when using
./configure ... >out 2>&1. So all I can see is that USE_POSIX_THREADS is
being set. [indented code is from threadlib.m4]


if test "$gl_use_threads" = yes || test "$gl_use_threads" = posix; then

+ test -n yes
+ gl_threads_api=posix

  AC_CHECK_HEADER([pthread.h],
[gl_have_pthread_h=yes], [gl_have_pthread_h=no])

+ gl_have_pthread=yes

  if test "$gl_have_pthread_h" = yes; then


if test -n "$gl_have_pthread"; then
  gl_threads_api=posix
  AC_DEFINE([USE_POSIX_THREADS], [1],

+ test -n yes
+ gl_threads_api=posix
+ printf '%s\n' '#define USE_POSIX_THREADS 1'


Little bit further above in the configure output:
+ gl_have_pthread=yes
+ LIBTHREAD=-pthread
+ LTLIBTHREAD=-pthread
+ LIBMULTITHREAD=-pthread
+ LTLIBMULTITHREAD=-pthread
+ rm -f core conftest.err conftest.o conftest.exe conftest.c
+ LIBS=
+ test -n yes
+ break
+ test -n yes
+ test -z -pthread
+ test -z yes
+ test -n yes
+ gl_threads_api=posix
+ printf '%s\n' '#define USE_POSIX_THREADS 1'
+ test -n -pthread
+ case "$gl_cv_have_weak" in


The output is pretty large. If interested, I'll send it to you via PM.

>> In file included from /usr/share/mingw-w64/include/signal.h:10,
>>  from ./signal.h:52,
>>  from pthread_sigmask.c:20:
>> pthread_sigmask.c:34:1: error: expected identifier or ‘(’ before numeric
>> constant
>>  pthread_sigmask (int how, const sigset_t *new_mask, sigset_t *old_mask)
>>  ^~~
>> make[4]: *** [Makefile:10495: pthread_sigmask.o] Fehler 1
> 
> This one, you can start with a "gcc -E" command, to see where a macro
> expansion of pthread_sigmask come from.

As Eric says, pthread_sigmask is defined to 0 in
/usr/share/mingw-w64/include/pthread_signal.h:

/* Windows has rudimentary signals support.  */
#define pthread_sigmask(H, S1, S2) 0

> Btw, I never build with mingw-pthreads installed. But even with mingw-pthreads
> installed, gnulib modules ought to not use it; they ought to use the Windows
> API instead.
> 
> Bruno
> 
> 

Regards, Tim



signature.asc
Description: OpenPGP digital signature

Re: Tests on MinGW: undef ref pthread_mutex_lock/unlock

[Tim Rühsen]

On 11.04.19 20:56, Bruno Haible wrote:
> Hi Tim,
> 
>>> For analyzing this, we would need the generated
>>>   - config.log,
>>>   - config.status,
>>>   - config.h.
>>
>> Thanks, attached.
> 
> You should also say which package you are building, and with which
> configure options you configured it.
> 
> It looks like
>   - you are building gnutls-3.6.7, not just a gnulib testdir,
>   - you gave the configure option --disable-threads,
>   - but at the same time the C macro USE_POSIX_THREADS is defined.
> How come?

Maybe Debian is currently broken regarding MinGW ? I never had issues
building libunistring with MinGW, but now compilation stops with

In file included from /usr/share/mingw-w64/include/signal.h:10,
 from ./signal.h:52,
 from pthread_sigmask.c:20:
pthread_sigmask.c:34:1: error: expected identifier or ‘(’ before numeric
constant
 pthread_sigmask (int how, const sigset_t *new_mask, sigset_t *old_mask)
 ^~~
make[4]: *** [Makefile:10495: pthread_sigmask.o] Fehler 1

Maybe this is related, all about pthreads.

Regards, Tim



signature.asc
Description: OpenPGP digital signature