Re: [Qemu-devel] [PATCH] error: fixed error_set_errno() to deal with a negative type of os_error.

[Max Reitz]

On 2014-11-05 at 11:57, Paolo Bonzini wrote:
> On 05/11/2014 09:12, Max Reitz wrote:
> > On 2014-11-05 at 09:09, SeokYeon Hwang wrote:
> > > Negative type of errno like -ERRNO is used a lot by developers.
> > > Therefore, error_set_errno() is modified to deal with a negative type
> > > of os_error.
> > > (Negative type is used at pcie_cap_slot_hotplug_common() in
> > > hw/pci/pcie.c)
> > >
> > > Signed-off-by: SeokYeon Hwang <address@hidden>
> > > ---
> > >    util/error.c | 2 +-
> > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/util/error.c b/util/error.c
> > > index 2ace0d8..5db00c9 100644
> > > --- a/util/error.c
> > > +++ b/util/error.c
> > > @@ -68,7 +68,7 @@ void error_set_errno(Error **errp, int os_errno,
> > > ErrorClass err_class,
> > >        va_start(ap, fmt);
> > >        msg1 = g_strdup_vprintf(fmt, ap);
> > >        if (os_errno != 0) {
> > > -        err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno));
> > > +        err->msg = g_strdup_printf("%s: %s", msg1,
> > > strerror(abs(os_errno)));
> > >            g_free(msg1);
> > >        } else {
> > >            err->msg = msg1;
> > This is utterly broken and we should fix all callers instead.
> >
> > ...But I like it.
> I don't, we really should fix the callers.

Of course I understand, but this patch doesn't make matters worse, as 
long as there are not systems which have negative values for errno 
(which I think we generally assume not to exist throughout qemu). That's 
why I'm fine with it. We should fix the callers but I don't see why we 
shouldn't apply this patch as well.

A similar issue already came up and led to commit b276d2499, where 
callers of error_setg_errno() assumed that it would not clobber errno, 
so we fixed some of the callers but also applied that commit which just 
saves errno because there's no reason not to.

Max

> Paolo
>
> > Reviewed-by: Max Reitz <address@hidden>