Re: [PATCH 1/3] x86/p2m.h: Add include guards

[Roberto Bagnara <roberto.bagnara@xxxxxxxxxxx>]

On 17/05/22 17:38, Jan Beulich wrote:
> On 09.05.2022 14:24, Andrew Cooper wrote:
> > Spotted by Eclair MISRA scanner.
> I'm sorry, but what exactly was it that the scanner spotted? It was
> actually deliberate to introduce this file without guards. I'm of
> the general opinion that (private) headers not to be included by
> other headers (but only by .c files) are not in need of guards. If
> it is project-wide consensus that _all_ header files should have
> guards, then I'll try to keep this in mind (in "x86emul: a few
> small steps towards disintegration" for example I introduce
> another such instance), but then it should also be put down in
> ./CODING_STYLE.
The rationale of this rule is as follows:

- With a complex hierarchy of nested header files, it is possible
  for a header file to be included more than once.

- This can bring to circular references of header files, which
  can result in undefined behavior and/or be difficult to debug.

- If multiple inclusion leads to multiple or conflicting definitions,
  then this can result in undefined or erroneous behavior.

- Compilation and analysis time is needlessly increased.

There has been a period (which lasted until the end of the '70s
or the beginning of the '80s, I would have to dig up to be
more precise) when the solution was thought to be "headers
shall not to be included by other headers but only by .c files."
Experience then showed that, in medium to large projects,
each .c file had to begin with a long list of #include
directives;  such lists needed to be ordered to accommodate
the dependencies between header files;  in some cases the
lists were so long that:

a) it was a kind of black magic to find out the right
   inclusion order, one that would work in any of
   possibly many project configurations;
b) the lists of #include directives often contained duplicates,
   possibly because the desperate programmers where trying
   to find the right order.

In the end, the software engineering community converged
on the idea that guards against multiple inclusion are
a much better alternative.

Of course there are valid reasons to deviate the rule:
some header files might be conceived to be included
multiple times.  A one-line configuration for ECLAIR
will do the trick to make sure such header files are
not reported.

Kind regards,

   Roberto

> > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > ---
> > CC: Jan Beulich <JBeulich@xxxxxxxx>
> > CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > CC: Wei Liu <wl@xxxxxxx>
> > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > CC: Julien Grall <julien@xxxxxxx>
> > CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> > CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> > ---
> >   xen/arch/x86/mm/p2m.h | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/xen/arch/x86/mm/p2m.h b/xen/arch/x86/mm/p2m.h
> > index cc0f6766e4df..dc706b8e4799 100644
> > --- a/xen/arch/x86/mm/p2m.h
> > +++ b/xen/arch/x86/mm/p2m.h
> > @@ -15,6 +15,9 @@
> >    * along with this program; If not, see <http://www.gnu.org/licenses/>.
> >    */
> >   
> > +#ifndef __ARCH_MM_P2M_H__
> > +#define __ARCH_MM_P2M_H__
> > +
> >   struct p2m_domain *p2m_init_one(struct domain *d);
> >   void p2m_free_one(struct p2m_domain *p2m);
> >   
> > @@ -39,6 +42,8 @@ int ept_p2m_init(struct p2m_domain *p2m);
> >   void ept_p2m_uninit(struct p2m_domain *p2m);
> >   void p2m_init_altp2m_ept(struct domain *d, unsigned int i);
> >   
> > +#endif /* __ARCH_MM_P2M_H__ */
> > +
> >   /*
> >    * Local variables:
> >    * mode: C