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

[SeokYeon Hwang]

> -----Original Message-----
> From: SeokYeon Hwang [mailto:address@hidden
> Sent: Wednesday, November 05, 2014 10:13 PM
> To: 'Paolo Bonzini'; 'Max Reitz'; 'address@hidden'
> Cc: 'address@hidden'; 'address@hidden'
> Subject: RE: [PATCH] error: fixed error_set_errno() to deal with a
> negative type of os_error.
> 
> > -----Original Message-----
> > From: Paolo Bonzini [mailto:address@hidden On Behalf Of
> > Paolo Bonzini
> > Sent: Wednesday, November 05, 2014 9:45 PM
> > To: Max Reitz; SeokYeon Hwang; address@hidden
> > Cc: address@hidden; address@hidden
> > Subject: Re: [PATCH] error: fixed error_set_errno() to deal with a
> > negative type of os_error.
> >
> >
> >
> > On 05/11/2014 12:11, Max Reitz wrote:
> > >
> > > 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.
> >
> > I think side effect are a different matter than misuse of QEMU.
> >
> > There are "only" 157 calls to error_setg_errno; 67 use "errno" as the
> > argument, and 4 use an explicit errno value (one of them is the wrong
> > - EBUSY).  The other 86 seem correct and should not be hard to audit.
> >
> > Let's instead add an assertion check to error_setg_errno.
> >
> > Paolo
> 
> I have expected to come out several opinions about this patch.
> 
> The use of negative errno on "strerror()" was obviously wrong. But that
> does not mean it is wrong to use the negative errno on
"error_set_errno()".
> The reason that I chose this one among the solutions is to change function
> specification. I think it seems good to us to respect the tradition of the
> developers that use negative errno.
> 
> But if error_set_errno() has strict specification - so, we must not change
> it's spec - I agree with Paolo's opinion.

I think we have 2 options.

1. "error_set_errno()" is just utility for developer's convenience.
Why can't we supply more convenience to developer ??
-> My first opinion.

2. It is not just utility function for convenience or we cannot change
its spec because it is well-known function.
-> If this is right, I'm ready to post 2nd patch that applied Paolo's
opinion.

What do you think about it??
Thanks.