Re: [RFC PATCH v2 1/2] util: Add thread-safe qemu_strerror() function

["Alex Bennée" <alex.bennee@linaro.org>]

Yohei Kojima <y-koj@outlook.jp> writes:

> Add qemu_strerror() which follows the POSIX specification for
> strerror(). While strerror() is not guaranteed to be thread-safe, this
> function is thread-safe.
>
> This function is added to solve the following issue:
> https://gitlab.com/qemu-project/qemu/-/issues/416
>
> Signed-off-by: Yohei Kojima <y-koj@outlook.jp>
> ---
>  include/qemu/cutils.h | 20 +++++++++++++++++++
>  util/cutils.c         | 45 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
>
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index 92c436d8c7..0bcae0049a 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -117,6 +117,26 @@ int stristart(const char *str, const char *val, const char **ptr);
>   * Returns: length of @s in bytes, or @max_len, whichever is smaller.
>   */
>  int qemu_strnlen(const char *s, int max_len);
> +/**
> + * qemu_strerror:
> + * @errnum: error number
> + *
> + * Return the pointer to a string that describes errnum, like
> + * strerror(). This function is thread-safe because the buffer for
> + * returned string is allocated per thread.
> + *
> + * This function is thread-safe, but not reentrant. In other words,
> + * if a thread is interrupted by a signal in this function, and the
> + * thread calls this function again in the signal handling, then
> + * the result might be corrupted.
> + *
> + * This function has the same behaviour as the POSIX strerror()
> + * function.
> + *
> + * Returns: the pointer to an error description, or an
> + * "Unknown error nnn" message if errnum is invalid.
> + */
> +char *qemu_strerror(int errnum);
>  /**
>   * qemu_strsep:
>   * @input: pointer to string to parse
> diff --git a/util/cutils.c b/util/cutils.c
> index 5887e74414..3d14f50c75 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -131,6 +131,51 @@ int qemu_strnlen(const char *s, int max_len)
>      return i;
>  }
>  
> +/**
> + * It assumes the length of error descriptions are at most 1024.
> + * The concern of write buffer overflow is cared by strerror_r().
> + */
> +static __thread char qemu_strerror_buf[1024];
> +
> +#if (_POSIX_C_SOURCE >= 200112L) && ! _GNU_SOURCE
> +/**
> + * In POSIX.1-2001 and after, the return type of strerror_r is int, but
> + * glibc overrides the definition of strerror_r to the old strerror_r
> + * if _GNU_SOURCE is defined. This condition handles it.
> + */
> +
> +char *qemu_strerror(int errnum)
> +{
> +    int is_error = strerror_r(errnum, qemu_strerror_buf, 1024);
> +
> +    if (is_error == 0) return qemu_strerror_buf;

Please follow coding style rules. Checkpatch would have complained here.

> +
> +    /**
> +     * handle the error occured in strerror_r
> +     *
> +     * If is_error is greater than 0, errno might not be updated by
> +     * strerror_r. Otherwise, errno is updated.
> +     */
> +    if (is_error > 0) errno = is_error;

and here.

> +
> +    strncpy(qemu_strerror_buf, "Error %d occured\n", errno);
> +    return qemu_strerror_buf;
> +}
> +#else
> +/**
> + * In glibc, the return type of strerror_r is char* if _GNU_SOURCE
> + * is defined. In this case, strerror_r returns qemu_strerror_buf iff
> + * some error occured in strerror_r, and otherwise it returns a pointer
> + * to the pre-defined description for errnum.
> + *
> + * This is the same behaviour until POSIX.1-2001.
> + */
> +char *qemu_strerror(int errnum)
> +{
> +    return strerror_r(errnum, qemu_strerror_buf, 1024);
> +}
> +#endif
> +
>  char *qemu_strsep(char **input, const char *delim)
>  {
>      char *result = *input;


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro