> -----Original Message----- > From: SeokYeon Hwang [mailto:syeon.hw...@samsung.com] > Sent: Wednesday, November 05, 2014 10:13 PM > To: 'Paolo Bonzini'; 'Max Reitz'; '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. > > > -----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.
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.