On 07/28/2016 12:19 PM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> On 07/27/2016 03:24 AM, Sascha Silbe wrote: >>> C11 allows errno to be clobbered by pretty much any library function >>> call, so in general callers need to take care to save errno before >>> calling other functions. >>> >>> However, for error reporting functions this is rather awkward and can >>> make the code on the caller side more complicated than >>> necessary. error_setg_errno() already takes care of preserving errno >>> and some functions rely on that, so just promise that we continue to >>> do so in the future. >>> >>> Signed-off-by: Sascha Silbe <si...@linux.vnet.ibm.com> >>> --- >>> >>> Alternative approach to "error: error_setg_errno(): errno may be >>> clobbered" [1]. >> >> I like this alternative better. >> >> >>> +++ 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? > > I guess we should for the ones that preserve errno, to make that > preservation actually useful. These are: error_setv(), > error_setg_errno_internal(), error_append_hint(). Indirectly: > error_set_internal(), error_set(), error_setg_internal(), error_setg(), > error_setg_file_open_internal(), error_setg_file_open(), possibly > error_setg_win32_internal() and error_setg_win32(). >
The implementation of preserve errno seems inconsistent to me. The function error_setv is static, and I guess it is supposed to provide this indirect errno preservation and is used for both error_setg und error_setg_errno, yet error_setg_ errno_internal does extra save-restore itself while error_setg_iternal relies on 'indirect', what is not OK in my opinion. As Sascha pointed out, in C11 any library functions may change errno unless explicitly told otherwise for the particular function. Since start_va and end_va has nothing on preserving errno it is guaranteed by the standard that they persevere errno, and we should assume they don't. I could prepare a patch for this. Should I? Halil