bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* [PATCH] base32, base64: prefer signed to unsigned integers
@ 2021-08-27 22:27 Paul Eggert
  2021-08-28 12:45 ` Bruno Haible
  2021-08-29  8:20 ` [PATCH] base32, base64: prefer signed to unsigned integers Simon Josefsson via Gnulib discussion list
  0 siblings, 2 replies; 17+ messages in thread
From: Paul Eggert @ 2021-08-27 22:27 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Paul Eggert

* lib/base32.c, lib/base64.c: Include ialloc.h instad of stdlib.h.
Include intprops.h, verify.h.
* lib/base32.c (base32_encode, base32_encode_alloc, get_8, decode_8)
(base32_decode_ctx, base32_decode_alloc_ctx):
* lib/base32.h (struct base32_decode_context):
* lib/base64.c (base64_encode_fast, base64_encode)
(base64_encode_alloc, get_4, decode_4, base64_decode_ctx)
(base64_decode_alloc_ctx):
* lib/base64.h (struct base64_decode_context):
* tests/test-base32.c (main):
* tests/test-base64.c (main):
Prefer signed to unsigned integers.
* lib/base32.c (base32_encode_alloc):
* lib/base64.c (base64_encode_alloc):
Use simpler and more-direct check for overflow, removing a TODO.
* lib/base32.h, lib/base64.h: Include idx.h instead of stddef.h.
* modules/base32, modules/base64 (Depends-on): Add ialloc, verify.
---
 ChangeLog           | 21 ++++++++++++++
 NEWS                |  3 ++
 lib/base32.c        | 61 ++++++++++++++++++++-------------------
 lib/base32.h        | 20 ++++++-------
 lib/base64.c        | 69 ++++++++++++++++++++-------------------------
 lib/base64.h        | 20 ++++++-------
 modules/base32      |  2 ++
 modules/base64      |  2 ++
 tests/test-base32.c |  2 +-
 tests/test-base64.c |  2 +-
 10 files changed, 111 insertions(+), 91 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index adfbcf3e21..498525a242 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2021-08-27  Paul Eggert  <eggert@cs.ucla.edu>
+
+	base32, base64: prefer signed to unsigned integers
+	* lib/base32.c, lib/base64.c: Include ialloc.h instad of stdlib.h.
+	Include intprops.h, verify.h.
+	* lib/base32.c (base32_encode, base32_encode_alloc, get_8, decode_8)
+	(base32_decode_ctx, base32_decode_alloc_ctx):
+	* lib/base32.h (struct base32_decode_context):
+	* lib/base64.c (base64_encode_fast, base64_encode)
+	(base64_encode_alloc, get_4, decode_4, base64_decode_ctx)
+	(base64_decode_alloc_ctx):
+	* lib/base64.h (struct base64_decode_context):
+	* tests/test-base32.c (main):
+	* tests/test-base64.c (main):
+	Prefer signed to unsigned integers.
+	* lib/base32.c (base32_encode_alloc):
+	* lib/base64.c (base64_encode_alloc):
+	Use simpler and more-direct check for overflow, removing a TODO.
+	* lib/base32.h, lib/base64.h: Include idx.h instead of stddef.h.
+	* modules/base32, modules/base64 (Depends-on): Add ialloc, verify.
+
 2021-08-26  Paul Eggert  <eggert@cs.ucla.edu>
 
 	regex: use __attr_access and C99-style array arg
diff --git a/NEWS b/NEWS
index e0f0d836c2..2ce1c73014 100644
--- a/NEWS
+++ b/NEWS
@@ -66,6 +66,9 @@ User visible incompatible changes
 
 Date        Modules         Changes
 
+2021-08-27  base32          These modules now use idx_t instead of size_t
+            base64          for indexes and counts.
+
 2021-07-29  (all)           Due to draft C2x, the following attributes should
                             now appear at the start of a function declaration:
                               _GL_ATTRIBUTE_DEPRECATED
diff --git a/lib/base32.c b/lib/base32.c
index 75d0b296fd..e8feaf1663 100644
--- a/lib/base32.c
+++ b/lib/base32.c
@@ -28,7 +28,7 @@
  *   FAIL: memory allocation error
  * OK: data in OUT/OUTLEN
  *
- * size_t outlen = base32_encode_alloc (in, inlen, &out);
+ * idx_t outlen = base32_encode_alloc (in, inlen, &out);
  * if (out == NULL && outlen == 0 && inlen != 0)
  *   FAIL: input too long
  * if (out == NULL)
@@ -42,15 +42,18 @@
 /* Get prototype. */
 #include "base32.h"
 
-/* Get malloc. */
-#include <stdlib.h>
+/* Get imalloc. */
+#include <ialloc.h>
+
+#include <intprops.h>
+#include <verify.h>
 
 /* Get UCHAR_MAX. */
 #include <limits.h>
 
 #include <string.h>
 
-/* C89 compliant way to cast 'char' to 'unsigned char'. */
+/* Convert 'char' to 'unsigned char' without casting.  */
 static unsigned char
 to_uchar (char ch)
 {
@@ -62,8 +65,8 @@ to_uchar (char ch)
    possible.  If OUTLEN is larger than BASE32_LENGTH(INLEN), also zero
    terminate the output buffer. */
 void
-base32_encode (const char *restrict in, size_t inlen,
-               char *restrict out, size_t outlen)
+base32_encode (const char *restrict in, idx_t inlen,
+               char *restrict out, idx_t outlen)
 {
   static const char b32str[32] =
     "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567";
@@ -136,24 +139,20 @@ base32_encode (const char *restrict in, size_t inlen,
    memory allocation failed, OUT is set to NULL, and the return value
    indicates length of the requested memory block, i.e.,
    BASE32_LENGTH(inlen) + 1. */
-size_t
-base32_encode_alloc (const char *in, size_t inlen, char **out)
+idx_t
+base32_encode_alloc (const char *in, idx_t inlen, char **out)
 {
-  size_t outlen = 1 + BASE32_LENGTH (inlen);
-
-  /* Check for overflow in outlen computation.
-   *
-   * If there is no overflow, outlen >= inlen.
-   *
-   * TODO Is this a sufficient check?  (See the notes in base64.c.)
-   */
-  if (inlen > outlen)
+  /* Check for overflow in outlen computation.  */
+  assume (0 <= inlen);
+  idx_t in_over_5 = inlen / 5 + (inlen % 5 != 0), outlen;
+  if (! INT_MULTIPLY_OK (in_over_5, 8, &outlen))
     {
       *out = NULL;
       return 0;
     }
+  outlen++;
 
-  *out = malloc (outlen);
+  *out = imalloc (outlen);
   if (!*out)
     return outlen;
 
@@ -305,7 +304,7 @@ base32_decode_ctx_init (struct base32_decode_context *ctx)
 static char *
 get_8 (struct base32_decode_context *ctx,
        char const *restrict *in, char const *restrict in_end,
-       size_t *n_non_newline)
+       idx_t *n_non_newline)
 {
   if (ctx->i == 8)
     ctx->i = 0;
@@ -357,14 +356,14 @@ get_8 (struct base32_decode_context *ctx,
    *OUT to point to the byte after the last one written, and decrement
    *OUTLEN to reflect the number of bytes remaining in *OUT.  */
 static bool
-decode_8 (char const *restrict in, size_t inlen,
-          char *restrict *outp, size_t *outleft)
+decode_8 (char const *restrict in, idx_t inlen,
+          char *restrict *outp, idx_t *outleft)
 {
   char *out = *outp;
   if (inlen < 8)
     return false;
 
-  if (!isbase32 (in[0]) || !isbase32 (in[1]) )
+  if (!isbase32 (in[0]) || !isbase32 (in[1]))
     return false;
 
   if (*outleft)
@@ -468,10 +467,10 @@ decode_8 (char const *restrict in, size_t inlen,
 
 bool
 base32_decode_ctx (struct base32_decode_context *ctx,
-                   const char *restrict in, size_t inlen,
-                   char *restrict out, size_t *outlen)
+                   const char *restrict in, idx_t inlen,
+                   char *restrict out, idx_t *outlen)
 {
-  size_t outleft = *outlen;
+  idx_t outleft = *outlen;
   bool ignore_newlines = ctx != NULL;
   bool flush_ctx = false;
   unsigned int ctx_i = 0;
@@ -485,7 +484,7 @@ base32_decode_ctx (struct base32_decode_context *ctx,
 
   while (true)
     {
-      size_t outleft_save = outleft;
+      idx_t outleft_save = outleft;
       if (ctx_i == 0 && !flush_ctx)
         {
           while (true)
@@ -559,17 +558,17 @@ base32_decode_ctx (struct base32_decode_context *ctx,
    undefined. */
 bool
 base32_decode_alloc_ctx (struct base32_decode_context *ctx,
-                         const char *in, size_t inlen, char **out,
-                         size_t *outlen)
+                         const char *in, idx_t inlen, char **out,
+                         idx_t *outlen)
 {
   /* This may allocate a few bytes too many, depending on input,
      but it's not worth the extra CPU time to compute the exact size.
      The exact size is 5 * inlen / 8, minus one or more bytes if the
      input is padded with one or more "=".
-     Dividing before multiplying avoids the possibility of overflow.  */
-  size_t needlen = 5 * (inlen / 8) + 5;
+     Shifting before multiplying avoids the possibility of overflow.  */
+  idx_t needlen = 5 * ((inlen >> 3) + 1);
 
-  *out = malloc (needlen);
+  *out = imalloc (needlen);
   if (!*out)
     return true;
 
diff --git a/lib/base32.h b/lib/base32.h
index f65e55b0fe..108a5ecd38 100644
--- a/lib/base32.h
+++ b/lib/base32.h
@@ -18,8 +18,8 @@
 #ifndef BASE32_H
 # define BASE32_H
 
-/* Get size_t. */
-# include <stddef.h>
+/* Get idx_t. */
+# include <idx.h>
 
 /* Get bool. */
 # include <stdbool.h>
@@ -30,26 +30,26 @@
 
 struct base32_decode_context
 {
-  unsigned int i;
+  int i;
   char buf[8];
 };
 
 extern bool isbase32 (char ch) _GL_ATTRIBUTE_CONST;
 
-extern void base32_encode (const char *restrict in, size_t inlen,
-                           char *restrict out, size_t outlen);
+extern void base32_encode (const char *restrict in, idx_t inlen,
+                           char *restrict out, idx_t outlen);
 
-extern size_t base32_encode_alloc (const char *in, size_t inlen, char **out);
+extern idx_t base32_encode_alloc (const char *in, idx_t inlen, char **out);
 
 extern void base32_decode_ctx_init (struct base32_decode_context *ctx);
 
 extern bool base32_decode_ctx (struct base32_decode_context *ctx,
-                               const char *restrict in, size_t inlen,
-                               char *restrict out, size_t *outlen);
+                               const char *restrict in, idx_t inlen,
+                               char *restrict out, idx_t *outlen);
 
 extern bool base32_decode_alloc_ctx (struct base32_decode_context *ctx,
-                                     const char *in, size_t inlen,
-                                     char **out, size_t *outlen);
+                                     const char *in, idx_t inlen,
+                                     char **out, idx_t *outlen);
 
 #define base32_decode(in, inlen, out, outlen) \
         base32_decode_ctx (NULL, in, inlen, out, outlen)
diff --git a/lib/base64.c b/lib/base64.c
index 7a6e7af5fc..2a01ed34e9 100644
--- a/lib/base64.c
+++ b/lib/base64.c
@@ -30,7 +30,7 @@
  *   FAIL: memory allocation error
  * OK: data in OUT/OUTLEN
  *
- * size_t outlen = base64_encode_alloc (in, inlen, &out);
+ * idx_t outlen = base64_encode_alloc (in, inlen, &out);
  * if (out == NULL && outlen == 0 && inlen != 0)
  *   FAIL: input too long
  * if (out == NULL)
@@ -44,15 +44,18 @@
 /* Get prototype. */
 #include "base64.h"
 
-/* Get malloc. */
-#include <stdlib.h>
+/* Get imalloc. */
+#include <ialloc.h>
+
+#include <intprops.h>
+#include <verify.h>
 
 /* Get UCHAR_MAX. */
 #include <limits.h>
 
 #include <string.h>
 
-/* C89 compliant way to cast 'char' to 'unsigned char'. */
+/* Convert 'char' to 'unsigned char' without casting.  */
 static unsigned char
 to_uchar (char ch)
 {
@@ -66,7 +69,7 @@ static const char b64c[64] =
    to be of length >= BASE64_LENGTH(INLEN), and INLEN needs to be
    a multiple of 3.  */
 static void
-base64_encode_fast (const char *restrict in, size_t inlen, char *restrict out)
+base64_encode_fast (const char *restrict in, idx_t inlen, char *restrict out)
 {
   while (inlen)
     {
@@ -85,8 +88,8 @@ base64_encode_fast (const char *restrict in, size_t inlen, char *restrict out)
    possible.  If OUTLEN is larger than BASE64_LENGTH(INLEN), also zero
    terminate the output buffer. */
 void
-base64_encode (const char *restrict in, size_t inlen,
-               char *restrict out, size_t outlen)
+base64_encode (const char *restrict in, idx_t inlen,
+               char *restrict out, idx_t outlen)
 {
   /* Note this outlen constraint can be enforced at compile time.
      I.E. that the output buffer is exactly large enough to hold
@@ -95,7 +98,7 @@ base64_encode (const char *restrict in, size_t inlen,
      at the end of input.  However the common case when reading
      large inputs is to have both constraints satisfied, so we depend
      on both in base_encode_fast().  */
-  if (outlen % 4 == 0 && inlen == outlen / 4 * 3)
+  if (outlen % 4 == 0 && inlen == (outlen >> 2) * 3)
     {
       base64_encode_fast (in, inlen, out);
       return;
@@ -141,30 +144,20 @@ base64_encode (const char *restrict in, size_t inlen,
    memory allocation failed, OUT is set to NULL, and the return value
    indicates length of the requested memory block, i.e.,
    BASE64_LENGTH(inlen) + 1. */
-size_t
-base64_encode_alloc (const char *in, size_t inlen, char **out)
+idx_t
+base64_encode_alloc (const char *in, idx_t inlen, char **out)
 {
-  size_t outlen = 1 + BASE64_LENGTH (inlen);
-
-  /* Check for overflow in outlen computation.
-   *
-   * If there is no overflow, outlen >= inlen.
-   *
-   * If the operation (inlen + 2) overflows then it yields at most +1, so
-   * outlen is 0.
-   *
-   * If the multiplication overflows, we lose at least half of the
-   * correct value, so the result is < ((inlen + 2) / 3) * 2, which is
-   * less than (inlen + 2) * 0.66667, which is less than inlen as soon as
-   * (inlen > 4).
-   */
-  if (inlen > outlen)
+  /* Check for overflow in outlen computation.  */
+  assume (0 <= inlen);
+  idx_t in_over_3 = inlen / 3 + (inlen % 3 != 0), outlen;
+  if (! INT_MULTIPLY_OK (in_over_3, 4, &outlen))
     {
       *out = NULL;
       return 0;
     }
+  outlen++;
 
-  *out = malloc (outlen);
+  *out = imalloc (outlen);
   if (!*out)
     return outlen;
 
@@ -348,7 +341,7 @@ base64_decode_ctx_init (struct base64_decode_context *ctx)
 static char *
 get_4 (struct base64_decode_context *ctx,
        char const *restrict *in, char const *restrict in_end,
-       size_t *n_non_newline)
+       idx_t *n_non_newline)
 {
   if (ctx->i == 4)
     ctx->i = 0;
@@ -400,8 +393,8 @@ get_4 (struct base64_decode_context *ctx,
    *OUT to point to the byte after the last one written, and decrement
    *OUTLEN to reflect the number of bytes remaining in *OUT.  */
 static bool
-decode_4 (char const *restrict in, size_t inlen,
-          char *restrict *outp, size_t *outleft)
+decode_4 (char const *restrict in, idx_t inlen,
+          char *restrict *outp, idx_t *outleft)
 {
   char *out = *outp;
   if (inlen < 2)
@@ -486,10 +479,10 @@ decode_4 (char const *restrict in, size_t inlen,
 
 bool
 base64_decode_ctx (struct base64_decode_context *ctx,
-                   const char *restrict in, size_t inlen,
-                   char *restrict out, size_t *outlen)
+                   const char *restrict in, idx_t inlen,
+                   char *restrict out, idx_t *outlen)
 {
-  size_t outleft = *outlen;
+  idx_t outleft = *outlen;
   bool ignore_newlines = ctx != NULL;
   bool flush_ctx = false;
   unsigned int ctx_i = 0;
@@ -503,7 +496,7 @@ base64_decode_ctx (struct base64_decode_context *ctx,
 
   while (true)
     {
-      size_t outleft_save = outleft;
+      idx_t outleft_save = outleft;
       if (ctx_i == 0 && !flush_ctx)
         {
           while (true)
@@ -577,17 +570,17 @@ base64_decode_ctx (struct base64_decode_context *ctx,
    undefined. */
 bool
 base64_decode_alloc_ctx (struct base64_decode_context *ctx,
-                         const char *in, size_t inlen, char **out,
-                         size_t *outlen)
+                         const char *in, idx_t inlen, char **out,
+                         idx_t *outlen)
 {
   /* This may allocate a few bytes too many, depending on input,
      but it's not worth the extra CPU time to compute the exact size.
      The exact size is 3 * (inlen + (ctx ? ctx->i : 0)) / 4, minus 1 if the
      input ends with "=" and minus another 1 if the input ends with "==".
-     Dividing before multiplying avoids the possibility of overflow.  */
-  size_t needlen = 3 * (inlen / 4) + 3;
+     Shifting before multiplying avoids the possibility of overflow.  */
+  idx_t needlen = 3 * ((inlen >> 2) + 1);
 
-  *out = malloc (needlen);
+  *out = imalloc (needlen);
   if (!*out)
     return true;
 
diff --git a/lib/base64.h b/lib/base64.h
index e45f1db728..e58ccfb1fb 100644
--- a/lib/base64.h
+++ b/lib/base64.h
@@ -18,8 +18,8 @@
 #ifndef BASE64_H
 # define BASE64_H
 
-/* Get size_t. */
-# include <stddef.h>
+/* Get idx_t.  */
+# include <idx.h>
 
 /* Get bool. */
 # include <stdbool.h>
@@ -34,26 +34,26 @@ extern "C" {
 
 struct base64_decode_context
 {
-  unsigned int i;
+  int i;
   char buf[4];
 };
 
 extern bool isbase64 (char ch) _GL_ATTRIBUTE_CONST;
 
-extern void base64_encode (const char *restrict in, size_t inlen,
-                           char *restrict out, size_t outlen);
+extern void base64_encode (const char *restrict in, idx_t inlen,
+                           char *restrict out, idx_t outlen);
 
-extern size_t base64_encode_alloc (const char *in, size_t inlen, char **out);
+extern idx_t base64_encode_alloc (const char *in, idx_t inlen, char **out);
 
 extern void base64_decode_ctx_init (struct base64_decode_context *ctx);
 
 extern bool base64_decode_ctx (struct base64_decode_context *ctx,
-                               const char *restrict in, size_t inlen,
-                               char *restrict out, size_t *outlen);
+                               const char *restrict in, idx_t inlen,
+                               char *restrict out, idx_t *outlen);
 
 extern bool base64_decode_alloc_ctx (struct base64_decode_context *ctx,
-                                     const char *in, size_t inlen,
-                                     char **out, size_t *outlen);
+                                     const char *in, idx_t inlen,
+                                     char **out, idx_t *outlen);
 
 #define base64_decode(in, inlen, out, outlen) \
         base64_decode_ctx (NULL, in, inlen, out, outlen)
diff --git a/modules/base32 b/modules/base32
index 20f38cf1a0..659081d7e3 100644
--- a/modules/base32
+++ b/modules/base32
@@ -7,8 +7,10 @@ lib/base32.c
 m4/base32.m4
 
 Depends-on:
+ialloc
 stdbool
 memchr
+verify
 
 configure.ac:
 gl_FUNC_BASE32
diff --git a/modules/base64 b/modules/base64
index b1a3513b7d..717c0697d1 100644
--- a/modules/base64
+++ b/modules/base64
@@ -7,8 +7,10 @@ lib/base64.c
 m4/base64.m4
 
 Depends-on:
+ialloc
 stdbool
 memchr
+verify
 
 configure.ac:
 gl_FUNC_BASE64
diff --git a/tests/test-base32.c b/tests/test-base32.c
index 0f7a43894d..24c46567de 100644
--- a/tests/test-base32.c
+++ b/tests/test-base32.c
@@ -34,7 +34,7 @@ main (void)
   const char *in = "abcdefghijklmnop";
   const char *b32in = "MFRGGZDFMZTWQ2LKNNWG23TPOA======";
   char out[255];
-  size_t len;
+  idx_t len;
   bool ok;
   char *p;
 
diff --git a/tests/test-base64.c b/tests/test-base64.c
index c16e50267e..bc75acd95f 100644
--- a/tests/test-base64.c
+++ b/tests/test-base64.c
@@ -33,7 +33,7 @@ main (void)
   const char *in = "abcdefghijklmnop";
   const char *b64in = "YWJjZGVmZw==";
   char out[255];
-  size_t len;
+  idx_t len;
   bool ok;
   char *p;
 
-- 
2.31.1



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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-27 22:27 [PATCH] base32, base64: prefer signed to unsigned integers Paul Eggert
@ 2021-08-28 12:45 ` Bruno Haible
  2021-08-28 14:12   ` Bruno Haible
  2021-08-29  7:49   ` Paul Eggert
  2021-08-29  8:20 ` [PATCH] base32, base64: prefer signed to unsigned integers Simon Josefsson via Gnulib discussion list
  1 sibling, 2 replies; 17+ messages in thread
From: Bruno Haible @ 2021-08-28 12:45 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

Hi Paul,

There are two problems with this patch:

1) $ ./gnulib-tool --create-testdir --dir=../testdir5 --single-configure base32 base64
   prints messages
   gnulib-tool: warning: module base32 depends on a module with an incompatible license: ialloc
   gnulib-tool: warning: module base64 depends on a module with an incompatible license: ialloc

   The obvious fix would be to change the license of 'ialloc' from LGPL to LGPLv2+.

2) The continuous integration reported a test failure (both with GCC and clang):

   test-base64.c:131: assertion 'len == 0' failed
   FAIL test-base64 (exit status: 134)

   But, strangely, I can't reproduce it locally. (Maybe it depends on the glibc
   version?)

Bruno





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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-28 12:45 ` Bruno Haible
@ 2021-08-28 14:12   ` Bruno Haible
  2021-08-29  7:57     ` Paul Eggert
  2021-08-29  7:49   ` Paul Eggert
  1 sibling, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-08-28 14:12 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

> 2) The continuous integration reported a test failure (both with GCC and clang):
> 
>    test-base64.c:131: assertion 'len == 0' failed
>    FAIL test-base64 (exit status: 134)
> 
>    But, strangely, I can't reproduce it locally. (Maybe it depends on the glibc
>    version?)

Or maybe the cause is the line

  assume (0 <= inlen);

At the entry point of a public function, it is better to use 'assert' than
'assume', IMO. 'assume' means "feel free to crash or press the red button
if there is an invalid argument".

Bruno





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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-28 12:45 ` Bruno Haible
  2021-08-28 14:12   ` Bruno Haible
@ 2021-08-29  7:49   ` Paul Eggert
  2021-09-19 22:07     ` ialloc: relicense Bruno Haible
  1 sibling, 1 reply; 17+ messages in thread
From: Paul Eggert @ 2021-08-29  7:49 UTC (permalink / raw)
  To: Bruno Haible; +Cc: bug-gnulib

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

On 8/28/21 5:45 AM, Bruno Haible wrote:

> There are two problems with this patch:
> 
> 1) $ ./gnulib-tool --create-testdir --dir=../testdir5 --single-configure base32 base64
>     prints messages
>     gnulib-tool: warning: module base32 depends on a module with an incompatible license: ialloc
>     gnulib-tool: warning: module base64 depends on a module with an incompatible license: ialloc
> 
>     The obvious fix would be to change the license of 'ialloc' from LGPL to LGPLv2+.

Sure, done in first attached patch.

> 2) The continuous integration reported a test failure (both with GCC and clang):
> 
>     test-base64.c:131: assertion 'len == 0' failed
>     FAIL test-base64 (exit status: 134)
> 
>     But, strangely, I can't reproduce it locally. (Maybe it depends on the glibc
>     version?)

There is a portability issue in that code, which I fixed by installing 
the second attached patch. I hope it fixes the problem you observed.

[-- Attachment #2: 0001-ialloc-relicense.patch --]
[-- Type: text/x-patch, Size: 904 bytes --]

From 34298f25c3d1d56f0502fdad42df3af6764e6a5d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 29 Aug 2021 00:27:10 -0700
Subject: [PATCH 1/2] ialloc: relicense

* modules/ialloc (License): Change from LGPL to LGPLv2+.
---
 ChangeLog      | 5 +++++
 modules/ialloc | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 599ce55ed..e46b36efb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2021-08-29  Paul Eggert  <eggert@cs.ucla.edu>
+
+	ialloc: relicense
+	* modules/ialloc (License): Change from LGPL to LGPLv2+.
+
 2021-08-28  Bruno Haible  <bruno@clisp.org>
 
 	fma: Fix compilation error on Linux/sh4.
diff --git a/modules/ialloc b/modules/ialloc
index 3bf377a62..bcf431399 100644
--- a/modules/ialloc
+++ b/modules/ialloc
@@ -23,7 +23,7 @@ Include:
 "ialloc.h"
 
 License:
-LGPL
+LGPLv2+
 
 Maintainer:
 all
-- 
2.30.2


[-- Attachment #3: 0002-base32-base64-fix-broken-tests.patch --]
[-- Type: text/x-patch, Size: 4173 bytes --]

From 93280a4bdca1c6e6fa1946fbf9d8621c42bdd692 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 29 Aug 2021 00:45:43 -0700
Subject: [PATCH 2/2] base32, base64: fix broken tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2021-08/msg00170.html
* lib/base32.c, lib/base64.c: Do not include verify.h,
and omit all uses of ‘assume’.
* modules/base32, modules/base64 (Depends-on): Remove verify.
* tests/test-base32.c, tests/test-base64.c:
Don’t pass out-of-range values to allocator,
as converting them to idx_t relies on implementation-defined
behavior that could trap.
---
 ChangeLog           | 11 +++++++++++
 lib/base32.c        |  2 --
 lib/base64.c        |  2 --
 modules/base32      |  1 -
 modules/base64      |  1 -
 tests/test-base32.c |  2 +-
 tests/test-base64.c |  2 +-
 7 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e46b36efb..d9f291fcd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
 2021-08-29  Paul Eggert  <eggert@cs.ucla.edu>
 
+	base32, base64: fix broken tests
+	Problem reported by Bruno Haible in:
+	https://lists.gnu.org/r/bug-gnulib/2021-08/msg00170.html
+	* lib/base32.c, lib/base64.c: Do not include verify.h,
+	and omit all uses of ‘assume’.
+	* modules/base32, modules/base64 (Depends-on): Remove verify.
+	* tests/test-base32.c, tests/test-base64.c:
+	Don’t pass out-of-range values to allocator,
+	as converting them to idx_t relies on implementation-defined
+	behavior that could trap.
+
 	ialloc: relicense
 	* modules/ialloc (License): Change from LGPL to LGPLv2+.
 
diff --git a/lib/base32.c b/lib/base32.c
index e8feaf166..e3f2f9b4c 100644
--- a/lib/base32.c
+++ b/lib/base32.c
@@ -46,7 +46,6 @@
 #include <ialloc.h>
 
 #include <intprops.h>
-#include <verify.h>
 
 /* Get UCHAR_MAX. */
 #include <limits.h>
@@ -143,7 +142,6 @@ idx_t
 base32_encode_alloc (const char *in, idx_t inlen, char **out)
 {
   /* Check for overflow in outlen computation.  */
-  assume (0 <= inlen);
   idx_t in_over_5 = inlen / 5 + (inlen % 5 != 0), outlen;
   if (! INT_MULTIPLY_OK (in_over_5, 8, &outlen))
     {
diff --git a/lib/base64.c b/lib/base64.c
index 2a01ed34e..4611fe548 100644
--- a/lib/base64.c
+++ b/lib/base64.c
@@ -48,7 +48,6 @@
 #include <ialloc.h>
 
 #include <intprops.h>
-#include <verify.h>
 
 /* Get UCHAR_MAX. */
 #include <limits.h>
@@ -148,7 +147,6 @@ idx_t
 base64_encode_alloc (const char *in, idx_t inlen, char **out)
 {
   /* Check for overflow in outlen computation.  */
-  assume (0 <= inlen);
   idx_t in_over_3 = inlen / 3 + (inlen % 3 != 0), outlen;
   if (! INT_MULTIPLY_OK (in_over_3, 4, &outlen))
     {
diff --git a/modules/base32 b/modules/base32
index 659081d7e..93c180b09 100644
--- a/modules/base32
+++ b/modules/base32
@@ -10,7 +10,6 @@ Depends-on:
 ialloc
 stdbool
 memchr
-verify
 
 configure.ac:
 gl_FUNC_BASE32
diff --git a/modules/base64 b/modules/base64
index 717c0697d..278e52fc8 100644
--- a/modules/base64
+++ b/modules/base64
@@ -10,7 +10,6 @@ Depends-on:
 ialloc
 stdbool
 memchr
-verify
 
 configure.ac:
 gl_FUNC_BASE64
diff --git a/tests/test-base32.c b/tests/test-base32.c
index 24c46567d..25df559fd 100644
--- a/tests/test-base32.c
+++ b/tests/test-base32.c
@@ -150,7 +150,7 @@ main (void)
   ASSERT (strcmp (p, "MFRGGZDFMZTWQ2LKNNWG23TPOA======") == 0);
   free (p);
 
-  len = base32_encode_alloc (in, SIZE_MAX - 5, &p);
+  len = base32_encode_alloc (in, IDX_MAX - 5, &p);
   ASSERT (len == 0);
 
   /* Decode context function */
diff --git a/tests/test-base64.c b/tests/test-base64.c
index bc75acd95..a7f4e5370 100644
--- a/tests/test-base64.c
+++ b/tests/test-base64.c
@@ -127,7 +127,7 @@ main (void)
   ASSERT (strcmp (p, "YWJjZGVmZ2hpamtsbW5vcA==") == 0);
   free (p);
 
-  len = base64_encode_alloc (in, SIZE_MAX - 5, &p);
+  len = base64_encode_alloc (in, IDX_MAX - 5, &p);
   ASSERT (len == 0);
 
   /* Decode context function */
-- 
2.30.2


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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-28 14:12   ` Bruno Haible
@ 2021-08-29  7:57     ` Paul Eggert
  2021-08-29 10:16       ` Bruno Haible
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Eggert @ 2021-08-29  7:57 UTC (permalink / raw)
  To: Bruno Haible; +Cc: bug-gnulib

On 8/28/21 7:12 AM, Bruno Haible wrote:
> Or maybe the cause is the line
> 
>    assume (0 <= inlen);
> 
> At the entry point of a public function, it is better to use 'assert' than
> 'assume', IMO. 'assume' means "feel free to crash or press the red button
> if there is an invalid argument".

'assume' was the intent. A negative idx_t arg is an error as serious as 
an out-of-range index in an array, and so should be undefined behavior. 
We shouldn't sprinkle 'assert's all over the place for this: it should 
be something builtin to the compiler and/or runtime system when one 
enables runtime checking.

I put in the 'assume' only to help GCC generate better code (to let it 
choose unsigned or signed division, whichever it thinks is faster). 
That's overkill here and the 'assume's are evidently dust magnets so I 
removed the 'assume's in the patch I recently installed.


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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-27 22:27 [PATCH] base32, base64: prefer signed to unsigned integers Paul Eggert
  2021-08-28 12:45 ` Bruno Haible
@ 2021-08-29  8:20 ` Simon Josefsson via Gnulib discussion list
  2021-08-29  9:56   ` Bruno Haible
  1 sibling, 1 reply; 17+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2021-08-29  8:20 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

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

Paul Eggert <eggert@cs.ucla.edu> writes:

> -extern void base64_encode (const char *restrict in, size_t inlen,
> -                           char *restrict out, size_t outlen);
> +extern void base64_encode (const char *restrict in, idx_t inlen,
> +                           char *restrict out, idx_t outlen);

Thanks for improving the code -- however, the API is quite wide spread
already, and size_t (or unsigned int) is widely used for many other
base64 APIs so this change causes friction at the API level.  What do
you think?  I'm not sure I understand why idx_t is better than size_t
here, can you elaborate?  Why not ssize_t?  Maybe a compromise is to
keep the old API but add new APIs with idx_t types and the
implementation of the old functions uses the new one.

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-29  8:20 ` [PATCH] base32, base64: prefer signed to unsigned integers Simon Josefsson via Gnulib discussion list
@ 2021-08-29  9:56   ` Bruno Haible
  2021-08-30 14:12     ` Simon Josefsson via Gnulib discussion list
  0 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-08-29  9:56 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: Paul Eggert, bug-gnulib

Hi Simon,

> I'm not sure I understand why idx_t is better than size_t
> here, can you elaborate?  Why not ssize_t?

You find a detailed explanation in the comments of idx.h.

> Maybe a compromise is to
> keep the old API but add new APIs with idx_t types and the
> implementation of the old functions uses the new one.

The objective is to eliminate bugs due to the use of unsigned types
for numerical values. We can achieve it only by increasing the use
of signed types such as 'idx_t'. If we keep the old function,
it needs to be marked with __attribute__ ((__deprecated__)), otherwise
existing code will continue to use the old function forever.

Such a compromise comes with a cost: extra function names, that will
stick around for a long time. Here, since size_t and idx_t have the
same size at the binary level, the churn is limited: only the functions
with a ctx argument have changed the element type of a pointer arguments,
and despite warnings, no wrong code will be executed — except where, like
in the test suite, a value > SIZE_MAX / 2 is passed as argument.

Bruno





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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-29  7:57     ` Paul Eggert
@ 2021-08-29 10:16       ` Bruno Haible
  2021-08-29 17:25         ` Paul Eggert
  0 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-08-29 10:16 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

Paul Eggert wrote:
> > Or maybe the cause is the line
> > 
> >    assume (0 <= inlen);
> > 
> > At the entry point of a public function, it is better to use 'assert' than
> > 'assume', IMO. 'assume' means "feel free to crash or press the red button
> > if there is an invalid argument".
> 
> 'assume' was the intent. A negative idx_t arg is an error as serious as 
> an out-of-range index in an array, and so should be undefined behavior. 

I could somehow agree with this, if base64_encode_alloc was a new API.
But there is an API change. There may be invocations out there, of this
function, with an argument between SIZE_MAX/2 and SIZE_MAX-1. Changing
such calls to be undefined behaviour means that these invocations now
need debugging in the packages that contain them.

IMO, it would be better to have code like this in base64_encode_alloc:

  if (inlen < 0)
    /* This argument is invalid, since the API change from 2021-08-28.  */
    abort ();

An abort() is a much better indication that something is wrong, for those
users of the function that have not updated their code yet, than a
test suite that spuriously fails.

Bruno





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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-29 10:16       ` Bruno Haible
@ 2021-08-29 17:25         ` Paul Eggert
  2021-08-29 18:36           ` Bruno Haible
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Eggert @ 2021-08-29 17:25 UTC (permalink / raw)
  To: Bruno Haible; +Cc: bug-gnulib

On 8/29/21 3:16 AM, Bruno Haible wrote:

> There may be invocations out there, of this
> function, with an argument between SIZE_MAX/2 and SIZE_MAX-1. Changing
> such calls to be undefined behaviour means that these invocations now
> need debugging in the packages that contain them.

Luckily these calls are not present in practical code (as opposed to 
artificial test cases).

> IMO, it would be better to have code like this in base64_encode_alloc:
> 
>   if (inlen < 0)
>     /* This argument is invalid, since the API change from 2021-08-28.  */
>     abort ();

Another possibility would be to treat inlen < 0 the same as integer 
overflow. I could go either way.


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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-29 17:25         ` Paul Eggert
@ 2021-08-29 18:36           ` Bruno Haible
  2021-08-29 20:01             ` Paul Eggert
  0 siblings, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-08-29 18:36 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

Paul Eggert wrote:
> > IMO, it would be better to have code like this in base64_encode_alloc:
> > 
> >   if (inlen < 0)
> >     /* This argument is invalid, since the API change from 2021-08-28.  */
> >     abort ();
> 
> Another possibility would be to treat inlen < 0 the same as integer 
> overflow. I could go either way.

Yes, either an abort() or an integer overflow return indicator would be better
than undefined behaviour, for something that was defined behaviour until last
week.

Bruno





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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-29 18:36           ` Bruno Haible
@ 2021-08-29 20:01             ` Paul Eggert
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Eggert @ 2021-08-29 20:01 UTC (permalink / raw)
  To: Bruno Haible; +Cc: bug-gnulib

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

On 8/29/21 11:36 AM, Bruno Haible wrote:

> Yes, either an abort() or an integer overflow return indicator would be better

OK, I installed the attached, which takes the integer overflow return route.

[-- Attachment #2: 0001-base32-base64-treat-negative-sizes-as-overflows.patch --]
[-- Type: text/x-patch, Size: 2403 bytes --]

From 6aafd2a92b4bb48937f3e767e51a4b7abf2f2217 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 29 Aug 2021 12:58:49 -0700
Subject: [PATCH] base32, base64: treat negative sizes as overflows

* lib/base64.c (base64_encode_alloc):
* lib/base32.c (base32_encode_alloc):
Treat negative sizes as overflows, for better compatibility
with previous API.
---
 ChangeLog    | 8 ++++++++
 lib/base32.c | 6 ++++--
 lib/base64.c | 6 ++++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ce9a2b366..ee933c9ef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2021-08-29  Paul Eggert  <eggert@cs.ucla.edu>
+
+	base32, base64: treat negative sizes as overflows
+	* lib/base64.c (base64_encode_alloc):
+	* lib/base32.c (base32_encode_alloc):
+	Treat negative sizes as overflows, for better compatibility
+	with previous API.
+
 2021-08-29  Bruno Haible  <bruno@clisp.org>
 
 	explicit_bzero test: Fix test failure due to GCC optimizations.
diff --git a/lib/base32.c b/lib/base32.c
index e3f2f9b4c..037747d80 100644
--- a/lib/base32.c
+++ b/lib/base32.c
@@ -141,9 +141,11 @@ base32_encode (const char *restrict in, idx_t inlen,
 idx_t
 base32_encode_alloc (const char *in, idx_t inlen, char **out)
 {
-  /* Check for overflow in outlen computation.  */
+  /* Check for overflow in outlen computation.
+     Treat negative INLEN as overflow, for better compatibility with
+     pre-2021-08-27 API, which used size_t.  */
   idx_t in_over_5 = inlen / 5 + (inlen % 5 != 0), outlen;
-  if (! INT_MULTIPLY_OK (in_over_5, 8, &outlen))
+  if (! INT_MULTIPLY_OK (in_over_5, 8, &outlen) || inlen < 0)
     {
       *out = NULL;
       return 0;
diff --git a/lib/base64.c b/lib/base64.c
index 4611fe548..b204cb711 100644
--- a/lib/base64.c
+++ b/lib/base64.c
@@ -146,9 +146,11 @@ base64_encode (const char *restrict in, idx_t inlen,
 idx_t
 base64_encode_alloc (const char *in, idx_t inlen, char **out)
 {
-  /* Check for overflow in outlen computation.  */
+  /* Check for overflow in outlen computation.
+     Treat negative INLEN as overflow, for better compatibility with
+     pre-2021-08-27 API, which used size_t.  */
   idx_t in_over_3 = inlen / 3 + (inlen % 3 != 0), outlen;
-  if (! INT_MULTIPLY_OK (in_over_3, 4, &outlen))
+  if (! INT_MULTIPLY_OK (in_over_3, 4, &outlen) || inlen < 0)
     {
       *out = NULL;
       return 0;
-- 
2.30.2


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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-29  9:56   ` Bruno Haible
@ 2021-08-30 14:12     ` Simon Josefsson via Gnulib discussion list
  2021-08-30 18:17       ` Paul Eggert
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2021-08-30 14:12 UTC (permalink / raw)
  To: Bruno Haible; +Cc: Paul Eggert, bug-gnulib

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

Bruno Haible <bruno@clisp.org> writes:

> Hi Simon,
>
>> I'm not sure I understand why idx_t is better than size_t
>> here, can you elaborate?  Why not ssize_t?
>
> You find a detailed explanation in the comments of idx.h.

Thanks for the pointer -- it doesn't say anything about why ssize_t
can't be used though?  As a signed variant of size_t, it seems relevant
to consider.

>> Maybe a compromise is to
>> keep the old API but add new APIs with idx_t types and the
>> implementation of the old functions uses the new one.
>
> The objective is to eliminate bugs due to the use of unsigned types
> for numerical values.

Is that a realistic goal with C using the unsigned type size_t for
low-level functions like strlen()?  It seems like an un-idiomatic goal.

> We can achieve it only by increasing the use of signed types such as
> 'idx_t'. If we keep the old function, it needs to be marked with
> __attribute__ ((__deprecated__)), otherwise existing code will
> continue to use the old function forever.

My idea was that both APIs would be supported indefinitely.

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-30 14:12     ` Simon Josefsson via Gnulib discussion list
@ 2021-08-30 18:17       ` Paul Eggert
  2021-08-30 18:58         ` Simon Josefsson via Gnulib discussion list
  2021-09-04 10:06         ` autoupdate again Bruno Haible
  0 siblings, 2 replies; 17+ messages in thread
From: Paul Eggert @ 2021-08-30 18:17 UTC (permalink / raw)
  To: Simon Josefsson, Bruno Haible; +Cc: Gnulib bugs

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

On 8/30/21 7:12 AM, Simon Josefsson wrote:

> Thanks for the pointer -- it doesn't say anything about why ssize_t
> can't be used though?  As a signed variant of size_t, it seems relevant
> to consider.

Good question. Addressed in the attached patch, which I pushed.

>> The objective is to eliminate bugs due to the use of unsigned types
>> for numerical values.
> 
> Is that a realistic goal with C using the unsigned type size_t for
> low-level functions like strlen()?  It seems like an un-idiomatic goal.

It is realistic, at least within the Gnulib context. It's also realistic 
in the context of Glibc, which has recently started to prohibit heap 
allocations larger than PTRDIFF_MAX for the usual security/correctness 
reasons.

The attached patch also attempts to address this question.

> My idea was that both APIs would be supported indefinitely.

Bruno already addressed this point, and I tend to agree with him for 
this particular API.

[-- Attachment #2: 0001-idx-add-commentary.patch --]
[-- Type: text/x-patch, Size: 2225 bytes --]

From 763dad031043b0342c825a8a05ca2fa28e9d905e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 30 Aug 2021 11:15:20 -0700
Subject: [PATCH] idx: add commentary

* lib/idx.h: Add comments about ssize_t and strlen, in
response to comments from Simon Josefsson in:
https://lists.gnu.org/r/bug-gnulib/2021-08/msg00196.html
---
 ChangeLog |  7 +++++++
 lib/idx.h | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index b86a6c33a..1b1372cb7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2021-08-30  Paul Eggert  <eggert@cs.ucla.edu>
+
+	idx: add commentary
+	* lib/idx.h: Add comments about ssize_t and strlen, in
+	response to comments from Simon Josefsson in:
+	https://lists.gnu.org/r/bug-gnulib/2021-08/msg00196.html
+
 2021-08-29  Paul Eggert  <eggert@cs.ucla.edu>
 
 	attribute: keep up with gnulib-common.m4
diff --git a/lib/idx.h b/lib/idx.h
index 483587eab..54ad5d81f 100644
--- a/lib/idx.h
+++ b/lib/idx.h
@@ -56,6 +56,26 @@
      * Because 'size_t' is an unsigned type, and a signed type is better.
        See above.
 
+   Why not use 'ssize_t'?
+
+     * 'ptrdiff_t' is more portable; it is standardized by ISO C
+       whereas 'ssize_t' is standardized only by POSIX.
+
+     * 'ssize_t' is not required to be as wide as 'size_t', and some
+       now-obsolete POSIX platforms had 'size_t' wider than 'ssize_t'.
+
+     * Conversely, some now-obsolete platforms had 'ptrdiff_t' wider
+       than 'size_t', which can be a win and conforms to POSIX.
+
+   Won't this cause a problem with objects larger than PTRDIFF_MAX?
+
+     * Typical modern or large platforms do not allocate such objects,
+       so this is not much of a problem in practice; for example, you
+       can safely write 'idx_t len = strlen (s);'.  To port to older
+       small platforms where allocations larger than PTRDIFF_MAX could
+       in theory be a problem, you can use Gnulib's ialloc module, or
+       functions like ximalloc in Gnulib's xalloc module.
+
    Why not use 'ptrdiff_t' directly?
 
      * Maintainability: When reading and modifying code, it helps to know that
-- 
2.30.2


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

* Re: [PATCH] base32, base64: prefer signed to unsigned integers
  2021-08-30 18:17       ` Paul Eggert
@ 2021-08-30 18:58         ` Simon Josefsson via Gnulib discussion list
  2021-09-04 10:06         ` autoupdate again Bruno Haible
  1 sibling, 0 replies; 17+ messages in thread
From: Simon Josefsson via Gnulib discussion list @ 2021-08-30 18:58 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Bruno Haible, Gnulib bugs

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

Paul Eggert <eggert@cs.ucla.edu> writes:

> Good question. Addressed in the attached patch, which I pushed.

Thanks for improving this -- it addresses my concerns.

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

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

* autoupdate again
  2021-08-30 18:17       ` Paul Eggert
  2021-08-30 18:58         ` Simon Josefsson via Gnulib discussion list
@ 2021-09-04 10:06         ` Bruno Haible
  2021-09-04 16:47           ` Paul Eggert
  1 sibling, 1 reply; 17+ messages in thread
From: Bruno Haible @ 2021-09-04 10:06 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

Paul Eggert wrote:
> Good question. Addressed in the attached patch, which I pushed.
> [0001-idx-add-commentary.patch]

Note that Karl's autoupdate reverted the patch.

Bruno





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

* Re: autoupdate again
  2021-09-04 10:06         ` autoupdate again Bruno Haible
@ 2021-09-04 16:47           ` Paul Eggert
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Eggert @ 2021-09-04 16:47 UTC (permalink / raw)
  To: Bruno Haible; +Cc: bug-gnulib

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

On 9/4/21 3:06 AM, Bruno Haible wrote:
> Karl's autoupdate reverted the patch.

Thanks, fixed via the attached. I plan to merge this stuff into glibc 
soon and to bring back the link then.

[-- Attachment #2: 0001-idx-break-copying-from-glibc.patch --]
[-- Type: text/x-patch, Size: 2602 bytes --]

From 298f08077b41078d5a5d9a2460bbb40f40512997 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 4 Sep 2021 09:45:06 -0700
Subject: [PATCH] idx: break copying from glibc

* config/srclist.txt: Comment out idx.h, and bring back recent change.
---
 ChangeLog          |  5 +++++
 config/srclist.txt |  2 +-
 lib/idx.h          | 20 ++++++++++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 0c18a58eb..39a892e28 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2021-09-04  Paul Eggert  <eggert@cs.ucla.edu>
+
+	idx: break copying from glibc
+	* config/srclist.txt: Comment out idx.h, and bring back recent change.
+
 2021-09-04  Sun Haiyong  <youbest@sina.com>  (tiny change)
 
 	sigsegv: Improve cross-compilation support for LoongArch CPU.
diff --git a/config/srclist.txt b/config/srclist.txt
index e118ede04..da906571c 100644
--- a/config/srclist.txt
+++ b/config/srclist.txt
@@ -50,7 +50,7 @@ $GNUORG Copyright/request-assign.program	doc/Copyright
 $GNUORG Copyright/request-disclaim.changes	doc/Copyright
 
 $LIBCSRC include/filename.h		lib
-$LIBCSRC include/idx.h			lib
+#$LIBCSRC include/idx.h			lib
 #$LIBCSRC malloc/dynarray-skeleton.c	lib/malloc
 #$LIBCSRC malloc/dynarray.h		lib/malloc
 #$LIBCSRC malloc/dynarray_at_failure.c	lib/malloc
diff --git a/lib/idx.h b/lib/idx.h
index 483587eab..54ad5d81f 100644
--- a/lib/idx.h
+++ b/lib/idx.h
@@ -56,6 +56,26 @@
      * Because 'size_t' is an unsigned type, and a signed type is better.
        See above.
 
+   Why not use 'ssize_t'?
+
+     * 'ptrdiff_t' is more portable; it is standardized by ISO C
+       whereas 'ssize_t' is standardized only by POSIX.
+
+     * 'ssize_t' is not required to be as wide as 'size_t', and some
+       now-obsolete POSIX platforms had 'size_t' wider than 'ssize_t'.
+
+     * Conversely, some now-obsolete platforms had 'ptrdiff_t' wider
+       than 'size_t', which can be a win and conforms to POSIX.
+
+   Won't this cause a problem with objects larger than PTRDIFF_MAX?
+
+     * Typical modern or large platforms do not allocate such objects,
+       so this is not much of a problem in practice; for example, you
+       can safely write 'idx_t len = strlen (s);'.  To port to older
+       small platforms where allocations larger than PTRDIFF_MAX could
+       in theory be a problem, you can use Gnulib's ialloc module, or
+       functions like ximalloc in Gnulib's xalloc module.
+
    Why not use 'ptrdiff_t' directly?
 
      * Maintainability: When reading and modifying code, it helps to know that
-- 
2.30.2


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

* Re: ialloc: relicense
  2021-08-29  7:49   ` Paul Eggert
@ 2021-09-19 22:07     ` Bruno Haible
  0 siblings, 0 replies; 17+ messages in thread
From: Bruno Haible @ 2021-09-19 22:07 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

Paul Eggert wrote:
> +2021-08-29  Paul Eggert  <eggert@cs.ucla.edu>
> +
> +	ialloc: relicense
> +	* modules/ialloc (License): Change from LGPL to LGPLv2+.
> +

Now that we systematically reflect the module's license in the source files,
this leads to a failure of 'check-copyright':

$ ./check-copyright
Module License    File License   File name
================= ============== =====================================
LGPLv2+           LGPL           lib/ialloc.c
LGPLv2+           LGPL           lib/ialloc.h

I'm updating it, using the template from etc/license-notices/LGPLv2+.


2021-09-19  Bruno Haible  <bruno@clisp.org>

	ialloc: Update license headers after license change from 2021-08-29.
	* lib/ialloc.h: Update license header.
	* lib/ialloc.c: Likewise.

diff --git a/lib/ialloc.c b/lib/ialloc.c
index f506b8402..013709954 100644
--- a/lib/ialloc.c
+++ b/lib/ialloc.c
@@ -4,7 +4,7 @@
 
    This file 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 3 of the
+   published by the Free Software Foundation; either version 2.1 of the
    License, or (at your option) any later version.
 
    This file is distributed in the hope that it will be useful,
diff --git a/lib/ialloc.h b/lib/ialloc.h
index 5ceda4633..d4f54ced8 100644
--- a/lib/ialloc.h
+++ b/lib/ialloc.h
@@ -4,7 +4,7 @@
 
    This file 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 3 of the
+   published by the Free Software Foundation; either version 2.1 of the
    License, or (at your option) any later version.
 
    This file is distributed in the hope that it will be useful,





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

end of thread, other threads:[~2021-09-19 22:07 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 22:27 [PATCH] base32, base64: prefer signed to unsigned integers Paul Eggert
2021-08-28 12:45 ` Bruno Haible
2021-08-28 14:12   ` Bruno Haible
2021-08-29  7:57     ` Paul Eggert
2021-08-29 10:16       ` Bruno Haible
2021-08-29 17:25         ` Paul Eggert
2021-08-29 18:36           ` Bruno Haible
2021-08-29 20:01             ` Paul Eggert
2021-08-29  7:49   ` Paul Eggert
2021-09-19 22:07     ` ialloc: relicense Bruno Haible
2021-08-29  8:20 ` [PATCH] base32, base64: prefer signed to unsigned integers Simon Josefsson via Gnulib discussion list
2021-08-29  9:56   ` Bruno Haible
2021-08-30 14:12     ` Simon Josefsson via Gnulib discussion list
2021-08-30 18:17       ` Paul Eggert
2021-08-30 18:58         ` Simon Josefsson via Gnulib discussion list
2021-09-04 10:06         ` autoupdate again Bruno Haible
2021-09-04 16:47           ` 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).