| Message ID | 1469611466-31574-1-git-send-email-silbe@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Sascha Silbe <silbe@linux.vnet.ibm.com> writes: > 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 <silbe@linux.vnet.ibm.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
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 <silbe@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?
Eric Blake <eblake@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 <silbe@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().
Dear Eric, Eric Blake <eblake@redhat.com> 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. Sascha
On 07/28/2016 12:19 PM, Markus Armbruster wrote: > Eric Blake <eblake@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 <silbe@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
On 07/28/2016 04:46 AM, Halil Pasic wrote: > 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 long as errno gets saved where it is documented as saved, I don't care whether it is direct or indirect (indirect is probably more efficient, where we can prove that nothing is called that is allowed to clobber errno). > > 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. You mean va_start, not start_va. And actually, C11 is clear that errno is unspecified after library functions (but not macros) that don't explicitly state otherwise. Since va_start() is a macro and not a library function, that means va_start does NOT have carte blanche permission to modify errno. For more reading on the topic: http://austingroupbugs.net/view.php?id=384 There are several related POSIX bug reports of other functions that have been requested to explicitly document that they don't modify errno, and I'm happy to submit even more, if we find other standard interfaces whose semantics are easier when they guarantee that errno is not clobbered.
On 07/28/2016 04:56 PM, Eric Blake wrote: > On 07/28/2016 04:46 AM, Halil Pasic wrote: > >> 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 long as errno gets saved where it is documented as saved, I don't > care whether it is direct or indirect (indirect is probably more > efficient, where we can prove that nothing is called that is allowed to > clobber errno). > It is still inconsistent (error_setg_errno_internal and error_setg_iternal). If it's ok for error_setg_internal to omit saving errno before calling va_start then there is no reason to do it in error_setg_errno_internal. >> >> 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. > > You mean va_start, not start_va. And actually, C11 is clear that errno > is unspecified after library functions (but not macros) that don't > explicitly state otherwise. Since va_start() is a macro and not a > library function, that means va_start does NOT have carte blanche > permission to modify errno. For more reading on the topic: I also considered this function/macro thing but in the end I am not aware of anything in C11 what would prohibit va_start to modify errno -- correct me if I'm wrong. With that it boils down to 'may' and relying on 'does not' means you are not covered by the standard C11 (but may be covered by something else -- in which case this should be documented in HACKING). > > http://austingroupbugs.net/view.php?id=384 > This got rejected, or? Means that there is no willingness to introduce this guarantee at POSIX level? > There are several related POSIX bug reports of other functions that have > been requested to explicitly document that they don't modify errno, and > I'm happy to submit even more, if we find other standard interfaces > whose semantics are easier when they guarantee that errno is not clobbered. > Regards, Halil
On 07/28/2016 09:29 AM, Halil Pasic wrote: >> You mean va_start, not start_va. And actually, C11 is clear that errno >> is unspecified after library functions (but not macros) that don't >> explicitly state otherwise. Since va_start() is a macro and not a >> library function, that means va_start does NOT have carte blanche >> permission to modify errno. For more reading on the topic: > > I also considered this function/macro thing but in the end I am not > aware of anything in C11 what would prohibit va_start to modify errno -- > correct me if I'm wrong. With that it boils down to 'may' and relying on > 'does not' means you are not covered by the standard C11 (but may > be covered by something else -- in which case this should be documented > in HACKING). > >> >> http://austingroupbugs.net/view.php?id=384 >> > > This got rejected, or? Means that there is no willingness to introduce > this guarantee at POSIX level? > That particular bug report was rejected because the POSIX folks decided that the C11 wording was clear enough that va_start() was already guaranteed to not mess with errno, so no additionally wording was needed in POSIX.
On 07/28/2016 11:03 PM, Eric Blake wrote: > On 07/28/2016 09:29 AM, Halil Pasic wrote: > >>> You mean va_start, not start_va. And actually, C11 is clear that errno >>> is unspecified after library functions (but not macros) that don't >>> explicitly state otherwise. Since va_start() is a macro and not a >>> library function, that means va_start does NOT have carte blanche >>> permission to modify errno. For more reading on the topic: >> >> I also considered this function/macro thing but in the end I am not >> aware of anything in C11 what would prohibit va_start to modify errno -- >> correct me if I'm wrong. With that it boils down to 'may' and relying on >> 'does not' means you are not covered by the standard C11 (but may >> be covered by something else -- in which case this should be documented >> in HACKING). >> >>> >>> http://austingroupbugs.net/view.php?id=384 >>> >> >> This got rejected, or? Means that there is no willingness to introduce >> this guarantee at POSIX level? >> > > That particular bug report was rejected because the POSIX folks decided > that the C11 wording was clear enough that va_start() was already > guaranteed to not mess with errno, so no additionally wording was needed > in POSIX. > Sadly, I still do not get it. I have re-read the relevant parts of N1570 and even had a conversation with the in house compiler team. The compiler guy's opinion was also that there is no guarantee provided by C11. In http://austingroupbugs.net/view.php?id=384 you stated in the description that the code example provided there is not conforming. Your last reply I read like you were wrong with that statement. I still do not understand why were you wrong there. In fact, I could argue that you were right, but I'm afraid the argument would be somewhat lengthy and confusing, and I'm already feeling bad about taking so much of your time with this. Since I'm admittedly quite inexperienced in this field I decided to just accept your the conclusion you and the POSIX guys reached -- without fully understanding it. Thanks again for your time. Regards, Halil
Sascha Silbe <silbe@linux.vnet.ibm.com> writes: > Dear Eric, > > Eric Blake <eblake@redhat.com> 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 :)
Markus Armbruster <armbru@redhat.com> writes: > Sascha Silbe <silbe@linux.vnet.ibm.com> writes: > >> Dear Eric, >> >> Eric Blake <eblake@redhat.com> 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 :)
On 07/29/2016 08:38 AM, Halil Pasic wrote: > > > On 07/28/2016 11:03 PM, Eric Blake wrote: >> On 07/28/2016 09:29 AM, Halil Pasic wrote: >> >>>> You mean va_start, not start_va. And actually, C11 is clear that errno >>>> is unspecified after library functions (but not macros) that don't >>>> explicitly state otherwise. Since va_start() is a macro and not a >>>> library function, that means va_start does NOT have carte blanche >>>> permission to modify errno. For more reading on the topic: >>> >>> I also considered this function/macro thing but in the end I am not >>> aware of anything in C11 what would prohibit va_start to modify errno -- >>> correct me if I'm wrong. With that it boils down to 'may' and relying on >>> 'does not' means you are not covered by the standard C11 (but may >>> be covered by something else -- in which case this should be documented >>> in HACKING). >>> >>>> >>>> http://austingroupbugs.net/view.php?id=384 >>>> >>> >>> This got rejected, or? Means that there is no willingness to introduce >>> this guarantee at POSIX level? >>> >> >> That particular bug report was rejected because the POSIX folks decided >> that the C11 wording was clear enough that va_start() was already >> guaranteed to not mess with errno, so no additionally wording was needed >> in POSIX. >> > > Sadly, I still do not get it. I have re-read the relevant parts of N1570 > and even had a conversation with the in house compiler team. The > compiler guy's opinion was also that there is no guarantee provided by > C11. In http://austingroupbugs.net/view.php?id=384 you stated in the > description that the code example provided there is not conforming. The description was the initial claim, before I had consulted with the rest of the Austin Group. Yes, my initial claim was that POSIX needed tightening to guarantee something not provided by C. > Your > last reply I read like you were wrong with that statement. Correct - after consultation with the full Austin Group, my initial claim was invalidated, and the reason it was invalidated was that the C99 standard only permits arbitrary changes to errno after function calls, and that the C99 definition of a function call does NOT include macro expansions. Therefore, POSIX does not need tightening, because the guarantee I wanted is already present; the initial description in that bug report is well-defined, rather than my claim of undefined. Any compliant C implementation, and therefore all POSIX-compliant implementations, already leave errno unchanged after any use of the varags macros. (The current version of POSIX is still stuck on C99, although the Austin Group is just barely starting work to incorporate C11 for the next version of POSIX.) > I still do > not understand why were you wrong there. In fact, I could argue that you > were right, but I'm afraid the argument would be somewhat lengthy and > confusing, and I'm already feeling bad about taking so much of your time > with this. Since I'm admittedly quite inexperienced in this field I > decided to just accept your the conclusion you and the POSIX guys > reached -- without fully understanding it. The C99 standard is annoying in that it does not use the usual RFC wording, so where C99 uses "may", many other standards (including POSIX) use "shall" or even "shall only". So the fact that C99 states that "The value of errno may be set to nonzero by a library function call" is a requirement that C can permit arbitrary modification of errno ONLY after a function call, and not for any other reason (including after a macro expansion if that macro does not expand to a documented function call). va_start() is usually not implemented as a function call, and even if it is, it is not a publicly documented function call. But you are certainly welcome to add further comments to the Austin Group bug, if you think anything was misinterpreted - in the end, the intent IS that va_* are safe to use without arbitrary changes to errno, and it is now just a matter of whether that intent is already met by C wording or whether POSIX indeed needs to add an additional requirement.
On 01/09/2017 03:50 PM, Eric Blake wrote: > On 07/29/2016 08:38 AM, Halil Pasic wrote: >> >> >> On 07/28/2016 11:03 PM, Eric Blake wrote: >>> On 07/28/2016 09:29 AM, Halil Pasic wrote: >>> >>>>> You mean va_start, not start_va. And actually, C11 is clear that errno >>>>> is unspecified after library functions (but not macros) that don't >>>>> explicitly state otherwise. Since va_start() is a macro and not a >>>>> library function, that means va_start does NOT have carte blanche >>>>> permission to modify errno. For more reading on the topic: >>>> >>>> I also considered this function/macro thing but in the end I am not >>>> aware of anything in C11 what would prohibit va_start to modify errno -- >>>> correct me if I'm wrong. With that it boils down to 'may' and relying on >>>> 'does not' means you are not covered by the standard C11 (but may >>>> be covered by something else -- in which case this should be documented >>>> in HACKING). >>>> >>>>> >>>>> http://austingroupbugs.net/view.php?id=384 >>>>> >>>> >>>> This got rejected, or? Means that there is no willingness to introduce >>>> this guarantee at POSIX level? >>>> >>> >>> That particular bug report was rejected because the POSIX folks decided >>> that the C11 wording was clear enough that va_start() was already >>> guaranteed to not mess with errno, so no additionally wording was needed >>> in POSIX. >>> >> >> Sadly, I still do not get it. I have re-read the relevant parts of N1570 >> and even had a conversation with the in house compiler team. The >> compiler guy's opinion was also that there is no guarantee provided by >> C11. In http://austingroupbugs.net/view.php?id=384 you stated in the >> description that the code example provided there is not conforming. > > The description was the initial claim, before I had consulted with the > rest of the Austin Group. Yes, my initial claim was that POSIX needed > tightening to guarantee something not provided by C. > >> Your >> last reply I read like you were wrong with that statement. > > Correct - after consultation with the full Austin Group, my initial > claim was invalidated, and the reason it was invalidated was that the > C99 standard only permits arbitrary changes to errno after function > calls, and that the C99 definition of a function call does NOT include > macro expansions. Therefore, POSIX does not need tightening, because > the guarantee I wanted is already present; the initial description in > that bug report is well-defined, rather than my claim of undefined. > Any compliant C implementation, and therefore all POSIX-compliant > implementations, already leave errno unchanged after any use of the > varags macros. (The current version of POSIX is still stuck on C99, > although the Austin Group is just barely starting work to incorporate > C11 for the next version of POSIX.) > >> I still do >> not understand why were you wrong there. In fact, I could argue that you >> were right, but I'm afraid the argument would be somewhat lengthy and >> confusing, and I'm already feeling bad about taking so much of your time >> with this. Since I'm admittedly quite inexperienced in this field I >> decided to just accept your the conclusion you and the POSIX guys >> reached -- without fully understanding it. > > The C99 standard is annoying in that it does not use the usual RFC > wording, so where C99 uses "may", many other standards (including POSIX) > use "shall" or even "shall only". So the fact that C99 states that "The > value of errno may be set to nonzero by a library function call" is a > requirement that C can permit arbitrary modification of errno ONLY after > a function call, and not for any other reason (including after a macro Thanks for the clarification. As a non-native speaker I find that usage of "may" highly non-intuitive. Especially since in chapter 4. "Conformance" (from n1124) does define how "shall" and "shall not" but there is nothing on "may". This way of saying macros expand to stuff that does not touch errno is IMHO quite unfriendly (if this was really the intention - I think it is quite likely that it was), and IMHO a more straight forward formulation would benefit the standard. > expansion if that macro does not expand to a documented function call). It's clear that macro expansion itself does not modify program state, so the question is to what is a library macro allowed to expand to. > va_start() is usually not implemented as a function call, and even if it > is, it is not a publicly documented function call. IMHO whether it is implemented as a function (call) or a macro is irrelevant here. Apparently library functions may be additionally implemented as a macro, and library macros may be implemented as functions (7.2 "The assert macro shall be implemented as a macro, not as an actual function"). Library macros can expand to stuff calling documented library functions (7.2.1.1 "It then calls the abort function."), but this is also irrelevant if we interpret that "may" as you explained. > > But you are certainly welcome to add further comments to the Austin > Group bug, if you think anything was misinterpreted - in the end, the > intent IS that va_* are safe to use without arbitrary changes to errno, > and it is now just a matter of whether that intent is already met by C > wording or whether POSIX indeed needs to add an additional requirement. > Thank you very much for making your point clear. I take away: "The value of errno may be set to nonzero by a library function call" also means/implies 'use of any library entity, which was not specified as a library function, shall not set errno to nonzero'. This really helps me a lot because it answers the question which part of the standard prohibits the va_* macros from clobbering errno. I see this primarily as a C ISO standard problem, so I'm reluctant to necro-bump that bug in order to start a discussion about how the C standard is to be interpreted. I'm going to ask some friends if it is only me who finds it difficult to read that sentence as you propose. Best Regards, Halil
On 01/09/2017 12:27 PM, Halil Pasic wrote: >>> I still do >>> not understand why were you wrong there. In fact, I could argue that you >>> were right, but I'm afraid the argument would be somewhat lengthy and >>> confusing, and I'm already feeling bad about taking so much of your time >>> with this. Since I'm admittedly quite inexperienced in this field I >>> decided to just accept your the conclusion you and the POSIX guys >>> reached -- without fully understanding it. >> >> The C99 standard is annoying in that it does not use the usual RFC >> wording, so where C99 uses "may", many other standards (including POSIX) >> use "shall" or even "shall only". So the fact that C99 states that "The >> value of errno may be set to nonzero by a library function call" is a >> requirement that C can permit arbitrary modification of errno ONLY after >> a function call, and not for any other reason (including after a macro > > Thanks for the clarification. As a non-native speaker I find that usage > of "may" highly non-intuitive. Especially since in chapter 4. > "Conformance" (from n1124) does define how "shall" and "shall not" but > there is nothing on "may". > > This way of saying macros expand to stuff that does not touch errno is > IMHO quite unfriendly (if this was really the intention - I think it is > quite likely that it was), and IMHO a more straight forward formulation > would benefit the standard. Sadly, I'm not responsible for the wording in the C standard; I also find it confusing sometimes, even as a native speaker. > > Thank you very much for making your point clear. I take away: "The value > of errno may be set to nonzero by a library function call" also > means/implies 'use of any library entity, which was not specified as a > library function, shall not set errno to nonzero'. This really helps me > a lot because it answers the question which part of the standard > prohibits the va_* macros from clobbering errno. Or better: "Use of any library entity which was not specified as a library function shall not modify errno". While most interfaces in the library are functions (because they have required linkage) and may also be a macro, there are some (like assert() and the va_* macros) which are explicitly documented to be macros only. > > I see this primarily as a C ISO standard problem, so I'm reluctant to > necro-bump that bug in order to start a discussion about how the C > standard is to be interpreted. I'm going to ask some friends if it is > only me who finds it difficult to read that sentence as you propose. Yes, you are probably right that raising an issue with the C authors may be the best path forward on this topic, as it is certainly getting beyond the bounds of what qemu cares about.
diff --git a/include/qapi/error.h b/include/qapi/error.h index 0576659..7e532d0 100644 --- a/include/qapi/error.h +++ 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__, \
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 <silbe@linux.vnet.ibm.com> --- Alternative approach to "error: error_setg_errno(): errno may be clobbered" [1]. [1] mid:1469558699-23314-1-git-send-email-silbe@linux.vnet.ibm.com "[Qemu-devel] [PATCH] error: error_setg_errno(): errno may be clobbered" by Sascha Silbe <silbe@linux.vnet.ibm.com>, sent on 2016-07-26. include/qapi/error.h | 3 +++ 1 file changed, 3 insertions(+)