| Message ID | a68267c7465a9b0d2ed8f844a5e0145de50b0f3a.1725550985.git.alessandro.zucchelli@bugseng.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [XEN,v6] CODING_STYLE: Add a section on header guards naming conventions | expand |
On 05.09.2024 17:48, Alessandro Zucchelli wrote: > This section explains which format should be followed by header > inclusion guards via a drop-down list of rules. > > No functional change. > > Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> > > --- > Changes in v6: > - edit inclusion guards naming conventions, including more details Yet I'm afraid that from my pov we're still not there. Specifically ... > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -159,6 +159,34 @@ Emacs local variables > A comment block containing local variables for emacs is permitted at > the end of files. It should be: > > +Header inclusion guards > +----------------------- > + > +Unless otherwise specified, all header files should include proper > +guards to prevent multiple inclusions. The following naming conventions > +apply: ... reading this, I can't derive ... > +- Private headers: <dir>__<filename>_H > + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H > + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H ... the absence of an equivalent of the arch/ part of the path. As per my recollection we agreed on that shortening, but it needs spelling out in the textual description. Such that it is possible to derived what to uses as a name for, say, a header under common/, crypto/, or drivers/ (or anywhere else of course). Specifically with the further examples ... > +- asm-generic headers: ASM_GENERIC__<filename>_H > + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H > + > +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H > + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H ... here and ... > +- Xen headers: XEN__<filename>_H > + - include/xen/something.h -> XEN__SOMETHING_H ... here, where more than just one path component is omitted, deriving what's meant can end up ambiguous. Yet ambiguity is what we absolutely want to avoid, to preempt later discussions on any such naming. Plus I think that only once properly spelled out as rules it'll become sufficiently clear whether there is any remaining risk of naming collisions. Jan
On Mon, 9 Sep 2024, Jan Beulich wrote: > On 05.09.2024 17:48, Alessandro Zucchelli wrote: > > This section explains which format should be followed by header > > inclusion guards via a drop-down list of rules. > > > > No functional change. > > > > Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> > > > > --- > > Changes in v6: > > - edit inclusion guards naming conventions, including more details > > Yet I'm afraid that from my pov we're still not there. Specifically ... > > > --- a/CODING_STYLE > > +++ b/CODING_STYLE > > @@ -159,6 +159,34 @@ Emacs local variables > > A comment block containing local variables for emacs is permitted at > > the end of files. It should be: > > > > +Header inclusion guards > > +----------------------- > > + > > +Unless otherwise specified, all header files should include proper > > +guards to prevent multiple inclusions. The following naming conventions > > +apply: > > ... reading this, I can't derive ... > > > +- Private headers: <dir>__<filename>_H > > + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > > + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H > > + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H > > ... the absence of an equivalent of the arch/ part of the path. As per > my recollection we agreed on that shortening, but it needs spelling out > in the textual description. Such that it is possible to derived what to > uses as a name for, say, a header under common/, crypto/, or drivers/ > (or anywhere else of course). Specifically with the further examples ... Are you asking for something like this? Omit the word "arch" from the filepath. If you prefer an alternative wording please suggest the text. > > +- asm-generic headers: ASM_GENERIC__<filename>_H > > + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H > > + > > +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H > > + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H > > ... here and ... Suggested text: Omit the words "arch" and "include/asm" from the filepath, ASM is also prefixed. > > +- Xen headers: XEN__<filename>_H > > + - include/xen/something.h -> XEN__SOMETHING_H > > ... here, where more than just one path component is omitted, deriving > what's meant can end up ambiguous. Yet ambiguity is what we absolutely > want to avoid, to preempt later discussions on any such naming. Suggested text: Omit the words "include/xen" from the filepath, XEN is always prefixed. Please suggest a specific alternative if you prefer
On 10.09.2024 06:57, Stefano Stabellini wrote: > On Mon, 9 Sep 2024, Jan Beulich wrote: >> On 05.09.2024 17:48, Alessandro Zucchelli wrote: >>> This section explains which format should be followed by header >>> inclusion guards via a drop-down list of rules. >>> >>> No functional change. >>> >>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> >>> >>> --- >>> Changes in v6: >>> - edit inclusion guards naming conventions, including more details >> >> Yet I'm afraid that from my pov we're still not there. Specifically ... >> >>> --- a/CODING_STYLE >>> +++ b/CODING_STYLE >>> @@ -159,6 +159,34 @@ Emacs local variables >>> A comment block containing local variables for emacs is permitted at >>> the end of files. It should be: >>> >>> +Header inclusion guards >>> +----------------------- >>> + >>> +Unless otherwise specified, all header files should include proper >>> +guards to prevent multiple inclusions. The following naming conventions >>> +apply: >> >> ... reading this, I can't derive ... >> >>> +- Private headers: <dir>__<filename>_H >>> + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H >>> + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H >>> + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H >> >> ... the absence of an equivalent of the arch/ part of the path. As per >> my recollection we agreed on that shortening, but it needs spelling out >> in the textual description. Such that it is possible to derived what to >> uses as a name for, say, a header under common/, crypto/, or drivers/ >> (or anywhere else of course). Specifically with the further examples ... > > Are you asking for something like this? > > Omit the word "arch" from the filepath. > > If you prefer an alternative wording please suggest the text. > > >>> +- asm-generic headers: ASM_GENERIC__<filename>_H >>> + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H >>> + >>> +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H >>> + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H >> >> ... here and ... > > Suggested text: > > Omit the words "arch" and "include/asm" from the filepath, ASM is also > prefixed. > > >>> +- Xen headers: XEN__<filename>_H >>> + - include/xen/something.h -> XEN__SOMETHING_H >> >> ... here, where more than just one path component is omitted, deriving >> what's meant can end up ambiguous. Yet ambiguity is what we absolutely >> want to avoid, to preempt later discussions on any such naming. > > Suggested text: > > Omit the words "include/xen" from the filepath, XEN is always prefixed. > > Please suggest a specific alternative if you prefer Looks like I still didn't get across my point: The verbal description that's ahead of all of the examples should be complete enough to describe the whole set of rules, in sufficiently abstract terms. Then the examples will be easy to prove as fitting those rules, and it will be easy to derive the naming for further identifiers. IOW - no, I'm not asking for the examples to be further commented, but for the naming rules to be _fully_ spelled out. Jan
On Tue, 10 Sep 2024, Jan Beulich wrote: > On 10.09.2024 06:57, Stefano Stabellini wrote: > > On Mon, 9 Sep 2024, Jan Beulich wrote: > >> On 05.09.2024 17:48, Alessandro Zucchelli wrote: > >>> This section explains which format should be followed by header > >>> inclusion guards via a drop-down list of rules. > >>> > >>> No functional change. > >>> > >>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> > >>> > >>> --- > >>> Changes in v6: > >>> - edit inclusion guards naming conventions, including more details > >> > >> Yet I'm afraid that from my pov we're still not there. Specifically ... > >> > >>> --- a/CODING_STYLE > >>> +++ b/CODING_STYLE > >>> @@ -159,6 +159,34 @@ Emacs local variables > >>> A comment block containing local variables for emacs is permitted at > >>> the end of files. It should be: > >>> > >>> +Header inclusion guards > >>> +----------------------- > >>> + > >>> +Unless otherwise specified, all header files should include proper > >>> +guards to prevent multiple inclusions. The following naming conventions > >>> +apply: > >> > >> ... reading this, I can't derive ... > >> > >>> +- Private headers: <dir>__<filename>_H > >>> + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > >>> + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H > >>> + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H > >> > >> ... the absence of an equivalent of the arch/ part of the path. As per > >> my recollection we agreed on that shortening, but it needs spelling out > >> in the textual description. Such that it is possible to derived what to > >> uses as a name for, say, a header under common/, crypto/, or drivers/ > >> (or anywhere else of course). Specifically with the further examples ... > > > > Are you asking for something like this? > > > > Omit the word "arch" from the filepath. > > > > If you prefer an alternative wording please suggest the text. > > > > > >>> +- asm-generic headers: ASM_GENERIC__<filename>_H > >>> + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H > >>> + > >>> +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H > >>> + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H > >> > >> ... here and ... > > > > Suggested text: > > > > Omit the words "arch" and "include/asm" from the filepath, ASM is also > > prefixed. > > > > > >>> +- Xen headers: XEN__<filename>_H > >>> + - include/xen/something.h -> XEN__SOMETHING_H > >> > >> ... here, where more than just one path component is omitted, deriving > >> what's meant can end up ambiguous. Yet ambiguity is what we absolutely > >> want to avoid, to preempt later discussions on any such naming. > > > > Suggested text: > > > > Omit the words "include/xen" from the filepath, XEN is always prefixed. > > > > Please suggest a specific alternative if you prefer > > Looks like I still didn't get across my point: The verbal description > that's ahead of all of the examples should be complete enough to describe > the whole set of rules, in sufficiently abstract terms. Then the examples > will be easy to prove as fitting those rules, and it will be easy to > derive the naming for further identifiers. IOW - no, I'm not asking for > the examples to be further commented, but for the naming rules to be > _fully_ spelled out. Hi Jan, we have gone back and forth on this a few times, but neither Alessandro nor I fully understand your perspective. To help streamline the process and save time for everyone, I suggest you provide an example of the rules written in the style you believe is appropriate. Once you set the initial direction, Alessandro and I can continue and complete the rest in that preferred style. On a related note, I have encountered formal specifications that use less formal language than this simple code style and naming convention adjustment. I feel we might be over-engineering this, and in my opinion, the current version is sufficient. Any additional time spent on this could be better used addressing MISRA violations that pose real safety risks for Xen users.
On 12.09.2024 03:13, Stefano Stabellini wrote: > On Tue, 10 Sep 2024, Jan Beulich wrote: >> On 10.09.2024 06:57, Stefano Stabellini wrote: >>> On Mon, 9 Sep 2024, Jan Beulich wrote: >>>> On 05.09.2024 17:48, Alessandro Zucchelli wrote: >>>>> This section explains which format should be followed by header >>>>> inclusion guards via a drop-down list of rules. >>>>> >>>>> No functional change. >>>>> >>>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> >>>>> >>>>> --- >>>>> Changes in v6: >>>>> - edit inclusion guards naming conventions, including more details >>>> >>>> Yet I'm afraid that from my pov we're still not there. Specifically ... >>>> >>>>> --- a/CODING_STYLE >>>>> +++ b/CODING_STYLE >>>>> @@ -159,6 +159,34 @@ Emacs local variables >>>>> A comment block containing local variables for emacs is permitted at >>>>> the end of files. It should be: >>>>> >>>>> +Header inclusion guards >>>>> +----------------------- >>>>> + >>>>> +Unless otherwise specified, all header files should include proper >>>>> +guards to prevent multiple inclusions. The following naming conventions >>>>> +apply: >>>> >>>> ... reading this, I can't derive ... >>>> >>>>> +- Private headers: <dir>__<filename>_H >>>>> + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H >>>>> + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H >>>>> + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H >>>> >>>> ... the absence of an equivalent of the arch/ part of the path. As per >>>> my recollection we agreed on that shortening, but it needs spelling out >>>> in the textual description. Such that it is possible to derived what to >>>> uses as a name for, say, a header under common/, crypto/, or drivers/ >>>> (or anywhere else of course). Specifically with the further examples ... >>> >>> Are you asking for something like this? >>> >>> Omit the word "arch" from the filepath. >>> >>> If you prefer an alternative wording please suggest the text. >>> >>> >>>>> +- asm-generic headers: ASM_GENERIC__<filename>_H >>>>> + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H >>>>> + >>>>> +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H >>>>> + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H >>>> >>>> ... here and ... >>> >>> Suggested text: >>> >>> Omit the words "arch" and "include/asm" from the filepath, ASM is also >>> prefixed. >>> >>> >>>>> +- Xen headers: XEN__<filename>_H >>>>> + - include/xen/something.h -> XEN__SOMETHING_H >>>> >>>> ... here, where more than just one path component is omitted, deriving >>>> what's meant can end up ambiguous. Yet ambiguity is what we absolutely >>>> want to avoid, to preempt later discussions on any such naming. >>> >>> Suggested text: >>> >>> Omit the words "include/xen" from the filepath, XEN is always prefixed. >>> >>> Please suggest a specific alternative if you prefer >> >> Looks like I still didn't get across my point: The verbal description >> that's ahead of all of the examples should be complete enough to describe >> the whole set of rules, in sufficiently abstract terms. Then the examples >> will be easy to prove as fitting those rules, and it will be easy to >> derive the naming for further identifiers. IOW - no, I'm not asking for >> the examples to be further commented, but for the naming rules to be >> _fully_ spelled out. > > > Hi Jan, we have gone back and forth on this a few times, but neither > Alessandro nor I fully understand your perspective. To help streamline > the process and save time for everyone, I suggest you provide an example > of the rules written in the style you believe is appropriate. Once you > set the initial direction, Alessandro and I can continue and complete > the rest in that preferred style. If you really expect me to do so (hence effectively me becoming the one to make the proposal, which I never meant to), it'll have to wait until I'm back from the GNU Tools Cauldron and the PTO I'm taking immediately afterwards. Jan > On a related note, I have encountered formal specifications that use less > formal language than this simple code style and naming convention > adjustment. I feel we might be over-engineering this, and in my opinion, > the current version is sufficient. Any additional time spent on this > could be better used addressing MISRA violations that pose real safety > risks for Xen users.
On Thu, Sep 12, 2024 at 3:35 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 12.09.2024 03:13, Stefano Stabellini wrote: > > On Tue, 10 Sep 2024, Jan Beulich wrote: > >> On 10.09.2024 06:57, Stefano Stabellini wrote: > >>> On Mon, 9 Sep 2024, Jan Beulich wrote: > >>>> On 05.09.2024 17:48, Alessandro Zucchelli wrote: > >>>>> This section explains which format should be followed by header > >>>>> inclusion guards via a drop-down list of rules. > >>>>> > >>>>> No functional change. > >>>>> > >>>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> > >>>>> > >>>>> --- > >>>>> Changes in v6: > >>>>> - edit inclusion guards naming conventions, including more details > >>>> > >>>> Yet I'm afraid that from my pov we're still not there. Specifically ... > >>>> > >>>>> --- a/CODING_STYLE > >>>>> +++ b/CODING_STYLE > >>>>> @@ -159,6 +159,34 @@ Emacs local variables > >>>>> A comment block containing local variables for emacs is permitted at > >>>>> the end of files. It should be: > >>>>> > >>>>> +Header inclusion guards > >>>>> +----------------------- > >>>>> + > >>>>> +Unless otherwise specified, all header files should include proper > >>>>> +guards to prevent multiple inclusions. The following naming conventions > >>>>> +apply: > >>>> > >>>> ... reading this, I can't derive ... > >>>> > >>>>> +- Private headers: <dir>__<filename>_H > >>>>> + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > >>>>> + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H > >>>>> + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H > >>>> > >>>> ... the absence of an equivalent of the arch/ part of the path. As per > >>>> my recollection we agreed on that shortening, but it needs spelling out > >>>> in the textual description. Such that it is possible to derived what to > >>>> uses as a name for, say, a header under common/, crypto/, or drivers/ > >>>> (or anywhere else of course). Specifically with the further examples ... > >>> > >>> Are you asking for something like this? > >>> > >>> Omit the word "arch" from the filepath. > >>> > >>> If you prefer an alternative wording please suggest the text. > >>> > >>> > >>>>> +- asm-generic headers: ASM_GENERIC__<filename>_H > >>>>> + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H > >>>>> + > >>>>> +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H > >>>>> + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H > >>>> > >>>> ... here and ... > >>> > >>> Suggested text: > >>> > >>> Omit the words "arch" and "include/asm" from the filepath, ASM is also > >>> prefixed. > >>> > >>> > >>>>> +- Xen headers: XEN__<filename>_H > >>>>> + - include/xen/something.h -> XEN__SOMETHING_H > >>>> > >>>> ... here, where more than just one path component is omitted, deriving > >>>> what's meant can end up ambiguous. Yet ambiguity is what we absolutely > >>>> want to avoid, to preempt later discussions on any such naming. > >>> > >>> Suggested text: > >>> > >>> Omit the words "include/xen" from the filepath, XEN is always prefixed. > >>> > >>> Please suggest a specific alternative if you prefer > >> > >> Looks like I still didn't get across my point: The verbal description > >> that's ahead of all of the examples should be complete enough to describe > >> the whole set of rules, in sufficiently abstract terms. Then the examples > >> will be easy to prove as fitting those rules, and it will be easy to > >> derive the naming for further identifiers. IOW - no, I'm not asking for > >> the examples to be further commented, but for the naming rules to be > >> _fully_ spelled out. > > > > > > Hi Jan, we have gone back and forth on this a few times, but neither > > Alessandro nor I fully understand your perspective. To help streamline > > the process and save time for everyone, I suggest you provide an example > > of the rules written in the style you believe is appropriate. Once you > > set the initial direction, Alessandro and I can continue and complete > > the rest in that preferred style. > > If you really expect me to do so (hence effectively me becoming the one > to make the proposal, which I never meant to), it'll have to wait until > I'm back from the GNU Tools Cauldron and the PTO I'm taking immediately > afterwards. > > Jan > > > On a related note, I have encountered formal specifications that use less > > formal language than this simple code style and naming convention > > adjustment. I feel we might be over-engineering this, and in my opinion, > > the current version is sufficient. Any additional time spent on this > > could be better used addressing MISRA violations that pose real safety > > risks for Xen users. > > Why not just following the simple rule? If file is arch/arm/arm64/lib/something.h have a ARCH__ARM__ARM64__LIB__SOMETHING_H guard, if file is arch/x86/lib/something.h have a ARCH__X86__LIB__SOMETHING_H guard. Frediano
On 12.09.2024 17:03, Frediano Ziglio wrote: > On Thu, Sep 12, 2024 at 3:35 PM Jan Beulich <jbeulich@suse.com> wrote: >> >> On 12.09.2024 03:13, Stefano Stabellini wrote: >>> On Tue, 10 Sep 2024, Jan Beulich wrote: >>>> On 10.09.2024 06:57, Stefano Stabellini wrote: >>>>> On Mon, 9 Sep 2024, Jan Beulich wrote: >>>>>> On 05.09.2024 17:48, Alessandro Zucchelli wrote: >>>>>>> This section explains which format should be followed by header >>>>>>> inclusion guards via a drop-down list of rules. >>>>>>> >>>>>>> No functional change. >>>>>>> >>>>>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> >>>>>>> >>>>>>> --- >>>>>>> Changes in v6: >>>>>>> - edit inclusion guards naming conventions, including more details >>>>>> >>>>>> Yet I'm afraid that from my pov we're still not there. Specifically ... >>>>>> >>>>>>> --- a/CODING_STYLE >>>>>>> +++ b/CODING_STYLE >>>>>>> @@ -159,6 +159,34 @@ Emacs local variables >>>>>>> A comment block containing local variables for emacs is permitted at >>>>>>> the end of files. It should be: >>>>>>> >>>>>>> +Header inclusion guards >>>>>>> +----------------------- >>>>>>> + >>>>>>> +Unless otherwise specified, all header files should include proper >>>>>>> +guards to prevent multiple inclusions. The following naming conventions >>>>>>> +apply: >>>>>> >>>>>> ... reading this, I can't derive ... >>>>>> >>>>>>> +- Private headers: <dir>__<filename>_H >>>>>>> + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H >>>>>>> + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H >>>>>>> + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H >>>>>> >>>>>> ... the absence of an equivalent of the arch/ part of the path. As per >>>>>> my recollection we agreed on that shortening, but it needs spelling out >>>>>> in the textual description. Such that it is possible to derived what to >>>>>> uses as a name for, say, a header under common/, crypto/, or drivers/ >>>>>> (or anywhere else of course). Specifically with the further examples ... >>>>> >>>>> Are you asking for something like this? >>>>> >>>>> Omit the word "arch" from the filepath. >>>>> >>>>> If you prefer an alternative wording please suggest the text. >>>>> >>>>> >>>>>>> +- asm-generic headers: ASM_GENERIC__<filename>_H >>>>>>> + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H >>>>>>> + >>>>>>> +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H >>>>>>> + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H >>>>>> >>>>>> ... here and ... >>>>> >>>>> Suggested text: >>>>> >>>>> Omit the words "arch" and "include/asm" from the filepath, ASM is also >>>>> prefixed. >>>>> >>>>> >>>>>>> +- Xen headers: XEN__<filename>_H >>>>>>> + - include/xen/something.h -> XEN__SOMETHING_H >>>>>> >>>>>> ... here, where more than just one path component is omitted, deriving >>>>>> what's meant can end up ambiguous. Yet ambiguity is what we absolutely >>>>>> want to avoid, to preempt later discussions on any such naming. >>>>> >>>>> Suggested text: >>>>> >>>>> Omit the words "include/xen" from the filepath, XEN is always prefixed. >>>>> >>>>> Please suggest a specific alternative if you prefer >>>> >>>> Looks like I still didn't get across my point: The verbal description >>>> that's ahead of all of the examples should be complete enough to describe >>>> the whole set of rules, in sufficiently abstract terms. Then the examples >>>> will be easy to prove as fitting those rules, and it will be easy to >>>> derive the naming for further identifiers. IOW - no, I'm not asking for >>>> the examples to be further commented, but for the naming rules to be >>>> _fully_ spelled out. >>> >>> >>> Hi Jan, we have gone back and forth on this a few times, but neither >>> Alessandro nor I fully understand your perspective. To help streamline >>> the process and save time for everyone, I suggest you provide an example >>> of the rules written in the style you believe is appropriate. Once you >>> set the initial direction, Alessandro and I can continue and complete >>> the rest in that preferred style. >> >> If you really expect me to do so (hence effectively me becoming the one >> to make the proposal, which I never meant to), it'll have to wait until >> I'm back from the GNU Tools Cauldron and the PTO I'm taking immediately >> afterwards. >> >> Jan >> >>> On a related note, I have encountered formal specifications that use less >>> formal language than this simple code style and naming convention >>> adjustment. I feel we might be over-engineering this, and in my opinion, >>> the current version is sufficient. Any additional time spent on this >>> could be better used addressing MISRA violations that pose real safety >>> risks for Xen users. > > Why not just following the simple rule? > If file is arch/arm/arm64/lib/something.h have a > ARCH__ARM__ARM64__LIB__SOMETHING_H guard, if file is > arch/x86/lib/something.h have a ARCH__X86__LIB__SOMETHING_H guard. We've been there before: Identifiers get overly long this way. Jan
On Thu, 12 Sep 2024, Jan Beulich wrote: > On 12.09.2024 03:13, Stefano Stabellini wrote: > > On Tue, 10 Sep 2024, Jan Beulich wrote: > >> On 10.09.2024 06:57, Stefano Stabellini wrote: > >>> On Mon, 9 Sep 2024, Jan Beulich wrote: > >>>> On 05.09.2024 17:48, Alessandro Zucchelli wrote: > >>>>> This section explains which format should be followed by header > >>>>> inclusion guards via a drop-down list of rules. > >>>>> > >>>>> No functional change. > >>>>> > >>>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> > >>>>> > >>>>> --- > >>>>> Changes in v6: > >>>>> - edit inclusion guards naming conventions, including more details > >>>> > >>>> Yet I'm afraid that from my pov we're still not there. Specifically ... > >>>> > >>>>> --- a/CODING_STYLE > >>>>> +++ b/CODING_STYLE > >>>>> @@ -159,6 +159,34 @@ Emacs local variables > >>>>> A comment block containing local variables for emacs is permitted at > >>>>> the end of files. It should be: > >>>>> > >>>>> +Header inclusion guards > >>>>> +----------------------- > >>>>> + > >>>>> +Unless otherwise specified, all header files should include proper > >>>>> +guards to prevent multiple inclusions. The following naming conventions > >>>>> +apply: > >>>> > >>>> ... reading this, I can't derive ... > >>>> > >>>>> +- Private headers: <dir>__<filename>_H > >>>>> + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > >>>>> + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H > >>>>> + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H > >>>> > >>>> ... the absence of an equivalent of the arch/ part of the path. As per > >>>> my recollection we agreed on that shortening, but it needs spelling out > >>>> in the textual description. Such that it is possible to derived what to > >>>> uses as a name for, say, a header under common/, crypto/, or drivers/ > >>>> (or anywhere else of course). Specifically with the further examples ... > >>> > >>> Are you asking for something like this? > >>> > >>> Omit the word "arch" from the filepath. > >>> > >>> If you prefer an alternative wording please suggest the text. > >>> > >>> > >>>>> +- asm-generic headers: ASM_GENERIC__<filename>_H > >>>>> + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H > >>>>> + > >>>>> +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H > >>>>> + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H > >>>> > >>>> ... here and ... > >>> > >>> Suggested text: > >>> > >>> Omit the words "arch" and "include/asm" from the filepath, ASM is also > >>> prefixed. > >>> > >>> > >>>>> +- Xen headers: XEN__<filename>_H > >>>>> + - include/xen/something.h -> XEN__SOMETHING_H > >>>> > >>>> ... here, where more than just one path component is omitted, deriving > >>>> what's meant can end up ambiguous. Yet ambiguity is what we absolutely > >>>> want to avoid, to preempt later discussions on any such naming. > >>> > >>> Suggested text: > >>> > >>> Omit the words "include/xen" from the filepath, XEN is always prefixed. > >>> > >>> Please suggest a specific alternative if you prefer > >> > >> Looks like I still didn't get across my point: The verbal description > >> that's ahead of all of the examples should be complete enough to describe > >> the whole set of rules, in sufficiently abstract terms. Then the examples > >> will be easy to prove as fitting those rules, and it will be easy to > >> derive the naming for further identifiers. IOW - no, I'm not asking for > >> the examples to be further commented, but for the naming rules to be > >> _fully_ spelled out. > > > > > > Hi Jan, we have gone back and forth on this a few times, but neither > > Alessandro nor I fully understand your perspective. To help streamline > > the process and save time for everyone, I suggest you provide an example > > of the rules written in the style you believe is appropriate. Once you > > set the initial direction, Alessandro and I can continue and complete > > the rest in that preferred style. > > If you really expect me to do so (hence effectively me becoming the one > to make the proposal, which I never meant to), it'll have to wait until > I'm back from the GNU Tools Cauldron and the PTO I'm taking immediately > afterwards. It looks like you have specific ideas on how it should be done so I think it would be better if you provide a couple of complete examples for a subset of the proposal. For instance, only covering Private headers: <dir>__<filename>_H. With that example, I think we can extrapolate the others. I understand if we need to wait until you are back from PTO.
On 12.09.2024 03:13, Stefano Stabellini wrote: > Hi Jan, we have gone back and forth on this a few times, but neither > Alessandro nor I fully understand your perspective. To help streamline > the process and save time for everyone, I suggest you provide an example > of the rules written in the style you believe is appropriate. Once you > set the initial direction, Alessandro and I can continue and complete > the rest in that preferred style. Header inclusion guards ----------------------- Unless otherwise specified, all header files should include proper guards to prevent multiple inclusions. The following naming conventions apply: - Guard names are derived from directory path underneath xen/ and the actual file name. Path components are separated by double underscores. Alphabetic characters are converted to upper case. Non-alphanumeric characters are replaced by single underscores. - Certain directory components are omitted, to keep identifier length bounded: - The top level include/, - Any architecture's arch/<arch>/include/asm/ collapses to ASM__<ARCH>__, - Architecture-specific private files' arch/. For example: - Xen headers: XEN__<filename>_H - include/xen/something.h -> XEN__SOMETHING_H - asm-generic headers: ASM_GENERIC__<filename>_H - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H - arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H - Private headers: <dir>__<filename>_H - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H - common/something.h -> COMMON__SOMETHING_H Note that this requires some discipline on the naming of future new sub-directories: There shouldn't be any random asm/ one anywhere, for example. Nor should any new ports be named the same as top-level (within xen/) directories. Which may in turn require some care if any new top- level directories were to be added. Rule of thumb: Whenever a new sub- directory is added, check the rules for no collisions to result. Jan
On Mon, Sep 30, 2024 at 9:58 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 12.09.2024 03:13, Stefano Stabellini wrote: > > Hi Jan, we have gone back and forth on this a few times, but neither > > Alessandro nor I fully understand your perspective. To help streamline > > the process and save time for everyone, I suggest you provide an example > > of the rules written in the style you believe is appropriate. Once you > > set the initial direction, Alessandro and I can continue and complete > > the rest in that preferred style. > > Header inclusion guards > ----------------------- > > Unless otherwise specified, all header files should include proper > guards to prevent multiple inclusions. The following naming conventions > apply: > > - Guard names are derived from directory path underneath xen/ and the > actual file name. Path components are separated by double underscores. > Alphabetic characters are converted to upper case. Non-alphanumeric > characters are replaced by single underscores. Possibly there should be no cases; but about "Non-alphanumeric characters are replaced by single underscores" are we talking about sequences or single entities? I would say sequences so "Non-alphanumeric character sequences are replaced by single underscores". For instance "foo--bar.h" -> "FOO_BAR_H" and not "foo--bar.h" -> "FOO__BAR_H". Another maybe not supported case is no ASCII characters in name. As far as I can see they are not supported. Is it written somewhere? > - Certain directory components are omitted, to keep identifier length > bounded: > - The top level include/, > - Any architecture's arch/<arch>/include/asm/ collapses to > ASM__<ARCH>__, > - Architecture-specific private files' arch/. > > For example: > > - Xen headers: XEN__<filename>_H > - include/xen/something.h -> XEN__SOMETHING_H > > - asm-generic headers: ASM_GENERIC__<filename>_H > - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H > > - arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H > - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H > > - Private headers: <dir>__<filename>_H > - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H > - common/something.h -> COMMON__SOMETHING_H > > Note that this requires some discipline on the naming of future new > sub-directories: There shouldn't be any random asm/ one anywhere, for > example. Nor should any new ports be named the same as top-level (within > xen/) directories. Which may in turn require some care if any new top- > level directories were to be added. Rule of thumb: Whenever a new sub- > directory is added, check the rules for no collisions to result. > > Jan > Thanks for taking time to get to a proposal. Frediano
On 30.09.2024 11:12, Frediano Ziglio wrote: > On Mon, Sep 30, 2024 at 9:58 AM Jan Beulich <jbeulich@suse.com> wrote: >> >> On 12.09.2024 03:13, Stefano Stabellini wrote: >>> Hi Jan, we have gone back and forth on this a few times, but neither >>> Alessandro nor I fully understand your perspective. To help streamline >>> the process and save time for everyone, I suggest you provide an example >>> of the rules written in the style you believe is appropriate. Once you >>> set the initial direction, Alessandro and I can continue and complete >>> the rest in that preferred style. >> >> Header inclusion guards >> ----------------------- >> >> Unless otherwise specified, all header files should include proper >> guards to prevent multiple inclusions. The following naming conventions >> apply: >> >> - Guard names are derived from directory path underneath xen/ and the >> actual file name. Path components are separated by double underscores. >> Alphabetic characters are converted to upper case. Non-alphanumeric >> characters are replaced by single underscores. > > Possibly there should be no cases; but about "Non-alphanumeric > characters are replaced by single underscores" are we talking about > sequences or single entities? I would say sequences so > "Non-alphanumeric character sequences are replaced by single > underscores". > For instance "foo--bar.h" -> "FOO_BAR_H" and not "foo--bar.h" -> "FOO__BAR_H". I think such files shouldn't be created in the first place. No matter whether you replace by a single underscore or a sequence thereof, ambiguities will arise. > Another maybe not supported case is no ASCII characters in name. As > far as I can see they are not supported. Is it written somewhere? Yet more absurd names, imo, which even more so shouldn't be allowed in. Jan
On Mon, 30 Sep 2024, Jan Beulich wrote: > On 12.09.2024 03:13, Stefano Stabellini wrote: > > Hi Jan, we have gone back and forth on this a few times, but neither > > Alessandro nor I fully understand your perspective. To help streamline > > the process and save time for everyone, I suggest you provide an example > > of the rules written in the style you believe is appropriate. Once you > > set the initial direction, Alessandro and I can continue and complete > > the rest in that preferred style. > > Header inclusion guards > ----------------------- > > Unless otherwise specified, all header files should include proper > guards to prevent multiple inclusions. The following naming conventions > apply: > > - Guard names are derived from directory path underneath xen/ and the > actual file name. Path components are separated by double underscores. > Alphabetic characters are converted to upper case. Non-alphanumeric > characters are replaced by single underscores. > - Certain directory components are omitted, to keep identifier length > bounded: > - The top level include/, > - Any architecture's arch/<arch>/include/asm/ collapses to > ASM__<ARCH>__, > - Architecture-specific private files' arch/. > > For example: > > - Xen headers: XEN__<filename>_H > - include/xen/something.h -> XEN__SOMETHING_H > > - asm-generic headers: ASM_GENERIC__<filename>_H > - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H > > - arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H > - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H > > - Private headers: <dir>__<filename>_H > - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H > - common/something.h -> COMMON__SOMETHING_H > > Note that this requires some discipline on the naming of future new > sub-directories: There shouldn't be any random asm/ one anywhere, for I would remove the word "random" > example. Nor should any new ports be named the same as top-level (within > xen/) directories. Which may in turn require some care if any new top- > level directories were to be added. Rule of thumb: Whenever a new sub- > directory is added, check the rules for no collisions to result. I guess you meant "no collisions among the results" ? This is fine by me. With these two minor NITs addressed, I think you can go ahead and commit the changes with my acked-by.
On 30.09.2024 23:40, Stefano Stabellini wrote: > On Mon, 30 Sep 2024, Jan Beulich wrote: >> On 12.09.2024 03:13, Stefano Stabellini wrote: >>> Hi Jan, we have gone back and forth on this a few times, but neither >>> Alessandro nor I fully understand your perspective. To help streamline >>> the process and save time for everyone, I suggest you provide an example >>> of the rules written in the style you believe is appropriate. Once you >>> set the initial direction, Alessandro and I can continue and complete >>> the rest in that preferred style. >> >> Header inclusion guards >> ----------------------- >> >> Unless otherwise specified, all header files should include proper >> guards to prevent multiple inclusions. The following naming conventions >> apply: >> >> - Guard names are derived from directory path underneath xen/ and the >> actual file name. Path components are separated by double underscores. >> Alphabetic characters are converted to upper case. Non-alphanumeric >> characters are replaced by single underscores. >> - Certain directory components are omitted, to keep identifier length >> bounded: >> - The top level include/, >> - Any architecture's arch/<arch>/include/asm/ collapses to >> ASM__<ARCH>__, >> - Architecture-specific private files' arch/. >> >> For example: >> >> - Xen headers: XEN__<filename>_H >> - include/xen/something.h -> XEN__SOMETHING_H >> >> - asm-generic headers: ASM_GENERIC__<filename>_H >> - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H >> >> - arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H >> - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H >> >> - Private headers: <dir>__<filename>_H >> - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H >> - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H >> - common/something.h -> COMMON__SOMETHING_H >> >> Note that this requires some discipline on the naming of future new >> sub-directories: There shouldn't be any random asm/ one anywhere, for > > I would remove the word "random" Fine with me; perhaps use "other" in its place? >> example. Nor should any new ports be named the same as top-level (within >> xen/) directories. Which may in turn require some care if any new top- >> level directories were to be added. Rule of thumb: Whenever a new sub- >> directory is added, check the rules for no collisions to result. > > I guess you meant "no collisions among the results" ? No, that would be insufficient. I specifically mean to say that there should be no _potential_ for collisions, i.e. not just "among the results", but also for hypothetical files added underneath such new subdirs. Would "for no collisions to potentially result" be more clear from you perspective? Jan
On Tue, 1 Oct 2024, Jan Beulich wrote: > On 30.09.2024 23:40, Stefano Stabellini wrote: > > On Mon, 30 Sep 2024, Jan Beulich wrote: > >> On 12.09.2024 03:13, Stefano Stabellini wrote: > >>> Hi Jan, we have gone back and forth on this a few times, but neither > >>> Alessandro nor I fully understand your perspective. To help streamline > >>> the process and save time for everyone, I suggest you provide an example > >>> of the rules written in the style you believe is appropriate. Once you > >>> set the initial direction, Alessandro and I can continue and complete > >>> the rest in that preferred style. > >> > >> Header inclusion guards > >> ----------------------- > >> > >> Unless otherwise specified, all header files should include proper > >> guards to prevent multiple inclusions. The following naming conventions > >> apply: > >> > >> - Guard names are derived from directory path underneath xen/ and the > >> actual file name. Path components are separated by double underscores. > >> Alphabetic characters are converted to upper case. Non-alphanumeric > >> characters are replaced by single underscores. > >> - Certain directory components are omitted, to keep identifier length > >> bounded: > >> - The top level include/, > >> - Any architecture's arch/<arch>/include/asm/ collapses to > >> ASM__<ARCH>__, > >> - Architecture-specific private files' arch/. > >> > >> For example: > >> > >> - Xen headers: XEN__<filename>_H > >> - include/xen/something.h -> XEN__SOMETHING_H > >> > >> - asm-generic headers: ASM_GENERIC__<filename>_H > >> - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H > >> > >> - arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H > >> - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H > >> > >> - Private headers: <dir>__<filename>_H > >> - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > >> - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H > >> - common/something.h -> COMMON__SOMETHING_H > >> > >> Note that this requires some discipline on the naming of future new > >> sub-directories: There shouldn't be any random asm/ one anywhere, for > > > > I would remove the word "random" > > Fine with me; perhaps use "other" in its place? yes, that's better > >> example. Nor should any new ports be named the same as top-level (within > >> xen/) directories. Which may in turn require some care if any new top- > >> level directories were to be added. Rule of thumb: Whenever a new sub- > >> directory is added, check the rules for no collisions to result. > > > > I guess you meant "no collisions among the results" ? > > No, that would be insufficient. I specifically mean to say that there > should be no _potential_ for collisions, i.e. not just "among the > results", but also for hypothetical files added underneath such new > subdirs. Would "for no collisions to potentially result" be more clear > from you perspective? Yes that's clearer. Maybe this works better from an English point of view: Rule of thumb: Whenever adding a new subdirectory, check the rules to prevent any potential collisions.
diff --git a/CODING_STYLE b/CODING_STYLE index 7f6e9ad065..711f6811f8 100644 --- a/CODING_STYLE +++ b/CODING_STYLE @@ -159,6 +159,34 @@ Emacs local variables A comment block containing local variables for emacs is permitted at the end of files. It should be: +Header inclusion guards +----------------------- + +Unless otherwise specified, all header files should include proper +guards to prevent multiple inclusions. The following naming conventions +apply: + +- Private headers: <dir>__<filename>_H + - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H + - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H + - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H + +- asm-generic headers: ASM_GENERIC__<filename>_H + - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H + +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H + - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H + +- Xen headers: XEN__<filename>_H + - include/xen/something.h -> XEN__SOMETHING_H + +Notes: + +- Filenames and directories are converted to uppercase. +- Non-alphanumeric characters are converted to underscores. +- Directories, subdirectories, and filenames are separated by double + underscores. + /* * Local variables: * mode: C
This section explains which format should be followed by header inclusion guards via a drop-down list of rules. No functional change. Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> --- Changes in v6: - edit inclusion guards naming conventions, including more details Changes in v5: - edit inclusion guards naming conventions, according to feedback received Note: This patch is part of a 17-element patch series, which can be found at the following link: https://lore.kernel.org/xen-devel/cover.1721720583.git.alessandro.zucchelli@bugseng.com/ Since this is the only patch that required revision, and as requested by the maintainers, it is now submitted as a standalone patch. --- CODING_STYLE | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)