From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53286) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmCPl-0002iv-8Z for qemu-devel@nongnu.org; Wed, 05 Nov 2014 21:03:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XmCPb-0002ZF-6N for qemu-devel@nongnu.org; Wed, 05 Nov 2014 21:03:09 -0500 Received: from mailout2.samsung.com ([203.254.224.25]:49465) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmCPa-0002Ya-T4 for qemu-devel@nongnu.org; Wed, 05 Nov 2014 21:02:59 -0500 Received: from epcpsbgm2.samsung.com (epcpsbgm2 [203.254.230.27]) by mailout2.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0NEL00INYHOWQD80@mailout2.samsung.com> for qemu-devel@nongnu.org; Thu, 06 Nov 2014 11:02:56 +0900 (KST) From: SeokYeon Hwang References: <1415174992-13246-1-git-send-email-syeon.hwang@samsung.com> <5459DBE1.30509@redhat.com> <545A0288.4010905@gnu.org> <545A05CC.4050109@redhat.com> <545A1BAE.4070802@gnu.org> In-reply-to: Date: Thu, 06 Nov 2014 11:02:58 +0900 Message-id: <045701cff965$ca09d990$5e1d8cb0$@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii Content-transfer-encoding: 7bit Content-language: ko Subject: Re: [Qemu-devel] [PATCH] error: fixed error_set_errno() to deal with a negative type of os_error. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Paolo Bonzini' , 'Max Reitz' , qemu-devel@nongnu.org Cc: Peter Maydell , paolo.bonzini@gmail.com, armbru@redhat.com > -----Original Message----- > From: SeokYeon Hwang [mailto:syeon.hwang@samsung.com] > Sent: Wednesday, November 05, 2014 10:13 PM > To: 'Paolo Bonzini'; 'Max Reitz'; 'qemu-devel@nongnu.org' > Cc: 'armbru@redhat.com'; 'paolo.bonzini@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.bonzini@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: armbru@redhat.com; paolo.bonzini@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.