[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v2 1/3] EFI: address a violation of MISRA C Rule 13.6
On 08.10.2024 14:49, Roberto Bagnara wrote: > On 2024-10-08 07:59, Jan Beulich wrote: >> On 02.10.2024 08:54, Roberto Bagnara wrote: >>> On 2024-10-02 08:09, Jan Beulich wrote: >>>> On 01.10.2024 23:36, Stefano Stabellini wrote: >>>>> On Tue, 1 Oct 2024, Jan Beulich wrote: >>>>>> On 01.10.2024 07:25, Roberto Bagnara wrote: >>>>>>> On 2024-09-30 15:07, Jan Beulich wrote: >>>>>>>> On 30.09.2024 14:49, Federico Serafini wrote: >>>>>>>>> guest_handle_ok()'s expansion contains a sizeof() involving its >>>>>>>>> first argument which is guest_handle_cast(). >>>>>>>>> The expansion of the latter, in turn, contains a variable >>>>>>>>> initialization. >>>>>>>>> >>>>>>>>> Since MISRA considers the initialization (even of a local variable) >>>>>>>>> a side effect, the chain of expansions mentioned above violates >>>>>>>>> MISRA C:2012 Rule 13.6 (The operand of the `sizeof' operator shall not >>>>>>>>> contain any expression which has potential side effect). >>>>>>>> >>>>>>>> I'm afraid I need to ask for clarification of terminology and alike >>>>>>>> here. >>>>>>>> While the Misra doc has a section on Persistent Side Effects in its >>>>>>>> Glossary appendix, what constitutes a side effect from its pov isn't >>>>>>>> really spelled out anywhere. Which in turn raises the question whether >>>>>>>> it >>>>>>>> is indeed Misra (and not just Eclair) which deems initialization a side >>>>>>>> effect. This is even more so relevant as 13.6 talks of only >>>>>>>> expressions, >>>>>>>> yet initializers fall under declarations (in turn involving an >>>>>>>> expression >>>>>>>> on the rhs of the equal sign). >>>>>>>> >>>>>>>> All the same of course affects patch 2 then, too. >>>>>>> >>>>>>> MISRA C leaves the definition of "side effect" to the C Standard. >>>>>>> E.g., C18 5.1.2.3p2: >>>>>>> >>>>>>> Accessing a volatile object, modifying an object, modifying a file, >>>>>>> or calling a function that does any of those operations are all >>>>>>> side effects,[omitted irrelevant footnote reference] which are >>>>>>> changes in the state of the execution environment. >>>>>>> >>>>>>> The MISRA C:2012/2023 Glossary entry for "Persistent side effect" >>>>>>> indirectly confirms that initialization is always a side effect. >>>>>> >>>>>> Hmm, that's interesting: There's indeed an example with an initializer >>>>>> there. Yet to me the text you quote from the C standard does not say >>>>>> that initialization is a side effect - it would be "modifying an >>>>>> object" aiui, yet ahead of initialization being complete the object >>>>>> doesn't "exist" imo, and hence can be "modified" only afterwards. >>>>> >>>>> I feel it's becoming a bit too philosophical. Since there's some room >>>>> for interpretation and only two violations left to address, I believe >>>>> it's best to stick with the stricter interpretation of the definition. >>>>> Therefore, I'd proceed with this series in its current form. >>>> >>>> Proceeding with the series in its current form may be okay (as you say, >>>> you view the changes as readability improvements anyway), but imo the >>>> interpretation needs settling on no matter what. In fact even for these >>>> two patches it may affect what their descriptions ought to say (would >>>> be nice imo to avoid permanently recording potentially misleading >>>> information by committing as is). And of course clarity would help >>>> dealing with future instances that might appear. I take it you realize >>>> that if someone had submitted a patch adding code similar to the >>>> original forms of what's being altered here, it would be relatively >>>> unlikely for a reviewer to spot the issue. IOW here we're making >>>> ourselves heavily dependent upon Eclair spotting (supposed) issues, >>>> adding extra work and delays for such changes to go in. >>> >>> You can do two things to obtain a second opinion: >>> >>> 1) Use the MISRA forum (here is the link to the forum >>> section devoted to the side-effect rules of MISRA C:2012 >>> and MISRA C:2023 (https://forum.misra.org.uk/forumdisplay.php?fid=168). >>> The MISRA C Working Group will, in due course, provide >>> you with an official answer to your questions about what, >>> for the interpretation of Rule 13.6, has to be considered >>> a side effect. >>> >>> 2) Reach out to your ISO National Body and try to obtain >>> an official answer from ISO/IEC JTC1/SC22/WG14 (the >>> international standardization working group for the >>> programming language C) to your questions about what the >>> C Standard considers to be side effects. >> >> I took the latter route, and to my (positive) surprise I got back an answer >> the same day. There was a request for someone to confirm, but so far I didn't >> see further replies. Since this is a German institution I raised the question >> in German and got back an answer in German (attached); I've tried my best to >> translate this without falsifying anything, but I've omitted non-technical >> parts: >> >> "Initialization of an object is never a side effect of the initialization >> by itself. Of course expressions used for initialization can themselves have >> side effects on other objects. >> >> @Andreas: Do you agree? C after all has a far simpler object model than C++. >> The (potential) change in object representation (i.e. the bytes underlying >> the object) is not a side effect of object initialization, but its primary >> purpose." >> >> Further for Misra she added a reference to a Swiss person, but I think with >> Bugseng we have sufficient expertise there. > > Unfortunately, the (translation of the) answer you received adds > confusion to previous confusion: who answered has highlighted the > "side" part of the term, which is indeed relevant in computer science, > but not for the C standard. I can't see any highlighting in the original reply I received. > To the point that the same argument could > be used to deny that ++i has a side effect because the increment is > the "primary" effect... Well, if it's just "++i;" there's no side effect, just a primary one. In "n = ++i + j--;" there are side effects (the increment and decrement). > Part of the confusion is maybe in the the following paragraph Jan > wrote earlier: > > > Hmm, that's interesting: There's indeed an example with an initializer > > there. Yet to me the text you quote from the C standard does not say > > that initialization is a side effect - it would be "modifying an > > object" aiui, yet ahead of initialization being complete the object > > doesn't "exist" imo, and hence can be "modified" only afterwards. > > In C, it is not true that the object does not exist ahead of > initialization. I quoted "exist" for that reason. Of course the object's lifetime starts with its declaration. It just has indeterminate value at that point. > Try the following: > > extern int f(int* p); > > int main() { > int i = f(&i); > } Which to me falls under "Of course expressions used for initialization can themselves have side effects on other objects." Just that "other" isn't quite right here. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |