Re: [PATCH] tools/9pfsd: Fix build error caused by strerror_r

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.03.2024 06:54, Henry Wang wrote:
> Below error can be seen when doing Yocto build of the toolstack:
> 
> | io.c: In function 'p9_error':
> | io.c:684:5: error: ignoring return value of 'strerror_r' declared
>   with attribute 'warn_unused_result' [-Werror=unused-result]
> |   684 |     strerror_r(err, ring->buffer, ring->ring_size);
> |       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | cc1: all warnings being treated as errors
> 
> Fix it by adding a return value and check it properly.
> 
> Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> ---
>  tools/9pfsd/io.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/9pfsd/io.c b/tools/9pfsd/io.c
> index adb887c7d9..163eee6ecc 100644
> --- a/tools/9pfsd/io.c
> +++ b/tools/9pfsd/io.c
> @@ -681,11 +681,17 @@ static void p9_error(struct ring *ring, uint16_t tag, 
> uint32_t err)
>  {
>      unsigned int erroff;
>  
> -    strerror_r(err, ring->buffer, ring->ring_size);
> -    erroff = add_string(ring, ring->buffer, strlen(ring->buffer));
> -    fill_buffer(ring, P9_CMD_ERROR, tag, "SU",
> -                erroff != ~0 ? ring->str + erroff : "cannot allocate memory",
> -                &err);
> +    char *ret = strerror_r(err, ring->buffer, ring->ring_size);
> +
> +    if ( ret )

While you're checking the return value, this looks to be the only use of
"ret". Which looks wrong for the GNU form of the function. But an up front
question is whether we can expect all host OSes / libc implementations to
actually support the GNU form. Using _GNU_SOURCE may, after all, then not
be appropriate for a component using this function. Jürgen, thoughts?

> +    {
> +        erroff = add_string(ring, ring->buffer, strlen(ring->buffer));
> +        fill_buffer(ring, P9_CMD_ERROR, tag, "SU",
> +                    erroff != ~0 ?
> +                             ring->str + erroff :
> +                             "cannot allocate memory",

Indentation looks questionable to me here, albeit tools/ may have different
preferences than xen/.

Jan

> +                    &err);
> +    }
>  }
>  
>  static void p9_version(struct ring *ring, struct p9_header *hdr)