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

[Max Reitz <mreitz@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 <syeon.hwang@samsung.com>
>>> ---
>>>    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 <mreitz@redhat.com>