Re: [Qemu-devel] [RFC PATCH 2/7] error reporting: Provide error_report_errno (and error_vreport_errno)

[Eric Blake]

On 04/26/2018 11:53 AM, Ian Jackson wrote:
> This will let us replace a lot of open coded calls to perror,
> strerror, etc.
> 
> No callers yet so no functional change.
> 
> Signed-off-by: Ian Jackson <address@hidden>
> ---
>  include/qemu/error-report.h |  2 ++
>  util/qemu-error.c           | 28 ++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> index e1c8ae1..2b29678 100644
> --- a/include/qemu/error-report.h
> +++ b/include/qemu/error-report.h
> @@ -37,10 +37,12 @@ void error_printf_unless_qmp(const char *fmt, ...) 
> GCC_FMT_ATTR(1, 2);
>  void error_set_progname(const char *argv0);
>  
>  void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
> +void error_vreport_errno(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);

I'd rather us ALWAYS require an explicit argument for the error value,
rather than reading errno.  That way, it's easier to audit that the
caller is passing in a known error value, rather than checking if errno
has been inadvertently changed during intermediate code between when the
error was detected and when the message is generated.  Furthermore, this
is inconsistent with the existing error_setg_errno().

Thus, I do not think we want this function.  Your addition in 5/7 is
better, but a rather verbose name, so I think THAT addition deserves to
be named error_vreport_errno.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature