Re: [PATCH] pokefmt,poked: s/strerror/strerror_r/

[Jose E. Marchesi]

> `strerror' function is not thread-safe.  This commit replace them
> with POSIX-compliant `strerror_r' function.
>
> 2024-02-15  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>
>       * bootstrap.conf (gnulib_modules): s/strerror/strerror_r-posix/.
>       * pokefmt/pokefmt.l (err): Use `strerror_r'.
>       * poked/poked.c (err): Likewise.
>       * poked/usock.c (errorf): New function.
>       (usock_handle_srv): Use `errorf' to make the buffer.
>       (usock_handle_notif): Likewise.
>       (usock_serve): Likewise.
> ---
>
> Hi Jose.
>
> Please review the changes.  At the moment, I'm too tired to convince myself
> that the changes are correct :D
>
> Regards,
> Mohammad-Reza
>
>
>  ChangeLog         | 10 +++++++++
>  bootstrap.conf    |  2 +-
>  poked/poked.c     |  5 ++++-
>  poked/usock.c     | 52 +++++++++++++++++++++++++++++++++++++++--------
>  pokefmt/pokefmt.l |  5 ++++-
>  5 files changed, 63 insertions(+), 11 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index f101815d..414fb7fc 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,13 @@
> +2024-02-15  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +     * bootstrap.conf (gnulib_modules): s/strerror/strerror_r-posix/.
> +     * pokefmt/pokefmt.l (err): Use `strerror_r'.
> +     * poked/poked.c (err): Likewise.
> +     * poked/usock.c (errorf): New function.
> +     (usock_handle_srv): Use `errorf' to make the buffer.
> +     (usock_handle_notif): Likewise.
> +     (usock_serve): Likewise.
> +
>  2024-02-12  Jose E. Marchesi  <jemarch@gnu.org>
>  
>       * bootstrap.conf (gnulib_modules): Import regex module from
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 72f13d4a..30245a6f 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -49,7 +49,7 @@ gnulib_modules="
>    stdarg
>    stdbool
>    strchrnul
> -  strerror
> +  strerror_r-posix
>    streq
>    sys_ioctl
>    terminfo
> diff --git a/poked/poked.c b/poked/poked.c
> index 04120660..43deaf08 100644
> --- a/poked/poked.c
> +++ b/poked/poked.c
> @@ -51,11 +51,14 @@ static void
>  err(int eval, const char *fmt, ...)
>  {
>    va_list ap;
> +  char buf[128];
>  
>    va_start (ap, fmt);
>    vfprintf (stderr, fmt, ap);
>    va_end (ap);
> -  fprintf (stderr, ": %s\n", strerror (errno));
> +  buf[0] = '\0';
> +  strerror_r (errno, buf, sizeof (buf));
> +  fprintf (stderr, ": %s\n", buf);
>    exit (eval);
>  }
>  
> diff --git a/poked/usock.c b/poked/usock.c
> index f1d3dc62..822a5382 100644
> --- a/poked/usock.c
> +++ b/poked/usock.c
> @@ -130,6 +130,43 @@ write_n_bytes (int fd, unsigned char *data, size_t *len, 
> size_t cap)
>  
>  //---
>  
> +static void
> +errorf (int errnum, char *buf, size_t bufsz, const char *fmt, ...)
> +{
> +  int n;
> +  va_list ap;
> +
> +  assert (buf);
> +  assert (bufsz);
> +
> +  va_start (ap, fmt);
> +  n = vsnprintf (buf, bufsz, fmt, ap);
> +  va_end (ap);
> +
> +  assert (n > 0);
> +  /* Worst case in release build.  */
> +  if (n < 0)
> +    {
> +      buf[0] = '\0';
> +      n = 0;
> +    }

Why is this so complicated?  What is this function supposed to do?

> +  bufsz -= n;
> +  buf += n;
> +
> +  if (bufsz < 3)
> +    return;
> +  buf[0] = ':';
> +  buf[1] = ' ';
> +  buf[2] = '\0';
> +  bufsz -= 2;
> +  buf += 2;
> +
> +  strerror_r (errnum, buf, bufsz);
> +}
> +
> +//---
> +
>  // CHKME proper use
>  #define USOCK_DIR_UNK 0
>  #define USOCK_DIR_IN 1
> @@ -368,8 +405,8 @@ usock_handle_srv (struct usock *u, struct pollfd *pfds)
>              break;
>            if (errno == EINTR)
>              continue;
> -          snprintf (u->errbuf, USOCK_ERRBUF_SIZE, "[%s] accept() failed: %s",
> -                    __func__, strerror (errno));
> +          errorf (errno, u->errbuf, USOCK_ERRBUF_SIZE, "[%s] accept() 
> failed",
> +                  __func__);
>            return USOCK_HANDLE_SRV_NOK;
>          }
>        if (u->nclients == USOCK_CLIENTS_MAX)
> @@ -380,9 +417,8 @@ usock_handle_srv (struct usock *u, struct pollfd *pfds)
>          }
>        if (fcntl (fd, F_SETFL, O_NONBLOCK) == -1)
>          {
> -          snprintf (u->errbuf, USOCK_ERRBUF_SIZE,
> -                    "[%s] fcntl(%d, O_NONBLOCK) failed: %s", __func__, fd,
> -                    strerror (errno));
> +          errorf (errno, u->errbuf, USOCK_ERRBUF_SIZE,
> +                  "[%s] fcntl(%d, O_NONBLOCK) failed", __func__, fd);
>            return USOCK_HANDLE_SRV_NOK;
>          }
>        c = &u->clients[u->nclients];
> @@ -424,7 +460,8 @@ usock_handle_notif (struct usock *u, struct pollfd *pfds)
>          {
>            if (errno == EAGAIN || errno == EWOULDBLOCK)
>              break;
> -          snprintf (u->errbuf, USOCK_ERRBUF_SIZE, "read(pipefd[0]) failed");
> +          errorf (errno, u->errbuf, USOCK_ERRBUF_SIZE,
> +                  "read(pipefd[0]) failed");
>            return USOCK_HANDLE_NOTIF_NOK;
>          }
>      }
> @@ -573,8 +610,7 @@ usock_serve (struct usock *u)
>          {
>            if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK)
>              continue;
> -          snprintf (u->errbuf, USOCK_ERRBUF_SIZE, "poll() failed: %s",
> -                    strerror (errno));
> +          errorf (errno, u->errbuf, USOCK_ERRBUF_SIZE, "poll() failed");
>            ret = USOCK_SERVE_NOK;
>            break;
>          }
> diff --git a/pokefmt/pokefmt.l b/pokefmt/pokefmt.l
> index 69c854ae..7a5c81b9 100644
> --- a/pokefmt/pokefmt.l
> +++ b/pokefmt/pokefmt.l
> @@ -278,11 +278,14 @@ static void
>  err (int eval, const char *fmt, ...)
>  {
>    va_list ap;
> +  char buf[128];
>  
>    va_start (ap, fmt);
>    vfprintf (stderr, fmt, ap);
>    va_end (ap);
> -  fprintf (stderr, ": %s\n", strerror (errno));
> +  buf[0] = '\0';
> +  strerror_r (errno, buf, sizeof (buf));
> +  fprintf (stderr, ": %s\n", buf);
>    exit (eval);
>  }