From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Add Rule 5.1, with the additional note that the character limit for Xen
is 63 characters.

The max length identifiers found by ECLAIR are:

__mitigate_spectre_bhb_clear_insn_start
domain_pause_by_systemcontroller_nosync

Both of them are 40 characters long. A limit of 63 characters work for
the existing code.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 docs/misra/rules.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index c0ee58ab25..6052fc6309 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -35,6 +35,11 @@ hypervisor.
   - Severity:  Required
   - Summary:  Line-splicing shall not be used in // comments
   - Link:  https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_03_02.c
+- Rule: Rule 5.1
+  - Severity:  Required
+  - Summary:  External identifiers shall be distinct
+  - Link:  https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_05_01_2.c
+  - Note: the Xen characters limit for identifiers is 63
 - Rule: Rule 6.2
   - Severity:  Required
   - Summary:  Single-bit named bit fields shall not be of a signed type
-- 
2.25.1

On 25.05.2022 02:35, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> Add Rule 5.1, with the additional note that the character limit for Xen
> is 63 characters.
> 
> The max length identifiers found by ECLAIR are:
> 
> __mitigate_spectre_bhb_clear_insn_start
> domain_pause_by_systemcontroller_nosync
> 
> Both of them are 40 characters long. A limit of 63 characters work for
> the existing code.

I have to admit that it hasn't become clear to me why we want to
permit (if not to say encourage) the use of such long identifiers.
If 40 is the longest we've got, why not limit it to 40 for now
with a goal of further reducing? A 40-char symbol plus some
indentation will already pose problems with 80-char line length.

Otoh, as said on the call, I think the public headers want
mentioning explicitly here in some way. Part of them (most or all
of what's under io/) aren't used when building Xen, so won't be
seen by Eclair (aiui). Yet they are a formal part of the code
base, and e.g. ring.h has some pretty long names (albeit still
below 40 chars as it looks). So once we're able to go down to e.g.
32 for the bulk of the code base, public headers should imo still
be explicitly allowed to use longer identifiers.

Jan

On Wed, 25 May 2022, Jan Beulich wrote:
> On 25.05.2022 02:35, Stefano Stabellini wrote:
> > From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > 
> > Add Rule 5.1, with the additional note that the character limit for Xen
> > is 63 characters.
> > 
> > The max length identifiers found by ECLAIR are:
> > 
> > __mitigate_spectre_bhb_clear_insn_start
> > domain_pause_by_systemcontroller_nosync
> > 
> > Both of them are 40 characters long. A limit of 63 characters work for
> > the existing code.
> 
> I have to admit that it hasn't become clear to me why we want to
> permit (if not to say encourage) the use of such long identifiers.
> If 40 is the longest we've got, why not limit it to 40 for now
> with a goal of further reducing? A 40-char symbol plus some
> indentation will already pose problems with 80-char line length.
 
We can go lower than 63 if we want to. I chose the closest power-of-two
length -1 for the NUL terminator. But it doesn't have to be a
power-of-two. So we could go with "41" if we want to.

Anyone in favor of that? I am happy with any number between 41 and 63.


> Otoh, as said on the call, I think the public headers want
> mentioning explicitly here in some way. Part of them (most or all
> of what's under io/) aren't used when building Xen, so won't be
> seen by Eclair (aiui). Yet they are a formal part of the code
> base, and e.g. ring.h has some pretty long names (albeit still
> below 40 chars as it looks). So once we're able to go down to e.g.
> 32 for the bulk of the code base, public headers should imo still
> be explicitly allowed to use longer identifiers.

Actually I thought about writing something for the public header but I
wasn't sure what to write. What about:

- Note: the Xen characters limit for identifiers is 41. Public headers
  (xen/include/public/) are allowed to retain longer identifiers for
  backward compatibility.

On 26.05.2022 03:18, Stefano Stabellini wrote:
> On Wed, 25 May 2022, Jan Beulich wrote:
>> On 25.05.2022 02:35, Stefano Stabellini wrote:
>>> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>>
>>> Add Rule 5.1, with the additional note that the character limit for Xen
>>> is 63 characters.
>>>
>>> The max length identifiers found by ECLAIR are:
>>>
>>> __mitigate_spectre_bhb_clear_insn_start
>>> domain_pause_by_systemcontroller_nosync
>>>
>>> Both of them are 40 characters long. A limit of 63 characters work for
>>> the existing code.
>>
>> I have to admit that it hasn't become clear to me why we want to
>> permit (if not to say encourage) the use of such long identifiers.
>> If 40 is the longest we've got, why not limit it to 40 for now
>> with a goal of further reducing? A 40-char symbol plus some
>> indentation will already pose problems with 80-char line length.
>  
> We can go lower than 63 if we want to. I chose the closest power-of-two
> length -1 for the NUL terminator. But it doesn't have to be a
> power-of-two. So we could go with "41" if we want to.
> 
> Anyone in favor of that? I am happy with any number between 41 and 63.

Why 41, not 40? 41 seems yet more arbitrary to me than e.g. 40.

>> Otoh, as said on the call, I think the public headers want
>> mentioning explicitly here in some way. Part of them (most or all
>> of what's under io/) aren't used when building Xen, so won't be
>> seen by Eclair (aiui). Yet they are a formal part of the code
>> base, and e.g. ring.h has some pretty long names (albeit still
>> below 40 chars as it looks). So once we're able to go down to e.g.
>> 32 for the bulk of the code base, public headers should imo still
>> be explicitly allowed to use longer identifiers.
> 
> Actually I thought about writing something for the public header but I
> wasn't sure what to write. What about:
> 
> - Note: the Xen characters limit for identifiers is 41. Public headers
>   (xen/include/public/) are allowed to retain longer identifiers for
>   backward compatibility.

Fine with me, except I wonder in how far going forward we actually
need to play by that limit there. Proper name-spacing is particularly
important in the public headers, so may warrant a higher limit for
certain (unusual?) circumstances.

Jan

On Thu, 26 May 2022, Jan Beulich wrote:
> On 26.05.2022 03:18, Stefano Stabellini wrote:
> > On Wed, 25 May 2022, Jan Beulich wrote:
> >> On 25.05.2022 02:35, Stefano Stabellini wrote:
> >>> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> >>>
> >>> Add Rule 5.1, with the additional note that the character limit for Xen
> >>> is 63 characters.
> >>>
> >>> The max length identifiers found by ECLAIR are:
> >>>
> >>> __mitigate_spectre_bhb_clear_insn_start
> >>> domain_pause_by_systemcontroller_nosync
> >>>
> >>> Both of them are 40 characters long. A limit of 63 characters work for
> >>> the existing code.
> >>
> >> I have to admit that it hasn't become clear to me why we want to
> >> permit (if not to say encourage) the use of such long identifiers.
> >> If 40 is the longest we've got, why not limit it to 40 for now
> >> with a goal of further reducing? A 40-char symbol plus some
> >> indentation will already pose problems with 80-char line length.
> >  
> > We can go lower than 63 if we want to. I chose the closest power-of-two
> > length -1 for the NUL terminator. But it doesn't have to be a
> > power-of-two. So we could go with "41" if we want to.
> > 
> > Anyone in favor of that? I am happy with any number between 41 and 63.
> 
> Why 41, not 40? 41 seems yet more arbitrary to me than e.g. 40.

I was adding +1 to account for the NUL terminator, but actually I
already added it before (__mitigate_spectre_bhb_clear_insn_start and
domain_pause_by_systemcontroller_nosync are 39 characters).

So yes we can go with 40.


> >> Otoh, as said on the call, I think the public headers want
> >> mentioning explicitly here in some way. Part of them (most or all
> >> of what's under io/) aren't used when building Xen, so won't be
> >> seen by Eclair (aiui). Yet they are a formal part of the code
> >> base, and e.g. ring.h has some pretty long names (albeit still
> >> below 40 chars as it looks). So once we're able to go down to e.g.
> >> 32 for the bulk of the code base, public headers should imo still
> >> be explicitly allowed to use longer identifiers.
> > 
> > Actually I thought about writing something for the public header but I
> > wasn't sure what to write. What about:
> > 
> > - Note: the Xen characters limit for identifiers is 41. Public headers
> >   (xen/include/public/) are allowed to retain longer identifiers for
> >   backward compatibility.
> 
> Fine with me, except I wonder in how far going forward we actually
> need to play by that limit there. Proper name-spacing is particularly
> important in the public headers, so may warrant a higher limit for
> certain (unusual?) circumstances.

Keep in mind that the rules are general guidelines and good defaults.
There can be always special cases, and that is especially true for
"unusual circumstances".

> On 26 May 2022, at 10:40, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 26.05.2022 03:18, Stefano Stabellini wrote:
>> On Wed, 25 May 2022, Jan Beulich wrote:
>>> On 25.05.2022 02:35, Stefano Stabellini wrote:
>>>> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>>> 
>>>> Add Rule 5.1, with the additional note that the character limit for Xen
>>>> is 63 characters.
>>>> 
>>>> The max length identifiers found by ECLAIR are:
>>>> 
>>>> __mitigate_spectre_bhb_clear_insn_start
>>>> domain_pause_by_systemcontroller_nosync
>>>> 
>>>> Both of them are 40 characters long. A limit of 63 characters work for
>>>> the existing code.
>>> 
>>> I have to admit that it hasn't become clear to me why we want to
>>> permit (if not to say encourage) the use of such long identifiers.
>>> If 40 is the longest we've got, why not limit it to 40 for now
>>> with a goal of further reducing? A 40-char symbol plus some
>>> indentation will already pose problems with 80-char line length.
>> 
>> We can go lower than 63 if we want to. I chose the closest power-of-two
>> length -1 for the NUL terminator. But it doesn't have to be a
>> power-of-two. So we could go with "41" if we want to.
>> 
>> Anyone in favor of that? I am happy with any number between 41 and 63.
> 
> Why 41, not 40? 41 seems yet more arbitrary to me than e.g. 40.
> 
>>> Otoh, as said on the call, I think the public headers want
>>> mentioning explicitly here in some way. Part of them (most or all
>>> of what's under io/) aren't used when building Xen, so won't be
>>> seen by Eclair (aiui). Yet they are a formal part of the code
>>> base, and e.g. ring.h has some pretty long names (albeit still
>>> below 40 chars as it looks). So once we're able to go down to e.g.
>>> 32 for the bulk of the code base, public headers should imo still
>>> be explicitly allowed to use longer identifiers.
>> 
>> Actually I thought about writing something for the public header but I
>> wasn't sure what to write. What about:
>> 
>> - Note: the Xen characters limit for identifiers is 41. Public headers
>> (xen/include/public/) are allowed to retain longer identifiers for
>> backward compatibility.
> 
> Fine with me, except I wonder in how far going forward we actually
> need to play by that limit there. Proper name-spacing is particularly
> important in the public headers, so may warrant a higher limit for
> certain (unusual?) circumstances.

I think we can have a “global” deviation for public headers with an higher
limit but there should still be a limit.

Bertrand

> 
> Jan

Hi,

On 25/05/2022 01:35, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> Add Rule 5.1, with the additional note that the character limit for Xen
> is 63 characters.

63 is a bit an odd numbers. Why not 64?

> 
> The max length identifiers found by ECLAIR are:
> 
> __mitigate_spectre_bhb_clear_insn_start
> domain_pause_by_systemcontroller_nosync
> 
> Both of them are 40 characters long. A limit of 63 characters work for
> the existing code.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>

Regardless what I wrote above:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

On 25/05/2022 08:40, Julien Grall wrote:
> Hi,
>
> On 25/05/2022 01:35, Stefano Stabellini wrote:
>> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>
>> Add Rule 5.1, with the additional note that the character limit for Xen
>> is 63 characters.
>
> 63 is a bit an odd numbers. Why not 64?

Because of the NUL terminator in a power-of-2 buffer in the compiler. 
That's why all of these spec limits exist in the first place.

~Andrew