Re: [Qemu-devel] [PATCH] error: error_setg_errno(): errno gets preserved

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Sascha Silbe <address@hidden> writes:
>
>> Dear Eric,
>>
>> Eric Blake <address@hidden> writes:
>>
>>>> +++ b/include/qapi/error.h
>>>> @@ -170,6 +170,9 @@ void error_setg_internal(Error **errp,
>>>>   * Just like error_setg(), with @os_error info added to the message.
>>>>   * If @os_error is non-zero, ": " + strerror(os_error) is appended to
>>>>   * the human-readable error message.
>>>> + *
>>>> + * The value of errno (which usually can get clobbered by almost any
>>>> + * function call) will be preserved.
>>>>   */
>>>>  #define error_setg_errno(errp, os_error, fmt, ...)                      \
>>>>      error_setg_errno_internal((errp), __FILE__, __LINE__, __func__,     \
>>>
>>> Do we need/want to make the guarantee of preserving errno across any of
>>> the other functions and macros declared in error.h?
>>
>> It would be more consistent to have all error reporting functions
>> promise this, even if they do not get passed the errno. In some cases
>> the errno might not matter to the user (so error_setg_errno() isn't
>> used), but still be passed on to the caller to signal an error (so
>> clobbering it could be problematic).
>>
>> Can prepare a follow-up patch that makes sure error_setg(),
>> error_propagate(), error_setg_file_open(), error_set() preserve
>> errno. Optionally also the other functions listed in
>> include/qapi/error.h and include/qemu/error-report.h.
>
> Suggest:
>
> * A patch to document existing errno-preserving behavior.
>
> * Patches to reduce inconsistency, if any.  E.g. say all but one
>   error_setg() function preserve errno, make the exception preserve it,
>   too.
>
> * Optionally, patches to add more errno-preserving behavior you consider
>   useful.  I can't promise such patches will be applied, only that they
>   will be reviewed :)

Hmm, looks like I held onto your patch in the hope of getting a few
more, then forgot about it.  Since it still applies, I'll take it now.
Doesn't mean I've lost my hope for more of them :)