Re: sort dynamic linking overhead

2024-02-27 Thread Bruno Haible
Pádraig Brady wrote:
> > Does this work for all the various names of libcrypto in various distros?
> > 
> > Debian 12   libcrypto.so.3
> > Ubuntu 22.04libcrypto.so.1.1 libcrypto.so.3
> > Slackware 15libcrypto.so.1.1
> > openSUSE 15.5   libcrypto.so.1.1
> > CentOS Stream 9 libcrypto.so.3
> > Guix 1.4libcrypto.so.1.1
> > Alpine 3.19 libcrypto.so.3
> > FreeBSD 14.0libcrypto.so.38
> > NetBSD 9.3  libcrypto.so.14
> > OpenBSD 7.4 libcrypto.so.52.0
> > 
> 
> I only tested with libcrypto.so.3, but it should match all of the above.
> It matches libcrypto.so.[.0-9]*

Here are my testing results (with the LIB_DL fix):

* On some machines, I had to install the packages with the 
  header files first:

  - Debian 12:
# apt install libssl-dev
  - openSUSE 15.5:
YaST software > install libopenssl-3-devel
  - Slackware 15:
Download and unpack the openssl-1.1.1w binary packages
  - Alpine Linux 3.19:
# apk add openssl openssl-dev

* On some platforms, the configure test gave

 checking whether openssl is GPL compatible... no

  due to the  header files being absent:

- Guix 1.4
- macOS 12.5
- Cygwin 2.9.0

* On some platforms, the configure test gave

 checking whether openssl is GPL compatible... no

  because the openssl version is not >= 3.

- Slackware 15 (has libcrypto.so.1.1)
- NetBSD 9.3 (has libcrypto.so.14)
- OpenBSD 7.4 (has libcrypto.so.52.0)
- Solaris 11.4 (has libcrypto.so.1.0.0)

* On these platforms, the configure test gave

 checking whether openssl is GPL compatible... yes

  and LIBCRYPTO_SONAME got defined.

- Debian 12
- Ubuntu 22.04
- openSUSE 15.5
- CentOS Stream 9
- Alpine Linux 3.19
- FreeBSD 14.0
- Android

  The value of LIBCRYPTO_SONAME is
"libcrypto.so.30" on Android,
"libcrypto.so.3" on the other platforms.

Bruno






Re: sort dynamic linking overhead

2024-02-27 Thread Bruno Haible
Paul Eggert wrote:
> Thanks for the patch. I was hoping that we didn't need to worry about 
> older platforms needing -ldl. Oh well.

Distros with glibc < 2.34 include:
  Debian 11
  CentOS 8 stream
  Fedora 34
  openSUSE 15.5
  Slackware 15
These are not obsolete, so far.

> The patch causes 'configure' to search for dlopen even when there's no 
> crypto library. 'configure' could instead use AC_SEARCH_LIBS only if the 
> AC_LINK_IFELSE fails (or simply put AC_LINK_ELSE in an 'for LIB_DL in "" 
> -ldl' loop). But perhaps it's better to leave things be, in case we ever 
> need dlopen for something else.

If we were to start optimizing configure.ac scripts like this, they would
quickly become hard to maintain, because here and there we would be
accessing a shell variable that has not been initialized (because the
initialization was in an 'if' branch). (There's a reason why AC_REQUIRE
has been invented...)

The configure tests which nag me the most are those which take one second
of time or more: the fcntl test, the sleep test, the strstr test, and a few
others. If someone is starting to optimize, here would be the starting point :)

> Also, if I understand things correctly, with this patch it's 
> theoretically possible to pass -ldl to gcc even when 'sort' doesn't do 
> dynamic linking, and we could complicate configure.ac further to omit 
> -ldl in that case. But I doubt whether it's worth worrying about this.

Correct, but this applies only to glibc and Android systems, and on these
systems configure finds

  checking for C compiler flag to ignore unused libraries... -Wl,--as-needed

So, if $(LIB_DL) is -ldl and this linker option gets used for 'sort', but
'sort' does not use dlopen(), the linker will eliminate it.

Bruno






Re: sort dynamic linking overhead

2024-02-27 Thread Paul Eggert
Thanks for the patch. I was hoping that we didn't need to worry about 
older platforms needing -ldl. Oh well.


The patch causes 'configure' to search for dlopen even when there's no 
crypto library. 'configure' could instead use AC_SEARCH_LIBS only if the 
AC_LINK_IFELSE fails (or simply put AC_LINK_ELSE in an 'for LIB_DL in "" 
-ldl' loop). But perhaps it's better to leave things be, in case we ever 
need dlopen for something else.


Also, if I understand things correctly, with this patch it's 
theoretically possible to pass -ldl to gcc even when 'sort' doesn't do 
dynamic linking, and we could complicate configure.ac further to omit 
-ldl in that case. But I doubt whether it's worth worrying about this.




Re: [PATCH] mktemp: cut the output info when suffix contains XXX.

2024-02-27 Thread Pádraig Brady

On 27/02/2024 01:41, lvgenggeng wrote:

* src/mktemp.c: When --suffix was supply in cmdline, the template
points to the meraged buffer(dest_name). As X is counted from
template which not contains suffix, So the output should be cut.

Signed-off-by: lvgenggeng 
---
  src/mktemp.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/mktemp.c b/src/mktemp.c
index 5f8e5155d..d194aeff5 100644
--- a/src/mktemp.c
+++ b/src/mktemp.c
@@ -156,7 +156,7 @@ main (int argc, char **argv)
int status = EXIT_SUCCESS;
size_t x_count;
size_t suffix_len;
-  char *dest_name;
+  char *dest_name = nullptr;
  
initialize_main (, );

set_program_name (argv[0]);
@@ -255,7 +255,12 @@ main (int argc, char **argv)
  }
x_count = count_consecutive_X_s (template, suffix - template);
if (x_count < 3)
-error (EXIT_FAILURE, 0, _("too few X's in template %s"), quote (template));
+{
+  /* when suffix was appended to template, template should be cut. */
+  if (template == dest_name)
+template[suffix - template] = '\0';
+  error (EXIT_FAILURE, 0, _("too few X's in template %s"), quote 
(template));
+}
  
if (use_dest_dir)

  {


Right. That would be especially confusing with:

  $ mktemp -u --suffix=XX template.X
  mktemp: too few X's in template ‘template.XXX’

I adjusted the mktemp test, added a NEWS entry and pushed:
https://github.com/coreutils/coreutils/commit/e397ba1a3

thanks!
Pádraig.



bug#69418: test failure when no french locale is installed

2024-02-27 Thread Pádraig Brady

On 26/02/2024 23:05, Bruno Haible wrote:

Testing the current coreutils git master:

On a Debian 12 system, in which I have not installed a French UTF-8 locale,
I see a test failure of tests/misc/join-utf8.

The essential lines from test-suite.log:

+ test set = set
+ LC_ALL=none
../tests/misc/join-utf8.sh: line 24: warning: setlocale: LC_ALL: cannot change 
locale (none): No such file or directory

The cause is that on such a system, LOCALE_FR_UTF8, as determined by
gnulib/m4/locale-fr.m4, is 'none', not empty or absent.

The attached patch fixes the failure.



Applied.

This also indicates that gnulib ensures that LOCALE_FR_UTF8
is set to "none" when not present or usable,
so I've made that simplification in other tests now.

Marking this as done.

thanks!
Pádraig.





Re: sort dynamic linking overhead

2024-02-27 Thread Pádraig Brady

On 27/02/2024 11:15, Bruno Haible wrote:

Paul Eggert wrote:

I installed the
attached patch into coreutils. With it, 'sort -R' continues to use MD5
but on GNUish platforms 'sort' links libcrypto dynamically only if -R is
used (Bruno's suggestion). This doesn't significantly affect 'sort -R'
performance, and reduces the startup overhead of plain 'sort'


The patch has no effect on openSUSE 15.5: DLOPEN_LIBCRYPTO is not defined
in config.h.

config.log shows a link error of the test program:
   undefined reference to symbol 'dlopen@@GLIBC_2.2.5'
Both the test program and 'sort' need to link with -ldl.

This is generally true on the following platforms:
   - glibc < 2.34,
   - Android (with some compilers, not with Termux (*)).

(*) In the Android Termux environment, the compiler is configured to pass
the options '-ldl -lc', rather than just '-lc', to the linker. Thus,
we don't need to pass '-ldl' to the compiler. But the 'sort' program will
be linked with -ldl. In other Android environments, things may be different,
though.

The proposed attached patch fixes the problem: It defines a variable LIB_DL
that contains '-ldl' where needed or '' if not needed, and uses it with
the test program and with 'sort'.

You might think that this patch is no win, because it trades one link
dependency for another link dependency? But that's not what it does:
'ldd' shows that without the patch, 'sort' loads the libraries

   linux-vdso.so.1
   libcrypto.so.3
   libpthread.so.0
   libc.so.6
   libz.so.1
   libdl.so.2
   /lib64/ld-linux-x86-64.so.2

and with the patch, 'sort' loads the libraries

   linux-vdso.so.1
   libdl.so.2
   libpthread.so.0
   libc.so.6
   /lib64/ld-linux-x86-64.so.2

— that is, libdl.so.2 is getting loaded anyway.


Applied.

thanks,
Pádraig



Re: sort dynamic linking overhead

2024-02-27 Thread Bruno Haible
Paul Eggert wrote:
> I installed the 
> attached patch into coreutils. With it, 'sort -R' continues to use MD5 
> but on GNUish platforms 'sort' links libcrypto dynamically only if -R is 
> used (Bruno's suggestion). This doesn't significantly affect 'sort -R' 
> performance, and reduces the startup overhead of plain 'sort'

The patch has no effect on openSUSE 15.5: DLOPEN_LIBCRYPTO is not defined
in config.h.

config.log shows a link error of the test program:
  undefined reference to symbol 'dlopen@@GLIBC_2.2.5'
Both the test program and 'sort' need to link with -ldl.

This is generally true on the following platforms:
  - glibc < 2.34,
  - Android (with some compilers, not with Termux (*)).

(*) In the Android Termux environment, the compiler is configured to pass
the options '-ldl -lc', rather than just '-lc', to the linker. Thus,
we don't need to pass '-ldl' to the compiler. But the 'sort' program will
be linked with -ldl. In other Android environments, things may be different,
though.

The proposed attached patch fixes the problem: It defines a variable LIB_DL
that contains '-ldl' where needed or '' if not needed, and uses it with
the test program and with 'sort'.

You might think that this patch is no win, because it trades one link
dependency for another link dependency? But that's not what it does:
'ldd' shows that without the patch, 'sort' loads the libraries

  linux-vdso.so.1
  libcrypto.so.3
  libpthread.so.0
  libc.so.6
  libz.so.1
  libdl.so.2
  /lib64/ld-linux-x86-64.so.2

and with the patch, 'sort' loads the libraries

  linux-vdso.so.1
  libdl.so.2
  libpthread.so.0
  libc.so.6
  /lib64/ld-linux-x86-64.so.2

— that is, libdl.so.2 is getting loaded anyway.

>From c30a0e55c95e0ae7062ee2ececf85cd0dbfe49fb Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Tue, 27 Feb 2024 12:12:59 +0100
Subject: [PATCH] sort: Make the startup time optimization effective on glibc <
 2.34

* configure.ac: Test where to find the dlopen function. Set LIB_DL.
Use it in the DLOPEN_LIBCRYPTO test.
* src/local.mk (src_sort_LDADD): Add $(LIB_DL).
---
 configure.ac | 11 ++-
 src/local.mk |  2 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index fe8408a06..248e30ca2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -351,6 +351,15 @@ if test $utils_cv_localtime_cache = yes; then
   AC_DEFINE([LOCALTIME_CACHE], [1], [FIXME])
 fi
 
+# Find the library for dynamic loading of shared libraries.
+AC_SEARCH_LIBS([dlopen], [dl])
+AS_CASE([$ac_cv_search_dlopen],
+  [no | 'none required'],
+[LIB_DL=],
+  [*],
+[LIB_DL="$ac_cv_search_dlopen"])
+AC_SUBST([LIB_DL])
+
 # Should 'sort' link libcrypto dynamically?
 AS_CASE([$LIB_CRYPTO],
   [-lcrypto],
@@ -360,7 +369,7 @@ AS_CASE([$LIB_CRYPTO],
[utils_cv_dlopen_libcrypto],
[utils_cv_dlopen_libcrypto=no
 saved_LIBS=$LIBS
-LIBS="$LIBS $LIB_CRYPTO"
+LIBS="$LIBS $LIB_DL $LIB_CRYPTO"
 AC_LINK_IFELSE(
   [AC_LANG_PROGRAM(
  [[#include 
diff --git a/src/local.mk b/src/local.mk
index 7bc5ba5bc..96ee941ca 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -304,7 +304,7 @@ src_printf_LDADD += $(LIBICONV)
 
 # for libcrypto hash routines
 src_md5sum_LDADD += $(LIB_CRYPTO)
-src_sort_LDADD += $(LIB_CRYPTO)
+src_sort_LDADD += $(LIB_DL) $(LIB_CRYPTO)
 src_sha1sum_LDADD += $(LIB_CRYPTO)
 src_sha224sum_LDADD += $(LIB_CRYPTO)
 src_sha256sum_LDADD += $(LIB_CRYPTO)
-- 
2.34.1