bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* scratch_buffer.h, scratch_buffer_dupfree.c sync
@ 2022-11-02 22:37 Karl Berry
  2022-11-03  1:18 ` Paul Eggert
  0 siblings, 1 reply; 13+ messages in thread
From: Karl Berry @ 2022-11-02 22:37 UTC (permalink / raw)
  To: bug-gnulib

1) LIBC/include/scratch_buffer.h has introduced some substantial changes
over GNULIB/lib/malloc/scratch_buffer.h. I'm not sure if it is safe
to sync them any more. Especially because:

2) LIBC/nalloc/scratch_buffer_dupfree.c no longer exists.  There is no
such file in libc any more.

Paul, anyone, please advise. -k


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-02 22:37 scratch_buffer.h, scratch_buffer_dupfree.c sync Karl Berry
@ 2022-11-03  1:18 ` Paul Eggert
  2022-11-03  2:37   ` Bruno Haible
  0 siblings, 1 reply; 13+ messages in thread
From: Paul Eggert @ 2022-11-03  1:18 UTC (permalink / raw)
  To: Karl Berry; +Cc: Gnulib bugs

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]

On 11/2/22 15:37, Karl Berry wrote:
> 1) LIBC/include/scratch_buffer.h has introduced some substantial changes
> over GNULIB/lib/malloc/scratch_buffer.h. I'm not sure if it is safe
> to sync them any more. Especially because:
> 
> 2) LIBC/nalloc/scratch_buffer_dupfree.c no longer exists.  There is no
> such file in libc any more.

Yes, it's a nontrivial merge. However, I think we can still sync 
LIBC/include/scratch_buffer.h. I installed the attached patch; please 
let me know of any further problems in this area.

[-- Attachment #2: 0001-scratch_buffer-adjust-to-glibc-changes.patch --]
[-- Type: text/x-patch, Size: 26434 bytes --]

From ddfcbc95a6be3ddc588a93f21edb69cc7c214d9c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 2 Nov 2022 18:14:15 -0700
Subject: [PATCH] scratch_buffer: adjust to glibc changes

Problem reported by Karl Berry in:
https://lists.gnu.org/r/bug-gnulib/2022-11/msg00004.html
* build-aux/depcomp, doc/make-stds.texi, lib/malloc/scratch_buffer.h:
Autoupdate.
* build-aux/install-reloc (func_create_wrapper):
Omit removed file scratch_buffer_dupfree.c.
* config/srclist.txt: Remove lib/malloc/scratch_buffer_dupfree.c
* lib/canonicalize-lgpl.c: Merge changes from glibc through its
commit ef0700004bf0dccf493a5e8e21f71d9e7972ea9f dated 2022-07-05
11:04:45 +0200.
(__strdup) [!_LIBC]: New macro.
(struct realpath_bufs): New type.
(realpath_stk): Use it as the extra argument.  All uses changed.
No longer any need for noinline or GCC_BOGUS_WRETURN_LOCAL_ADDR.
* lib/canonicalize.c (struct realpath_bufs)
(canonicalize_filename_mode_stk): Likewise.
* lib/malloc/scratch_buffer_dupfree.c:
Remove, since it was removed in glibc.
* lib/scratch_buffer.h (scratch_buffer_dupfree) [0]:
(__libc_scratch_buffer_dupfree): Remove decls.
* modules/relocatable-prog-wrapper (Files):
* modules/scratch_buffer (Files, lib_SOURCES):
Remove lib/malloc/scratch_buffer_dupfree.c.
---
 ChangeLog                           |  27 ++++++
 build-aux/depcomp                   |   4 +-
 build-aux/install-reloc             |   2 -
 config/srclist.txt                  |   1 -
 doc/make-stds.texi                  |   4 +-
 lib/canonicalize-lgpl.c             | 122 ++++++++++++++--------------
 lib/canonicalize.c                  |  91 +++++++++------------
 lib/malloc/scratch_buffer.h         |  16 ----
 lib/malloc/scratch_buffer_dupfree.c |  41 ----------
 lib/scratch_buffer.h                |  10 ---
 modules/relocatable-prog-wrapper    |   1 -
 modules/scratch_buffer              |   4 +-
 12 files changed, 129 insertions(+), 194 deletions(-)
 delete mode 100644 lib/malloc/scratch_buffer_dupfree.c

diff --git a/ChangeLog b/ChangeLog
index 3dc7d8966f..0defe2469e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+2022-11-02  Paul Eggert  <eggert@cs.ucla.edu>
+
+	scratch_buffer: adjust to glibc changes
+	Problem reported by Karl Berry in:
+	https://lists.gnu.org/r/bug-gnulib/2022-11/msg00004.html
+	* build-aux/depcomp, doc/make-stds.texi, lib/malloc/scratch_buffer.h:
+	Autoupdate.
+	* build-aux/install-reloc (func_create_wrapper):
+	Omit removed file scratch_buffer_dupfree.c.
+	* config/srclist.txt: Remove lib/malloc/scratch_buffer_dupfree.c
+	* lib/canonicalize-lgpl.c: Merge changes from glibc through its
+	commit ef0700004bf0dccf493a5e8e21f71d9e7972ea9f dated 2022-07-05
+	11:04:45 +0200.
+	(__strdup) [!_LIBC]: New macro.
+	(struct realpath_bufs): New type.
+	(realpath_stk): Use it as the extra argument.  All uses changed.
+	No longer any need for noinline or GCC_BOGUS_WRETURN_LOCAL_ADDR.
+	* lib/canonicalize.c (struct realpath_bufs)
+	(canonicalize_filename_mode_stk): Likewise.
+	* lib/malloc/scratch_buffer_dupfree.c:
+	Remove, since it was removed in glibc.
+	* lib/scratch_buffer.h (scratch_buffer_dupfree) [0]:
+	(__libc_scratch_buffer_dupfree): Remove decls.
+	* modules/relocatable-prog-wrapper (Files):
+	* modules/scratch_buffer (Files, lib_SOURCES):
+	Remove lib/malloc/scratch_buffer_dupfree.c.
+
 2022-11-01  Bruno Haible  <bruno@clisp.org>
 
 	relocatable-script: Relax license.
diff --git a/build-aux/depcomp b/build-aux/depcomp
index 75323b7392..703eed2750 100755
--- a/build-aux/depcomp
+++ b/build-aux/depcomp
@@ -1,7 +1,7 @@
 #! /bin/sh
 # depcomp - compile a program generating dependencies as side-effects
 
-scriptversion=2018-03-07.03; # UTC
+scriptversion=2022-09-18.14; # UTC
 
 # Copyright (C) 1999-2022 Free Software Foundation, Inc.
 
@@ -197,7 +197,7 @@ gcc3)
   ;;
 
 gcc)
-## Note that this doesn't just cater to obsosete pre-3.x GCC compilers.
+## Note that this doesn't just cater to obsolete pre-3.x GCC compilers.
 ## but also to in-use compilers like IMB xlc/xlC and the HP C compiler.
 ## (see the conditional assignment to $gccflag above).
 ## There are various ways to get dependency output from gcc.  Here's
diff --git a/build-aux/install-reloc b/build-aux/install-reloc
index 63aeb9af1c..22739681ae 100755
--- a/build-aux/install-reloc
+++ b/build-aux/install-reloc
@@ -237,7 +237,6 @@ func_create_wrapper ()
                "$srcdir"/readlink.c \
                "$srcdir"/stat.c \
                "$srcdir"/canonicalize-lgpl.c \
-               "$srcdir"/malloc/scratch_buffer_dupfree.c \
                "$srcdir"/malloc/scratch_buffer_grow.c \
                "$srcdir"/malloc/scratch_buffer_grow_preserve.c \
                "$srcdir"/malloc/scratch_buffer_set_array_size.c \
@@ -263,7 +262,6 @@ func_create_wrapper ()
         readlink.o \
         stat.o \
         canonicalize-lgpl.o \
-        scratch_buffer_dupfree.o \
         scratch_buffer_grow.o \
         scratch_buffer_grow_preserve.o \
         scratch_buffer_set_array_size.o \
diff --git a/config/srclist.txt b/config/srclist.txt
index b4e36845be..bff6a02d8e 100644
--- a/config/srclist.txt
+++ b/config/srclist.txt
@@ -59,7 +59,6 @@ $LIBCSRC include/filename.h		lib
 #$LIBCSRC malloc/dynarray_resize.c	lib/malloc
 #$LIBCSRC malloc/dynarray_resize_clear.c	lib/malloc
 $LIBCSRC include/scratch_buffer.h	lib/malloc
-$LIBCSRC malloc/scratch_buffer_dupfree.c	lib/malloc
 $LIBCSRC malloc/scratch_buffer_grow.c	lib/malloc
 $LIBCSRC malloc/scratch_buffer_grow_preserve.c	lib/malloc
 $LIBCSRC malloc/scratch_buffer_set_array_size.c	lib/malloc
diff --git a/doc/make-stds.texi b/doc/make-stds.texi
index b0745a8a3e..132476c7a3 100644
--- a/doc/make-stds.texi
+++ b/doc/make-stds.texi
@@ -160,7 +160,7 @@ installation should not use any utilities directly except these:
 @c mkfifo mknod tee uname
 
 @example
-awk cat cmp cp diff echo egrep expr false grep install-info ln ls
+awk cat cmp cp diff echo expr false grep install-info ln ls
 mkdir mv printf pwd rm rmdir sed sleep sort tar test touch tr true
 @end example
 
@@ -1135,7 +1135,7 @@ programs except for these:
 
 @example
 [ basename bash cat chgrp chmod chown cmp cp dd diff echo
-egrep expand expr false fgrep find getopt grep gunzip gzip
+expand expr false find getopt grep gunzip gzip
 hostname install install-info kill ldconfig ln ls md5sum
 mkdir mkfifo mknod mv printenv pwd rm rmdir sed sort tee
 test touch true uname xargs yes
diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c
index 8c3d7f7cf8..870a663505 100644
--- a/lib/canonicalize-lgpl.c
+++ b/lib/canonicalize-lgpl.c
@@ -47,6 +47,7 @@
 #else
 # define __canonicalize_file_name canonicalize_file_name
 # define __realpath realpath
+# define __strdup strdup
 # include "pathmax.h"
 # define __faccessat faccessat
 # if defined _WIN32 && !defined __CYGWIN__
@@ -179,27 +180,16 @@ get_path_max (void)
   return path_max < 0 ? 1024 : path_max <= IDX_MAX ? path_max : IDX_MAX;
 }
 
-/* Act like __realpath (see below), with an additional argument
-   rname_buf that can be used as temporary storage.
-
-   If GCC_LINT is defined, do not inline this function with GCC 10.1
-   and later, to avoid creating a pointer to the stack that GCC
-   -Wreturn-local-addr incorrectly complains about.  See:
-   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644
-   Although the noinline attribute can hurt performance a bit, no better way
-   to pacify GCC is known; even an explicit #pragma does not pacify GCC.
-   When the GCC bug is fixed this workaround should be limited to the
-   broken GCC versions.  */
-# if __GNUC_PREREQ (10, 1)
-#  if defined GCC_LINT || defined lint
-__attribute__ ((__noinline__))
-#  elif __OPTIMIZE__ && !__NO_INLINE__
-#   define GCC_BOGUS_WRETURN_LOCAL_ADDR
-#  endif
-# endif
+/* Scratch buffers used by realpath_stk and managed by __realpath.  */
+struct realpath_bufs
+{
+  struct scratch_buffer rname;
+  struct scratch_buffer extra;
+  struct scratch_buffer link;
+};
+
 static char *
-realpath_stk (const char *name, char *resolved,
-              struct scratch_buffer *rname_buf)
+realpath_stk (const char *name, char *resolved, struct realpath_bufs *bufs)
 {
   char *dest;
   char const *start;
@@ -224,12 +214,7 @@ realpath_stk (const char *name, char *resolved,
       return NULL;
     }
 
-  struct scratch_buffer extra_buffer, link_buffer;
-  scratch_buffer_init (&extra_buffer);
-  scratch_buffer_init (&link_buffer);
-  scratch_buffer_init (rname_buf);
-  char *rname_on_stack = rname_buf->data;
-  char *rname = rname_on_stack;
+  char *rname = bufs->rname.data;
   bool end_in_extra_buffer = false;
   bool failed = true;
 
@@ -239,16 +224,16 @@ realpath_stk (const char *name, char *resolved,
 
   if (!IS_ABSOLUTE_FILE_NAME (name))
     {
-      while (!__getcwd (rname, rname_buf->length))
+      while (!__getcwd (bufs->rname.data, bufs->rname.length))
         {
           if (errno != ERANGE)
             {
               dest = rname;
               goto error;
             }
-          if (!scratch_buffer_grow (rname_buf))
-            goto error_nomem;
-          rname = rname_buf->data;
+          if (!scratch_buffer_grow (&bufs->rname))
+            return NULL;
+          rname = bufs->rname.data;
         }
       dest = __rawmemchr (rname, '\0');
       start = name;
@@ -302,13 +287,13 @@ realpath_stk (const char *name, char *resolved,
           if (!ISSLASH (dest[-1]))
             *dest++ = '/';
 
-          while (rname + rname_buf->length - dest
+          while (rname + bufs->rname.length - dest
                  < startlen + sizeof dir_suffix)
             {
               idx_t dest_offset = dest - rname;
-              if (!scratch_buffer_grow_preserve (rname_buf))
-                goto error_nomem;
-              rname = rname_buf->data;
+              if (!scratch_buffer_grow_preserve (&bufs->rname))
+                return NULL;
+              rname = bufs->rname.data;
               dest = rname + dest_offset;
             }
 
@@ -319,13 +304,13 @@ realpath_stk (const char *name, char *resolved,
           ssize_t n;
           while (true)
             {
-              buf = link_buffer.data;
-              idx_t bufsize = link_buffer.length;
+              buf = bufs->link.data;
+              idx_t bufsize = bufs->link.length;
               n = __readlink (rname, buf, bufsize - 1);
               if (n < bufsize - 1)
                 break;
-              if (!scratch_buffer_grow (&link_buffer))
-                goto error_nomem;
+              if (!scratch_buffer_grow (&bufs->link))
+                return NULL;
             }
           if (0 <= n)
             {
@@ -337,7 +322,7 @@ realpath_stk (const char *name, char *resolved,
 
               buf[n] = '\0';
 
-              char *extra_buf = extra_buffer.data;
+              char *extra_buf = bufs->extra.data;
               idx_t end_idx IF_LINT (= 0);
               if (end_in_extra_buffer)
                 end_idx = end - extra_buf;
@@ -345,13 +330,13 @@ realpath_stk (const char *name, char *resolved,
               if (INT_ADD_OVERFLOW (len, n))
                 {
                   __set_errno (ENOMEM);
-                  goto error_nomem;
+                  return NULL;
                 }
-              while (extra_buffer.length <= len + n)
+              while (bufs->extra.length <= len + n)
                 {
-                  if (!scratch_buffer_grow_preserve (&extra_buffer))
-                    goto error_nomem;
-                  extra_buf = extra_buffer.data;
+                  if (!scratch_buffer_grow_preserve (&bufs->extra))
+                    return NULL;
+                  extra_buf = bufs->extra.data;
                 }
               if (end_in_extra_buffer)
                 end = extra_buf + end_idx;
@@ -403,20 +388,30 @@ realpath_stk (const char *name, char *resolved,
 
 error:
   *dest++ = '\0';
-  if (resolved != NULL && dest - rname <= get_path_max ())
-    rname = strcpy (resolved, rname);
-
-error_nomem:
-  scratch_buffer_free (&extra_buffer);
-  scratch_buffer_free (&link_buffer);
-
-  if (failed || rname == resolved)
+  if (resolved != NULL)
+    {
+      /* Copy the full result on success or partial result if failure was due
+         to the path not existing or not being accessible.  */
+      if ((!failed || errno == ENOENT || errno == EACCES)
+          && dest - rname <= get_path_max ())
+        {
+          strcpy (resolved, rname);
+          if (failed)
+            return NULL;
+          else
+            return resolved;
+        }
+      if (!failed)
+        __set_errno (ENAMETOOLONG);
+      return NULL;
+    }
+  else
     {
-      scratch_buffer_free (rname_buf);
-      return failed ? NULL : resolved;
+      if (failed)
+        return NULL;
+      else
+        return __strdup (bufs->rname.data);
     }
-
-  return scratch_buffer_dupfree (rname_buf, dest - rname);
 }
 
 /* Return the canonical absolute name of file NAME.  A canonical name
@@ -433,12 +428,15 @@ error_nomem:
 char *
 __realpath (const char *name, char *resolved)
 {
-  #ifdef GCC_BOGUS_WRETURN_LOCAL_ADDR
-   #warning "GCC might issue a bogus -Wreturn-local-addr warning here."
-   #warning "See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644>."
-  #endif
-  struct scratch_buffer rname_buffer;
-  return realpath_stk (name, resolved, &rname_buffer);
+  struct realpath_bufs bufs;
+  scratch_buffer_init (&bufs.rname);
+  scratch_buffer_init (&bufs.extra);
+  scratch_buffer_init (&bufs.link);
+  char *result = realpath_stk (name, resolved, &bufs);
+  scratch_buffer_free (&bufs.link);
+  scratch_buffer_free (&bufs.extra);
+  scratch_buffer_free (&bufs.rname);
+  return result;
 }
 libc_hidden_def (__realpath)
 versioned_symbol (libc, __realpath, realpath, GLIBC_2_3);
diff --git a/lib/canonicalize.c b/lib/canonicalize.c
index e331e3ff1b..3342f70140 100644
--- a/lib/canonicalize.c
+++ b/lib/canonicalize.c
@@ -162,28 +162,18 @@ seen_triple (Hash_table **ht, char const *filename, struct stat const *st)
   return false;
 }
 
+/* Scratch buffers used by canonicalize_filename_mode_stk and managed
+   by __realpath.  */
+struct realpath_bufs
+{
+  struct scratch_buffer rname;
+  struct scratch_buffer extra;
+  struct scratch_buffer link;
+};
 
-/* Act like canonicalize_filename_mode (see below), with an additional argument
-   rname_buf that can be used as temporary storage.
-
-   If GCC_LINT is defined, do not inline this function with GCC 10.1
-   and later, to avoid creating a pointer to the stack that GCC
-   -Wreturn-local-addr incorrectly complains about.  See:
-   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644
-   Although the noinline attribute can hurt performance a bit, no better way
-   to pacify GCC is known; even an explicit #pragma does not pacify GCC.
-   When the GCC bug is fixed this workaround should be limited to the
-   broken GCC versions.  */
-#if _GL_GNUC_PREREQ (10, 1)
-# if defined GCC_LINT || defined lint
-__attribute__ ((__noinline__))
-# elif __OPTIMIZE__ && !__NO_INLINE__
-#  define GCC_BOGUS_WRETURN_LOCAL_ADDR
-# endif
-#endif
 static char *
 canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
-                                struct scratch_buffer *rname_buf)
+                                struct realpath_bufs *bufs)
 {
   char *dest;
   char const *start;
@@ -211,12 +201,7 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
       return NULL;
     }
 
-  struct scratch_buffer extra_buffer, link_buffer;
-  scratch_buffer_init (&extra_buffer);
-  scratch_buffer_init (&link_buffer);
-  scratch_buffer_init (rname_buf);
-  char *rname_on_stack = rname_buf->data;
-  char *rname = rname_on_stack;
+  char *rname = bufs->rname.data;
   bool end_in_extra_buffer = false;
   bool failed = true;
 
@@ -226,12 +211,12 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
 
   if (!IS_ABSOLUTE_FILE_NAME (name))
     {
-      while (!getcwd (rname, rname_buf->length))
+      while (!getcwd (bufs->rname.data, bufs->rname.length))
         {
           switch (errno)
             {
             case ERANGE:
-              if (scratch_buffer_grow (rname_buf))
+              if (scratch_buffer_grow (&bufs->rname))
                 break;
               FALLTHROUGH;
             case ENOMEM:
@@ -241,7 +226,7 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
               dest = rname;
               goto error;
             }
-          rname = rname_buf->data;
+          rname = bufs->rname.data;
         }
       dest = rawmemchr (rname, '\0');
       start = name;
@@ -265,7 +250,7 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
                 for (i = 2; name[i] != '\0' && !ISSLASH (name[i]); )
                   i++;
                 if (name[i] != '\0' /* implies ISSLASH (name[i]) */
-                    && i + 1 < rname_buf->length)
+                    && i + 1 < bufs->rname.length)
                   {
                     prefix_len = i;
                     memcpy (dest, name + 2, i - 2 + 1);
@@ -275,7 +260,7 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
                   {
                     /* Either name = '\\server'; this is an invalid file name.
                        Or name = '\\server\...' and server is more than
-                       rname_buf->length - 4 bytes long.  In either
+                       bufs->rname.length - 4 bytes long.  In either
                        case, stop the UNC processing.  */
                   }
               }
@@ -320,13 +305,13 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
           if (!ISSLASH (dest[-1]))
             *dest++ = '/';
 
-          while (rname + rname_buf->length - dest
+          while (rname + bufs->rname.length - dest
                  < startlen + sizeof dir_suffix)
             {
               idx_t dest_offset = dest - rname;
-              if (!scratch_buffer_grow_preserve (rname_buf))
+              if (!scratch_buffer_grow_preserve (&bufs->rname))
                 xalloc_die ();
-              rname = rname_buf->data;
+              rname = bufs->rname.data;
               dest = rname + dest_offset;
             }
 
@@ -339,12 +324,12 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
             {
               while (true)
                 {
-                  buf = link_buffer.data;
-                  idx_t bufsize = link_buffer.length;
+                  buf = bufs->link.data;
+                  idx_t bufsize = bufs->link.length;
                   n = readlink (rname, buf, bufsize - 1);
                   if (n < bufsize - 1)
                     break;
-                  if (!scratch_buffer_grow (&link_buffer))
+                  if (!scratch_buffer_grow (&bufs->link))
                     xalloc_die ();
                 }
             }
@@ -383,18 +368,18 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
 
               buf[n] = '\0';
 
-              char *extra_buf = extra_buffer.data;
+              char *extra_buf = bufs->extra.data;
               idx_t end_idx IF_LINT (= 0);
               if (end_in_extra_buffer)
                 end_idx = end - extra_buf;
               size_t len = strlen (end);
               if (INT_ADD_OVERFLOW (len, n))
                 xalloc_die ();
-              while (extra_buffer.length <= len + n)
+              while (bufs->extra.length <= len + n)
                 {
-                  if (!scratch_buffer_grow_preserve (&extra_buffer))
+                  if (!scratch_buffer_grow_preserve (&bufs->extra))
                     xalloc_die ();
-                  extra_buf = extra_buffer.data;
+                  extra_buf = bufs->extra.data;
                 }
               if (end_in_extra_buffer)
                 end = extra_buf + end_idx;
@@ -453,20 +438,15 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode,
 error:
   if (ht)
     hash_free (ht);
-  scratch_buffer_free (&extra_buffer);
-  scratch_buffer_free (&link_buffer);
 
   if (failed)
-    {
-      scratch_buffer_free (rname_buf);
-      return NULL;
-    }
+    return NULL;
 
   *dest++ = '\0';
-  char *result = scratch_buffer_dupfree (rname_buf, dest - rname);
+  char *result = malloc (dest - rname);
   if (!result)
     xalloc_die ();
-  return result;
+  return memcpy (result, rname, dest - rname);
 }
 
 /* Return the canonical absolute name of file NAME, while treating
@@ -479,10 +459,13 @@ error:
 char *
 canonicalize_filename_mode (const char *name, canonicalize_mode_t can_mode)
 {
-  #ifdef GCC_BOGUS_WRETURN_LOCAL_ADDR
-   #warning "GCC might issue a bogus -Wreturn-local-addr warning here."
-   #warning "See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644>."
-  #endif
-  struct scratch_buffer rname_buffer;
-  return canonicalize_filename_mode_stk (name, can_mode, &rname_buffer);
+  struct realpath_bufs bufs;
+  scratch_buffer_init (&bufs.rname);
+  scratch_buffer_init (&bufs.extra);
+  scratch_buffer_init (&bufs.link);
+  char *result = canonicalize_filename_mode_stk (name, can_mode, &bufs);
+  scratch_buffer_free (&bufs.link);
+  scratch_buffer_free (&bufs.extra);
+  scratch_buffer_free (&bufs.rname);
+  return result;
 }
diff --git a/lib/malloc/scratch_buffer.h b/lib/malloc/scratch_buffer.h
index e4c5c8a85d..a9bdcadec2 100644
--- a/lib/malloc/scratch_buffer.h
+++ b/lib/malloc/scratch_buffer.h
@@ -132,20 +132,4 @@ scratch_buffer_set_array_size (struct scratch_buffer *buffer,
 			 (buffer, nelem, size));
 }
 
-/* Return a copy of *BUFFER's first SIZE bytes as a heap-allocated block,
-   deallocating *BUFFER if it was heap-allocated.  SIZE must be at
-   most *BUFFER's size.  Return NULL (setting errno) on memory
-   exhaustion.  */
-void *__libc_scratch_buffer_dupfree (struct scratch_buffer *buffer,
-                                     size_t size);
-libc_hidden_proto (__libc_scratch_buffer_dupfree)
-
-/* Alias for __libc_scratch_dupfree.  */
-static __always_inline void *
-scratch_buffer_dupfree (struct scratch_buffer *buffer, size_t size)
-{
-  void *r = __libc_scratch_buffer_dupfree (buffer, size);
-  return __glibc_likely (r != NULL) ? r : NULL;
-}
-
 #endif /* _SCRATCH_BUFFER_H */
diff --git a/lib/malloc/scratch_buffer_dupfree.c b/lib/malloc/scratch_buffer_dupfree.c
deleted file mode 100644
index eb3b95c1b1..0000000000
--- a/lib/malloc/scratch_buffer_dupfree.c
+++ /dev/null
@@ -1,41 +0,0 @@
-/* Variable-sized buffer with on-stack default allocation.
-   Copyright (C) 2020-2022 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _LIBC
-# include <libc-config.h>
-#endif
-
-#include <scratch_buffer.h>
-#include <string.h>
-
-void *
-__libc_scratch_buffer_dupfree (struct scratch_buffer *buffer, size_t size)
-{
-  void *data = buffer->data;
-  if (data == buffer->__space.__c)
-    {
-      void *copy = malloc (size);
-      return copy != NULL ? memcpy (copy, data, size) : NULL;
-    }
-  else
-    {
-      void *copy = realloc (data, size);
-      return copy != NULL ? copy : data;
-    }
-}
-libc_hidden_def (__libc_scratch_buffer_dupfree)
diff --git a/lib/scratch_buffer.h b/lib/scratch_buffer.h
index f4fe5e8d34..c0aa21630f 100644
--- a/lib/scratch_buffer.h
+++ b/lib/scratch_buffer.h
@@ -98,20 +98,10 @@ extern bool scratch_buffer_set_array_size (struct scratch_buffer *buffer,
                                            size_t nelem, size_t size);
 #endif
 
-/* Return a copy of *BUFFER's first SIZE bytes as a heap-allocated block,
-   deallocating *BUFFER if it was heap-allocated.  SIZE must be at
-   most *BUFFER's size.  Return NULL (setting errno) on memory
-   exhaustion.  */
-#if 0
-extern void *scratch_buffer_dupfree (struct scratch_buffer *buffer,
-                                     size_t size);
-#endif
-
 
 /* The implementation is imported from glibc.  */
 
 /* Avoid possible conflicts with symbols exported by the GNU libc.  */
-#define __libc_scratch_buffer_dupfree gl_scratch_buffer_dupfree
 #define __libc_scratch_buffer_grow gl_scratch_buffer_grow
 #define __libc_scratch_buffer_grow_preserve gl_scratch_buffer_grow_preserve
 #define __libc_scratch_buffer_set_array_size gl_scratch_buffer_set_array_size
diff --git a/modules/relocatable-prog-wrapper b/modules/relocatable-prog-wrapper
index a60aa24d67..052499c1ad 100644
--- a/modules/relocatable-prog-wrapper
+++ b/modules/relocatable-prog-wrapper
@@ -19,7 +19,6 @@ lib/stat.c
 lib/canonicalize-lgpl.c
 lib/scratch_buffer.h
 lib/malloc/scratch_buffer.h
-lib/malloc/scratch_buffer_dupfree.c
 lib/malloc/scratch_buffer_grow.c
 lib/malloc/scratch_buffer_grow_preserve.c
 lib/malloc/scratch_buffer_set_array_size.c
diff --git a/modules/scratch_buffer b/modules/scratch_buffer
index 921c8f5497..ace8a34d2c 100644
--- a/modules/scratch_buffer
+++ b/modules/scratch_buffer
@@ -4,7 +4,6 @@ Variable-sized buffer with on-stack default allocation.
 Files:
 lib/scratch_buffer.h
 lib/malloc/scratch_buffer.h
-lib/malloc/scratch_buffer_dupfree.c
 lib/malloc/scratch_buffer_grow.c
 lib/malloc/scratch_buffer_grow_preserve.c
 lib/malloc/scratch_buffer_set_array_size.c
@@ -37,8 +36,7 @@ malloc/scratch_buffer.gl.h: malloc/scratch_buffer.h
 	$(AM_V_at)mv $@-t $@
 MOSTLYCLEANFILES += malloc/scratch_buffer.gl.h malloc/scratch_buffer.gl.h-t
 
-lib_SOURCES += malloc/scratch_buffer_dupfree.c \
-               malloc/scratch_buffer_grow.c \
+lib_SOURCES += malloc/scratch_buffer_grow.c \
                malloc/scratch_buffer_grow_preserve.c \
                malloc/scratch_buffer_set_array_size.c
 
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03  1:18 ` Paul Eggert
@ 2022-11-03  2:37   ` Bruno Haible
  2022-11-03  3:09     ` Paul Eggert
  0 siblings, 1 reply; 13+ messages in thread
From: Bruno Haible @ 2022-11-03  2:37 UTC (permalink / raw)
  To: bug-gnulib, Paul Eggert; +Cc: Karl Berry, Florian Weimer

Paul Eggert wrote:
> Yes, it's a nontrivial merge. However, I think we can still sync 
> LIBC/include/scratch_buffer.h. I installed the attached patch

It's a backward-incompatible change: A documented function is now
suddenly gone, without a deprecation period. Let me document it in
NEWS.

I think this has some consequences on how we deal with glibc internals
in Gnulib. We exported the 'scratch_buffer' module, thinking that it's
a welcome addition to the Gnulib API. But we are seeing that
either
   the glibc people did not know that this API is exported from Gnulib,
or
   they knew but ignored the fact that this API is exported from Gnulib.
Could anything be done to avoid such things from occurring again?

If not, then I'm inclined to view this way of exporting glibc internals
as a failed experiment. And as a consequence, we should only take and
export glibc code if it is
  - either a POSIX API, or
  - documented in the glibc manual.
And if we have Gnulib modules that contain glibc-internal code, we should
make it clear that it is not Gnulib public API.

In other words, I'm suggesting to rename the modules
  scratch_buffer -> glibc-internal/scratch_buffer
  dynarray       -> glibc-internal/dynarray


2022-11-02  Bruno Haible  <bruno@clisp.org>

	scratch_buffer: Document last change.
	* NEWS: Mention last change.

diff --git a/NEWS b/NEWS
index 840564703a..327fc8ceee 100644
--- a/NEWS
+++ b/NEWS
@@ -74,6 +74,8 @@ User visible incompatible changes
 
 Date        Modules         Changes
 
+2022-11-02  scratch_buffer  The function 'gl_scratch_buffer_dupfree' is removed.
+
 2022-09-10  stdbool         This module now assumes C99 and provides C23,
                             instead of providing C99.  For the old behavior,
                             use the already-deprecated stdbool-c99 module.





^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03  2:37   ` Bruno Haible
@ 2022-11-03  3:09     ` Paul Eggert
  2022-11-03 10:51       ` Bruno Haible
  0 siblings, 1 reply; 13+ messages in thread
From: Paul Eggert @ 2022-11-03  3:09 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib; +Cc: Karl Berry, Florian Weimer

On 11/2/22 19:37, Bruno Haible wrote:
> In other words, I'm suggesting to rename the modules
>    scratch_buffer -> glibc-internal/scratch_buffer
>    dynarray       -> glibc-internal/dynarray

I don't see any problem with that, since as far as I know the only 
current users of the two modules are glibc-related modules in Gnulib 
itself. However, if in the future programs find these two modules 
useful, I suppose we'll need to rename them back.

The scratch_buffer_dupfree function was added to glibc in 2020 only to 
make it easier to sync canonicalize.c with Gnulib at the time. So it is 
a bit ironic that removing scratch_buffer_dupfree from Gnulib now would 
cause us to in some sense deprecate scratch_buffer.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03  3:09     ` Paul Eggert
@ 2022-11-03 10:51       ` Bruno Haible
  2022-11-03 11:03         ` Florian Weimer
  0 siblings, 1 reply; 13+ messages in thread
From: Bruno Haible @ 2022-11-03 10:51 UTC (permalink / raw)
  To: bug-gnulib, Paul Eggert; +Cc: Karl Berry, Florian Weimer

Hi Paul,

Paul Eggert wrote:
> > In other words, I'm suggesting to rename the modules
> >    scratch_buffer -> glibc-internal/scratch_buffer
> >    dynarray       -> glibc-internal/dynarray
> 
> I don't see any problem with that, since as far as I know the only 
> current users of the two modules are glibc-related modules in Gnulib 
> itself.

Yes, as far as I can see, no packages have started to use these two
modules yet.

> However, if in the future programs find these two modules 
> useful, I suppose we'll need to rename them back.

No, I don't think this is going to fly, since we would be offering an API
for use, for which we cannot guarantee a reasonable degree of stability.

The liaison between Gnulib and Glibc is mostly you. If the glibc people
want to make changes to intprops.h, idx.h, filename.h, or the regex code,
you will certainly be aware of it, and be able to prevent breakage for
Gnulib users.

But when it comes to scratch_buffer or dynarray code, this week's experience
has shown that we cannot count on as much care for Gnulib users. Rather,
the mindset on the glibc side seems to be: "This is glibc internal code;
we can refactor / reshuffle / trim it as we like." [1][2]

So, if we want to offer a Gnulib module from glibc-internal code, it would
be our job to maintain compatibility across glibc's refactorings. In this
particular case, it would have meant to add the scratch_buffer_dupfree.c
as a Gnulib-owned source file. But it the long run, this is going to be
a growing maintenance effort. (We have a similar situation: gettext's libintl
is derived from glibc/intl/, and it's a continuing effort to keep the two
more or less in sync.)

Therefore I agree with what you did yesterday: remove scratch_buffer_dupfree.c
from Gnulib, since glibc dropped it.

But it means that we cannot promise that these .h files are even remotely
stable APIs.

Bruno

[1] https://sourceware.org/pipermail/libc-alpha/2022-October/143030.html
[2] https://sourceware.org/pipermail/libc-alpha/2022-October/143033.html





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03 10:51       ` Bruno Haible
@ 2022-11-03 11:03         ` Florian Weimer
  2022-11-03 12:41           ` Bruno Haible
  2022-11-03 18:26           ` Paul Eggert
  0 siblings, 2 replies; 13+ messages in thread
From: Florian Weimer @ 2022-11-03 11:03 UTC (permalink / raw)
  To: Bruno Haible; +Cc: bug-gnulib, Paul Eggert, Karl Berry

* Bruno Haible:

> But when it comes to scratch_buffer or dynarray code, this week's experience
> has shown that we cannot count on as much care for Gnulib users. Rather,
> the mindset on the glibc side seems to be: "This is glibc internal code;
> we can refactor / reshuffle / trim it as we like." [1][2]
>
> So, if we want to offer a Gnulib module from glibc-internal code, it would
> be our job to maintain compatibility across glibc's refactorings. In this
> particular case, it would have meant to add the scratch_buffer_dupfree.c
> as a Gnulib-owned source file. But it the long run, this is going to be
> a growing maintenance effort. (We have a similar situation: gettext's libintl
> is derived from glibc/intl/, and it's a continuing effort to keep the two
> more or less in sync.)
>
> Therefore I agree with what you did yesterday: remove scratch_buffer_dupfree.c
> from Gnulib, since glibc dropped it.
>
> But it means that we cannot promise that these .h files are even remotely
> stable APIs.

I must say I was surprised to see dynarray and scratch_buffer end up in
gnulib.  I never intended them to escape this way from glibc.  The
interfaces and their implementation are problematic in some ways, and I
can't recommend them for general use.

Thanks,
Florian



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03 11:03         ` Florian Weimer
@ 2022-11-03 12:41           ` Bruno Haible
  2022-11-03 18:27             ` Paul Eggert
  2022-11-03 18:26           ` Paul Eggert
  1 sibling, 1 reply; 13+ messages in thread
From: Bruno Haible @ 2022-11-03 12:41 UTC (permalink / raw)
  To: Florian Weimer; +Cc: bug-gnulib, Paul Eggert, Karl Berry

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

Florian Weimer wrote:
> > But it means that we cannot promise that these .h files are even remotely
> > stable APIs.
> 
> I must say I was surprised to see dynarray and scratch_buffer end up in
> gnulib.  I never intended them to escape this way from glibc.  The
> interfaces and their implementation are problematic in some ways, and I
> can't recommend them for general use.

Thanks for this clear statement, Florian.

So I was too optimistic when I wrote
  - "It looks like the 'scratch_buffer' could be useful to some programs
     outside of glibc." [1]
  - "dynarray looks useful" [2]

Here are proposed patches to rename the modules.

[1] https://lists.gnu.org/archive/html/bug-gnulib/2021-02/msg00073.html
[2] https://lists.gnu.org/archive/html/bug-gnulib/2021-03/msg00042.html


2022-11-03  Bruno Haible  <bruno@clisp.org>

	dynarray: Rename to glibc-internal/dynarray.
	* modules/glibc-internal/dynarray: Renamed from modules/dynarray.
	* modules/glibc-internal/dynarray-tests: Renamed from
	modules/dynarray-tests.
	* modules/regex (Depends-on): Update.
	* NEWS: Mention this change and the previous one.

2022-11-03  Bruno Haible  <bruno@clisp.org>

	scratch_buffer: Rename to glibc-internal/scratch_buffer.
	* modules/glibc-internal/scratch_buffer: Renamed from
	modules/scratch_buffer.
	* modules/glibc-internal/scratch_buffer-tests: Renamed from
	modules/scratch_buffer-tests.
	* modules/canonicalize (Depends-on): Update.
	* modules/canonicalize-lgpl (Depends-on): Likewise.
	* modules/glob (Depends-on): Likewise.


[-- Attachment #2: 0001-scratch_buffer-Rename-to-glibc-internal-scratch_buff.patch --]
[-- Type: text/x-patch, Size: 9642 bytes --]

From 371a0f95a7e90d29a7272d0e7cfde1eb0b61a235 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Thu, 3 Nov 2022 13:30:04 +0100
Subject: [PATCH 1/2] scratch_buffer: Rename to glibc-internal/scratch_buffer.

* modules/glibc-internal/scratch_buffer: Renamed from
modules/scratch_buffer.
* modules/glibc-internal/scratch_buffer-tests: Renamed from
modules/scratch_buffer-tests.
* modules/canonicalize (Depends-on): Update.
* modules/canonicalize-lgpl (Depends-on): Likewise.
* modules/glob (Depends-on): Likewise.
---
 ChangeLog                                     | 11 +++++
 modules/canonicalize                          |  2 +-
 modules/canonicalize-lgpl                     | 36 ++++++++---------
 modules/{ => glibc-internal}/scratch_buffer   |  0
 .../{ => glibc-internal}/scratch_buffer-tests |  0
 modules/glob                                  | 40 +++++++++----------
 6 files changed, 50 insertions(+), 39 deletions(-)
 rename modules/{ => glibc-internal}/scratch_buffer (100%)
 rename modules/{ => glibc-internal}/scratch_buffer-tests (100%)

diff --git a/ChangeLog b/ChangeLog
index a7a3f30c0d..70e717284b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2022-11-03  Bruno Haible  <bruno@clisp.org>
+
+	scratch_buffer: Rename to glibc-internal/scratch_buffer.
+	* modules/glibc-internal/scratch_buffer: Renamed from
+	modules/scratch_buffer.
+	* modules/glibc-internal/scratch_buffer-tests: Renamed from
+	modules/scratch_buffer-tests.
+	* modules/canonicalize (Depends-on): Update.
+	* modules/canonicalize-lgpl (Depends-on): Likewise.
+	* modules/glob (Depends-on): Likewise.
+
 2022-11-02  Bruno Haible  <bruno@clisp.org>
 
 	scratch_buffer: Document last change.
diff --git a/modules/canonicalize b/modules/canonicalize
index ddd83bc841..25667c431b 100644
--- a/modules/canonicalize
+++ b/modules/canonicalize
@@ -24,7 +24,7 @@ mempcpy
 nocrash
 rawmemchr
 readlink
-scratch_buffer
+glibc-internal/scratch_buffer
 stat
 stdbool
 sys_stat
diff --git a/modules/canonicalize-lgpl b/modules/canonicalize-lgpl
index 252618e95e..7842cda2da 100644
--- a/modules/canonicalize-lgpl
+++ b/modules/canonicalize-lgpl
@@ -11,24 +11,24 @@ Depends-on:
 extensions
 stdlib
 nocrash
-double-slash-root [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-eloop-threshold   [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-errno             [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-fcntl-h           [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-filename          [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-idx               [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-intprops          [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-libc-config       [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-memmove           [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-mempcpy           [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-pathmax           [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-rawmemchr         [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-readlink          [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-scratch_buffer    [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-stat              [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-stdbool           [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-sys_stat          [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
-unistd            [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+double-slash-root             [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+eloop-threshold               [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+errno                         [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+fcntl-h                       [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+filename                      [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+idx                           [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+intprops                      [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+libc-config                   [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+memmove                       [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+mempcpy                       [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+pathmax                       [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+rawmemchr                     [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+readlink                      [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+glibc-internal/scratch_buffer [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+stat                          [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+stdbool                       [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+sys_stat                      [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
+unistd                        [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1]
 
 configure.ac:
 gl_CANONICALIZE_LGPL
diff --git a/modules/scratch_buffer b/modules/glibc-internal/scratch_buffer
similarity index 100%
rename from modules/scratch_buffer
rename to modules/glibc-internal/scratch_buffer
diff --git a/modules/scratch_buffer-tests b/modules/glibc-internal/scratch_buffer-tests
similarity index 100%
rename from modules/scratch_buffer-tests
rename to modules/glibc-internal/scratch_buffer-tests
diff --git a/modules/glob b/modules/glob
index 83cd729ceb..4a89521e01 100644
--- a/modules/glob
+++ b/modules/glob
@@ -13,26 +13,26 @@ Depends-on:
 glob-h
 c99
 largefile
-alloca          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-builtin-expect  [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-closedir        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-d-type          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-dirfd           [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-flexmember      [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-fnmatch         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-fstatat         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-getlogin_r      [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-libc-config     [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-memchr          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-mempcpy         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-opendir         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-readdir         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-scratch_buffer  [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-stdbool         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-stdint          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-strdup          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-unistd          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-malloc-posix    [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+alloca                        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+builtin-expect                [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+closedir                      [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+d-type                        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+dirfd                         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+flexmember                    [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+fnmatch                       [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+fstatat                       [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+getlogin_r                    [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+libc-config                   [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+memchr                        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+mempcpy                       [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+opendir                       [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+readdir                       [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+glibc-internal/scratch_buffer [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+stdbool                       [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+stdint                        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+strdup                        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+unistd                        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+malloc-posix                  [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 
 configure.ac:
 gl_GLOB
-- 
2.34.1


[-- Attachment #3: 0002-dynarray-Rename-to-glibc-internal-dynarray.patch --]
[-- Type: text/x-patch, Size: 4967 bytes --]

From 9543a96e7e0975cda93cfb777d2b2e346cc290c5 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Thu, 3 Nov 2022 13:32:42 +0100
Subject: [PATCH 2/2] dynarray: Rename to glibc-internal/dynarray.

* modules/glibc-internal/dynarray: Renamed from modules/dynarray.
* modules/glibc-internal/dynarray-tests: Renamed from
modules/dynarray-tests.
* modules/regex (Depends-on): Update.
* NEWS: Mention this change and the previous one.
---
 ChangeLog                                   |  9 +++++
 NEWS                                        |  4 ++
 modules/{ => glibc-internal}/dynarray       |  0
 modules/{ => glibc-internal}/dynarray-tests |  0
 modules/regex                               | 42 ++++++++++-----------
 5 files changed, 34 insertions(+), 21 deletions(-)
 rename modules/{ => glibc-internal}/dynarray (100%)
 rename modules/{ => glibc-internal}/dynarray-tests (100%)

diff --git a/ChangeLog b/ChangeLog
index 70e717284b..70ece5200a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2022-11-03  Bruno Haible  <bruno@clisp.org>
+
+	dynarray: Rename to glibc-internal/dynarray.
+	* modules/glibc-internal/dynarray: Renamed from modules/dynarray.
+	* modules/glibc-internal/dynarray-tests: Renamed from
+	modules/dynarray-tests.
+	* modules/regex (Depends-on): Update.
+	* NEWS: Mention this change and the previous one.
+
 2022-11-03  Bruno Haible  <bruno@clisp.org>
 
 	scratch_buffer: Rename to glibc-internal/scratch_buffer.
diff --git a/NEWS b/NEWS
index 327fc8ceee..3b83e2978d 100644
--- a/NEWS
+++ b/NEWS
@@ -74,6 +74,10 @@ User visible incompatible changes
 
 Date        Modules         Changes
 
+2022-11-03  dynarray        These modules are renamed to glibc-internal/dynarray
+            scratch_buffer  and glibc-internal/scratch_buffer, respectively.
+                            They are not meant for general use.
+
 2022-11-02  scratch_buffer  The function 'gl_scratch_buffer_dupfree' is removed.
 
 2022-09-10  stdbool         This module now assumes C99 and provides C23,
diff --git a/modules/dynarray b/modules/glibc-internal/dynarray
similarity index 100%
rename from modules/dynarray
rename to modules/glibc-internal/dynarray
diff --git a/modules/dynarray-tests b/modules/glibc-internal/dynarray-tests
similarity index 100%
rename from modules/dynarray-tests
rename to modules/glibc-internal/dynarray-tests
diff --git a/modules/regex b/modules/regex
index b780427221..7779a1e796 100644
--- a/modules/regex
+++ b/modules/regex
@@ -20,27 +20,27 @@ c99
 extensions
 ssize_t
 vararrays
-attribute       [test $ac_use_included_regex = yes]
-btowc           [test $ac_use_included_regex = yes]
-builtin-expect  [test $ac_use_included_regex = yes]
-dynarray        [test $ac_use_included_regex = yes]
-intprops        [test $ac_use_included_regex = yes]
-iswctype        [test $ac_use_included_regex = yes]
-langinfo        [test $ac_use_included_regex = yes]
-libc-config     [test $ac_use_included_regex = yes]
-lock            [test $ac_use_included_regex = yes]
-memcmp          [test $ac_use_included_regex = yes]
-memmove         [test $ac_use_included_regex = yes]
-mbrtowc         [test $ac_use_included_regex = yes]
-mbsinit         [test $ac_use_included_regex = yes]
-nl_langinfo     [test $ac_use_included_regex = yes]
-stdbool         [test $ac_use_included_regex = yes]
-stdint          [test $ac_use_included_regex = yes]
-verify          [test $ac_use_included_regex = yes]
-wchar           [test $ac_use_included_regex = yes]
-wcrtomb         [test $ac_use_included_regex = yes]
-wctype-h        [test $ac_use_included_regex = yes]
-wctype          [test $ac_use_included_regex = yes]
+attribute               [test $ac_use_included_regex = yes]
+btowc                   [test $ac_use_included_regex = yes]
+builtin-expect          [test $ac_use_included_regex = yes]
+glibc-internal/dynarray [test $ac_use_included_regex = yes]
+intprops                [test $ac_use_included_regex = yes]
+iswctype                [test $ac_use_included_regex = yes]
+langinfo                [test $ac_use_included_regex = yes]
+libc-config             [test $ac_use_included_regex = yes]
+lock                    [test $ac_use_included_regex = yes]
+memcmp                  [test $ac_use_included_regex = yes]
+memmove                 [test $ac_use_included_regex = yes]
+mbrtowc                 [test $ac_use_included_regex = yes]
+mbsinit                 [test $ac_use_included_regex = yes]
+nl_langinfo             [test $ac_use_included_regex = yes]
+stdbool                 [test $ac_use_included_regex = yes]
+stdint                  [test $ac_use_included_regex = yes]
+verify                  [test $ac_use_included_regex = yes]
+wchar                   [test $ac_use_included_regex = yes]
+wcrtomb                 [test $ac_use_included_regex = yes]
+wctype-h                [test $ac_use_included_regex = yes]
+wctype                  [test $ac_use_included_regex = yes]
 
 configure.ac:
 gl_REGEX
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03 11:03         ` Florian Weimer
  2022-11-03 12:41           ` Bruno Haible
@ 2022-11-03 18:26           ` Paul Eggert
  2022-11-03 19:37             ` Florian Weimer
  2022-11-03 20:40             ` Karl Berry
  1 sibling, 2 replies; 13+ messages in thread
From: Paul Eggert @ 2022-11-03 18:26 UTC (permalink / raw)
  To: Florian Weimer, Bruno Haible; +Cc: bug-gnulib, Karl Berry

On 2022-11-03 04:03, Florian Weimer wrote:
> I must say I was surprised to see dynarray and scratch_buffer end up in
> gnulib.  I never intended them to escape this way from glibc.

They escaped from glibc because they're used by code shared with Gnulib 
(e.g., canonicalize.c).

>  The
> interfaces and their implementation are problematic in some ways, and I
> can't recommend them for general use.

Thanks for letting us know.

What problems do you see with the interfaces, and are there efforts to 
come up with a better API? The need is there in GNU apps, each of which 
tends to roll its own code here.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03 12:41           ` Bruno Haible
@ 2022-11-03 18:27             ` Paul Eggert
  2022-11-03 20:44               ` Bruno Haible
  0 siblings, 1 reply; 13+ messages in thread
From: Paul Eggert @ 2022-11-03 18:27 UTC (permalink / raw)
  To: Bruno Haible, Florian Weimer; +Cc: bug-gnulib, Karl Berry

On 2022-11-03 05:41, Bruno Haible wrote:
> Here are proposed patches to rename the modules.

Thanks, those look good to me too.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03 18:26           ` Paul Eggert
@ 2022-11-03 19:37             ` Florian Weimer
  2022-11-03 20:40             ` Karl Berry
  1 sibling, 0 replies; 13+ messages in thread
From: Florian Weimer @ 2022-11-03 19:37 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Bruno Haible, bug-gnulib, Karl Berry

* Paul Eggert:

> What problems do you see with the interfaces, and are there efforts to
> come up with a better API? The need is there in GNU apps, each of
> which tends to roll its own code here.

dynarray has an aliasing violation in its implementation.  The embedded
pointer should be void * (not DYNARRAY_ELEMENT *), with casts added in
the generated code, so that the out-of-line code can use the correct
types.

Defining the element free function has a trap for the unwary programmer:
it's easy to miss the required pointer indirection.  I don't know a good
fix for that.

I view dynarray as a stop-gap until we can use a C++ template inside
glibc.  We won't be able to use std::vector because of gaps in memory
allocation failure handling, but application code interested in basic
type-generic data structures should really use libstdc++ and not
preprocessor hacks like dynarray.  With such wide use of xmalloc &
friends, the memory allocation issue would impact few applications.

The scratch buffer interface is mainly intended as a helper for calling
NSS functions because of their highly problematic buffer interface: it's
not just that the caller allocates, the callee does not provide any size
hint at all if the provided buffer is not large enough.  So the only
thing you can do is keep retrying with larger and larger buffers.  No
one should create interfaces like that.

One could argue that scratch buffers are needed because the NSS
interfaces exist today, but from a glibc perspective, I think we should
provide replacements for the problematic NSS functions that are
significantly easier to use, rather than relying on developers to put
something together using scratch buffers.  Maybe it's sufficient to make
the original functions like getpwuid thread-safe, although such
functions manipulating global state wouldn't be really library-safe.
(You wouldn't want to call getpwuid in a library because it might
clobber another getpwuid result still in use elsewhere, but that's an
issue that is present with or without thread safety.)

I added scratch_buffer_grow_preserve and scratch_buffer_set_array_size
as an afterthought.  Conceptually, they are unrelated to the NSS usage.
They predate dynarrays.  If dynarrays were easier to define, we should
have switched over to them because they have implicit overflow checking
for allocation sizes.

Thanks,
Florian



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03 18:26           ` Paul Eggert
  2022-11-03 19:37             ` Florian Weimer
@ 2022-11-03 20:40             ` Karl Berry
  2022-11-03 21:12               ` Paul Eggert
  1 sibling, 1 reply; 13+ messages in thread
From: Karl Berry @ 2022-11-03 20:40 UTC (permalink / raw)
  To: eggert, bruno, bug-gnulib

Whatever happens, can one of you make the desired changes in gnulib?  I
have never tried to follow the glibc/gnulib stuff. This one is above my
pay grade :). I just blindly sync the changes ... --thanks, karl.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03 18:27             ` Paul Eggert
@ 2022-11-03 20:44               ` Bruno Haible
  0 siblings, 0 replies; 13+ messages in thread
From: Bruno Haible @ 2022-11-03 20:44 UTC (permalink / raw)
  To: Florian Weimer, Paul Eggert; +Cc: bug-gnulib, Karl Berry

Paul Eggert wrote:
> > Here are proposed patches to rename the modules.
> 
> Thanks, those look good to me too.

Thanks for the review. Pushed.

Bruno





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
  2022-11-03 20:40             ` Karl Berry
@ 2022-11-03 21:12               ` Paul Eggert
  0 siblings, 0 replies; 13+ messages in thread
From: Paul Eggert @ 2022-11-03 21:12 UTC (permalink / raw)
  To: Karl Berry, bruno, bug-gnulib

On 11/3/22 13:40, Karl Berry wrote:
> Whatever happens, can one of you make the desired changes in gnulib?

Already done and installed, here:

https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=ddfcbc95a6be3ddc588a93f21edb69cc7c214d9c

As part of that patch, I did the same sort of sync that you regularly do 
(and thanks for that).


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-11-03 21:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-02 22:37 scratch_buffer.h, scratch_buffer_dupfree.c sync Karl Berry
2022-11-03  1:18 ` Paul Eggert
2022-11-03  2:37   ` Bruno Haible
2022-11-03  3:09     ` Paul Eggert
2022-11-03 10:51       ` Bruno Haible
2022-11-03 11:03         ` Florian Weimer
2022-11-03 12:41           ` Bruno Haible
2022-11-03 18:27             ` Paul Eggert
2022-11-03 20:44               ` Bruno Haible
2022-11-03 18:26           ` Paul Eggert
2022-11-03 19:37             ` Florian Weimer
2022-11-03 20:40             ` Karl Berry
2022-11-03 21:12               ` Paul Eggert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).