> -----Original Message----- > From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo > Bonzini > Sent: Wednesday, November 05, 2014 9:45 PM > To: Max Reitz; SeokYeon Hwang; qemu-devel@nongnu.org > Cc: arm...@redhat.com; paolo.bonz...@gmail.com > 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.
