Re: strsignal module

[Colin Watson]

On Wed, Jan 09, 2008 at 10:52:01AM +0100, Bruno Haible wrote:
> Colin Watson wrote:
> > The attached updated patches address your concerns and Paul's
> 
> Thanks; looks better now. Two points still:
> 
> - This code will not compile with C89 compilers on platforms without
>   threading, due to the semicolon:
> 
>   strsignal (int signum)
>   {
>     __libc_once_define (static, once);
>     const char *desc;
> 
>   Here I think you need to swap the two lines; then you can also remove the
>   semicolon in your definition of the macro __libc_once_define.
> 
> - In the init function, definition of init_sig, you don't need a
>   do { ... } while (0);
>   wrapper since it is a no-op. The comment could read:
> 
>   /* No need to use a do {} while (0) here since init_sig(...) must expand
>      to a complete statement.  (We cannot use the ISO C99 designated array
>      initializer syntax since it is not supported by ANSI C compilers and
>      since some signal numbers might exceed NSIG.)  */
> 
> Attached you find a proposed patch.

This all looks fine. Thanks.

> > I'd still like to know if anyone has any suggestions that avoid the
> > snprintf dependency.
> 
> The snprintf is needed here. For i18n reasons, the message
> _("Unknown signal %d") cannot be split; some *printf function is needed to
> executed the translated format string. Since the given buffer is statically
> sized, snprintf is the best match.
> 
> Here the return value of snprintf is not used, but I don't think it's worth
> creating another snprintf variant module that guarantees only the existence
> of the snprintf function but not its correct return value.

I agree. On the other hand, it just occurred to me that there's no real
reason to use the snprintf-posix module here; plain snprintf will do
just fine and is much more lightweight. With this change, the only new
dependent modules pulled into man-db by importing strsignal are memset
and tls.

I've attached an updated patch.

Thanks,

-- 
Colin Watson                                       address@hidden

gnulib.strsignal.diff
Description: Text Data