qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH 5/7] error reporting: Provide error_report_e


From: Ian Jackson
Subject: Re: [Qemu-devel] [RFC PATCH 5/7] error reporting: Provide error_report_errnoval (and error_vreport_errnoval)
Date: Thu, 26 Apr 2018 18:42:27 +0100

Eric Blake writes ("Re: [RFC PATCH 5/7] error reporting: Provide 
error_report_errnoval (and error_vreport_errnoval)"):
> On 04/26/2018 11:53 AM, Ian Jackson wrote:
> > I have chosen to provide all of
> >    error_report_errno       error_vreport_errno
> >    error_report_errnoval    error_vreport_errnoval
> > because the former are much more common, and deserve a short spelling;
> > whereas there are still at least 30-40 potential callers of the latter.
> 
> As mentioned in 2/7, that's inconsistent with error_setg_errno().

I didn't see error_setg_errno().  But I don't mind your alternative
approach, with the explicit errno, if that's what people prefer.  I
agree about the names of the functions, in that case.

> > +void error_vreport_errnoval(int errnoval,
> > +                            const char *fmt, va_list ap) GCC_FMT_ATTR(2, 
> > 0);
> > +void error_report_errnoval(int errnoval,
> > +                           const char *fmt, ...) GCC_FMT_ATTR(2, 3);
> 
> Bikeshedding - can we name the parameter 'os_error' (as in
> error_seg_errno), or 'err' or 'errval', rather than the longer 'errnoval'?

In speech, one always refers to these things as `errno values'.
`os_error' and `errnoval' are the same length of course.  See my other
mail about this particular bikeshed.

> Should this explicitly document that passing 0 for errnoval is
> acceptable or forbidden?  If acceptable, does that mean no suffix (other
> than \n) is added?  If forbidden, do we want to assert() that?

My intend was that passing 0 for errnoval is exactly as permitted or
forbidden as passing 0 to strerror.  I think that this is neatly
implied by the specification:

> > + * Prepend the current location and append ": " strerror(errnoval) "\n".

These functions already have a problem with excessive boilerplate in
their language.  I'm loathe to make that worse by being other than
terse.  I wouldn't mind a note somewhere else.

I would be open to asserting that the value is not negative, because
we do have some of the confusing -errno convention in some of qemu.
OTOH that turns a mishandling of -errno on an error path from a wrong
error message into a crash which is a bit unfortunate.

Ian.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]