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

[Jose E. Marchesi]

> On Mon, Feb 19, 2024 at 03:38:50PM +0100, Jose E. Marchesi wrote:
>> >  
>> > +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?
>> 
>
>
> What `err' function does!

What is the meaning of "Worst case in release build" above?  In what
cases vsnprintf is expected to fail?

Note you have an

  assert (n > 0);

followed by:

  if (n < 0)
    {
      [...]
    }

> First format the string in the `buf', and ...
>
>
>> > +  bufsz -= n;
>> > +  buf += n;

It seems to me that you know the size of buf at compile time:
USOCK_ERRBUF_SIZE.  You don't need it passed as an argument to errorf.

If you make errorf to get as an argument a struct usock * instead of
buffer + buffer size, it seems to me you can simplify a lot the whole
thing.

>> > +  if (bufsz < 3)
>> > +    return;
>> > +  buf[0] = ':';
>> > +  buf[1] = ' ';
>> > +  buf[2] = '\0';
>> > +  bufsz -= 2;
>> > +  buf += 2;
>> > +
>> > +  strerror_r (errnum, buf, bufsz);
>> > +}
>> > +
>
>
> at the end adds a ": " string plus the textual description of the `errno'.
>
> Yes, more comments can make it more helpful :)