diff mbox series

fix Rule 10.2 violation

Message ID alpine.DEB.2.22.394.2405101634500.2544314@ubuntu-linux-20-04-desktop (mailing list archive)
State New, archived
Headers show
Series fix Rule 10.2 violation | expand

Commit Message

Stefano Stabellini May 10, 2024, 11:37 p.m. UTC
Change opt_conswitch to char to fix a violation of Rule 10.2.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Comments

Julien Grall May 13, 2024, 9:28 p.m. UTC | #1
Hi Stefano,

title: Is this the only violation we have in Xen? If so, then please add 
the subsystem in the title.

On 11/05/2024 00:37, Stefano Stabellini wrote:
> Change opt_conswitch to char to fix a violation of Rule 10.2.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 2c363d9c1d..3a3a97bcbe 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -49,7 +49,7 @@ string_param("console", opt_console);
>   /* Char 1: CTRL+<char1> is used to switch console input between Xen and DOM0 */
>   /* Char 2: If this character is 'x', then do not auto-switch to DOM0 when it */
>   /*         boots. Any other value, or omitting the char, enables auto-switch */
> -static unsigned char __read_mostly opt_conswitch[3] = "a";
> +static char __read_mostly opt_conswitch[3] = "a";

Looking at the rest of the code, we have:

#define switch_code (opt_conswitch[0] - 'a' + 1)

Can you confirm whether this is not somehow adding a new violation?

Cheers,
Stefano Stabellini May 14, 2024, 12:09 a.m. UTC | #2
On Mon, 13 May 2024, Julien Grall wrote:
> Hi Stefano,
> 
> title: Is this the only violation we have in Xen? If so, then please add the
> subsystem in the title.

The only remaining violations are about the use of the "toupper" macro.
Bugseng is recommending to add a cast to fix those or deviate toupper.


> On 11/05/2024 00:37, Stefano Stabellini wrote:
> > Change opt_conswitch to char to fix a violation of Rule 10.2.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > 
> > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > index 2c363d9c1d..3a3a97bcbe 100644
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -49,7 +49,7 @@ string_param("console", opt_console);
> >   /* Char 1: CTRL+<char1> is used to switch console input between Xen and
> > DOM0 */
> >   /* Char 2: If this character is 'x', then do not auto-switch to DOM0 when
> > it */
> >   /*         boots. Any other value, or omitting the char, enables
> > auto-switch */
> > -static unsigned char __read_mostly opt_conswitch[3] = "a";
> > +static char __read_mostly opt_conswitch[3] = "a";
> 
> Looking at the rest of the code, we have:
> 
> #define switch_code (opt_conswitch[0] - 'a' + 1)
> 
> Can you confirm whether this is not somehow adding a new violation?

No, this patch is to fix a violation exactly there.
Jan Beulich May 14, 2024, 8:38 a.m. UTC | #3
On 14.05.2024 02:09, Stefano Stabellini wrote:
> On Mon, 13 May 2024, Julien Grall wrote:
>> Hi Stefano,
>>
>> title: Is this the only violation we have in Xen? If so, then please add the
>> subsystem in the title.
> 
> The only remaining violations are about the use of the "toupper" macro.
> Bugseng is recommending to add a cast to fix those or deviate toupper.
> 
> 
>> On 11/05/2024 00:37, Stefano Stabellini wrote:
>>> Change opt_conswitch to char to fix a violation of Rule 10.2.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>
>>> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
>>> index 2c363d9c1d..3a3a97bcbe 100644
>>> --- a/xen/drivers/char/console.c
>>> +++ b/xen/drivers/char/console.c
>>> @@ -49,7 +49,7 @@ string_param("console", opt_console);
>>>   /* Char 1: CTRL+<char1> is used to switch console input between Xen and
>>> DOM0 */
>>>   /* Char 2: If this character is 'x', then do not auto-switch to DOM0 when
>>> it */
>>>   /*         boots. Any other value, or omitting the char, enables
>>> auto-switch */
>>> -static unsigned char __read_mostly opt_conswitch[3] = "a";
>>> +static char __read_mostly opt_conswitch[3] = "a";
>>
>> Looking at the rest of the code, we have:
>>
>> #define switch_code (opt_conswitch[0] - 'a' + 1)
>>
>> Can you confirm whether this is not somehow adding a new violation?
> 
> No, this patch is to fix a violation exactly there.

Right. With a suitable subject prefix added:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Julien Grall May 14, 2024, 11:19 a.m. UTC | #4
Hi Stefano,

On 14/05/2024 01:09, Stefano Stabellini wrote:
> On Mon, 13 May 2024, Julien Grall wrote:
>> Hi Stefano,
>>
>> title: Is this the only violation we have in Xen? If so, then please add the
>> subsystem in the title.
> 
> The only remaining violations are about the use of the "toupper" macro.
> Bugseng is recommending to add a cast to fix those or deviate toupper.

Ok. Please can you add a prefix in the title then?

> 
> 
>> On 11/05/2024 00:37, Stefano Stabellini wrote:
>>> Change opt_conswitch to char to fix a violation of Rule 10.2.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>
>>> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
>>> index 2c363d9c1d..3a3a97bcbe 100644
>>> --- a/xen/drivers/char/console.c
>>> +++ b/xen/drivers/char/console.c
>>> @@ -49,7 +49,7 @@ string_param("console", opt_console);
>>>    /* Char 1: CTRL+<char1> is used to switch console input between Xen and
>>> DOM0 */
>>>    /* Char 2: If this character is 'x', then do not auto-switch to DOM0 when
>>> it */
>>>    /*         boots. Any other value, or omitting the char, enables
>>> auto-switch */
>>> -static unsigned char __read_mostly opt_conswitch[3] = "a";
>>> +static char __read_mostly opt_conswitch[3] = "a";
>>
>> Looking at the rest of the code, we have:
>>
>> #define switch_code (opt_conswitch[0] - 'a' + 1)
>>
>> Can you confirm whether this is not somehow adding a new violation?
> 
> No, this patch is to fix a violation exactly there.

Thanks for the confirmation:

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

Cheers,
diff mbox series

Patch

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2c363d9c1d..3a3a97bcbe 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -49,7 +49,7 @@  string_param("console", opt_console);
 /* Char 1: CTRL+<char1> is used to switch console input between Xen and DOM0 */
 /* Char 2: If this character is 'x', then do not auto-switch to DOM0 when it */
 /*         boots. Any other value, or omitting the char, enables auto-switch */
-static unsigned char __read_mostly opt_conswitch[3] = "a";
+static char __read_mostly opt_conswitch[3] = "a";
 string_runtime_param("conswitch", opt_conswitch);
 
 /* sync_console: force synchronous console output (useful for debugging). */