From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50212) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XlyUj-0004bM-Ue for qemu-devel@nongnu.org; Wed, 05 Nov 2014 06:11:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XlyUf-0003TM-3S for qemu-devel@nongnu.org; Wed, 05 Nov 2014 06:11:21 -0500 Message-ID: <545A05CC.4050109@redhat.com> Date: Wed, 05 Nov 2014 12:11:08 +0100 From: Max Reitz MIME-Version: 1.0 References: <1415174992-13246-1-git-send-email-syeon.hwang@samsung.com> <5459DBE1.30509@redhat.com> <545A0288.4010905@gnu.org> In-Reply-To: <545A0288.4010905@gnu.org> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit 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 , SeokYeon Hwang , qemu-devel@nongnu.org Cc: paolo.bonzini@gmail.com, armbru@redhat.com 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 >>> --- >>> 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