| Message ID | e67bd46f204bef64cefdbe7a0b447148f7f9c9c6.1691162261.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: address MISRA C:2012 Rule 5.3 | expand |
On Fri, 4 Aug 2023, Nicola Vetrini wrote: > The variable 'msec' shadows the local variable in > 'ehci_dbgp_bios_handoff', but to prevent any future clashes, the one in > the macro gains a suffix. > > No functional change. > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
On 04.08.2023 17:27, Nicola Vetrini wrote: > --- a/xen/include/xen/delay.h > +++ b/xen/include/xen/delay.h > @@ -5,6 +5,6 @@ > > #include <asm/delay.h> > #define mdelay(n) (\ > - {unsigned long msec=(n); while (msec--) udelay(1000);}) > + {unsigned long msec_=(n); while (msec_--) udelay(1000);}) As elsewhere, please also adjust style while touching the line, at least as far as the obviously wrong case goes: #define mdelay(n) (\ {unsigned long msec_ = (n); while (msec_--) udelay(1000);}) Even better would be #define mdelay(n) ({ \ unsigned long msec_ = (n); while (msec_--) udelay(1000); \ }) or some such. I can take care of this while committing. Jan
Hi Jan, On 07/08/2023 09:14, Jan Beulich wrote: > On 04.08.2023 17:27, Nicola Vetrini wrote: >> --- a/xen/include/xen/delay.h >> +++ b/xen/include/xen/delay.h >> @@ -5,6 +5,6 @@ >> >> #include <asm/delay.h> >> #define mdelay(n) (\ >> - {unsigned long msec=(n); while (msec--) udelay(1000);}) >> + {unsigned long msec_=(n); while (msec_--) udelay(1000);}) > > As elsewhere, please also adjust style while touching the line, at > least as far as the obviously wrong case goes: > > #define mdelay(n) (\ > {unsigned long msec_ = (n); while (msec_--) udelay(1000);}) > > Even better would be > > #define mdelay(n) ({ \ > unsigned long msec_ = (n); while (msec_--) udelay(1000); \ > }) If you are touching the style, about converting to a staging inline and also splitting the line in multiple one? Cheers,
On 07.08.2023 11:01, Julien Grall wrote: > On 07/08/2023 09:14, Jan Beulich wrote: >> On 04.08.2023 17:27, Nicola Vetrini wrote: >>> --- a/xen/include/xen/delay.h >>> +++ b/xen/include/xen/delay.h >>> @@ -5,6 +5,6 @@ >>> >>> #include <asm/delay.h> >>> #define mdelay(n) (\ >>> - {unsigned long msec=(n); while (msec--) udelay(1000);}) >>> + {unsigned long msec_=(n); while (msec_--) udelay(1000);}) >> >> As elsewhere, please also adjust style while touching the line, at >> least as far as the obviously wrong case goes: >> >> #define mdelay(n) (\ >> {unsigned long msec_ = (n); while (msec_--) udelay(1000);}) >> >> Even better would be >> >> #define mdelay(n) ({ \ >> unsigned long msec_ = (n); while (msec_--) udelay(1000); \ >> }) > > If you are touching the style, about converting to a staging inline and > also splitting the line in multiple one? I'd be happy about this being done, but I wouldn't want to go as far with on-commit adjustments. Nicola, are you up to doing so in v2? Jan
On 07/08/2023 11:15, Jan Beulich wrote: > On 07.08.2023 11:01, Julien Grall wrote: >> On 07/08/2023 09:14, Jan Beulich wrote: >>> On 04.08.2023 17:27, Nicola Vetrini wrote: >>>> --- a/xen/include/xen/delay.h >>>> +++ b/xen/include/xen/delay.h >>>> @@ -5,6 +5,6 @@ >>>> >>>> #include <asm/delay.h> >>>> #define mdelay(n) (\ >>>> - {unsigned long msec=(n); while (msec--) udelay(1000);}) >>>> + {unsigned long msec_=(n); while (msec_--) udelay(1000);}) >>> >>> As elsewhere, please also adjust style while touching the line, at >>> least as far as the obviously wrong case goes: >>> >>> #define mdelay(n) (\ >>> {unsigned long msec_ = (n); while (msec_--) udelay(1000);}) >>> >>> Even better would be >>> >>> #define mdelay(n) ({ \ >>> unsigned long msec_ = (n); while (msec_--) udelay(1000); \ >>> }) >> >> If you are touching the style, about converting to a staging inline >> and >> also splitting the line in multiple one? > > I'd be happy about this being done, but I wouldn't want to go as far > with > on-commit adjustments. Nicola, are you up to doing so in v2? > > Jan I'm afraid I don't understand what "staging inline" refers to. Other than that, sure thing.
On 07.08.2023 11:23, Nicola Vetrini wrote: > On 07/08/2023 11:15, Jan Beulich wrote: >> On 07.08.2023 11:01, Julien Grall wrote: >>> On 07/08/2023 09:14, Jan Beulich wrote: >>>> On 04.08.2023 17:27, Nicola Vetrini wrote: >>>>> --- a/xen/include/xen/delay.h >>>>> +++ b/xen/include/xen/delay.h >>>>> @@ -5,6 +5,6 @@ >>>>> >>>>> #include <asm/delay.h> >>>>> #define mdelay(n) (\ >>>>> - {unsigned long msec=(n); while (msec--) udelay(1000);}) >>>>> + {unsigned long msec_=(n); while (msec_--) udelay(1000);}) >>>> >>>> As elsewhere, please also adjust style while touching the line, at >>>> least as far as the obviously wrong case goes: >>>> >>>> #define mdelay(n) (\ >>>> {unsigned long msec_ = (n); while (msec_--) udelay(1000);}) >>>> >>>> Even better would be >>>> >>>> #define mdelay(n) ({ \ >>>> unsigned long msec_ = (n); while (msec_--) udelay(1000); \ >>>> }) >>> >>> If you are touching the style, about converting to a staging inline >>> and >>> also splitting the line in multiple one? >> >> I'd be happy about this being done, but I wouldn't want to go as far >> with >> on-commit adjustments. Nicola, are you up to doing so in v2? > > I'm afraid I don't understand what "staging inline" refers to. Other > than that, sure thing. Surely Julien meant static inline. Jan
On 07/08/2023 10:32, Jan Beulich wrote: > On 07.08.2023 11:23, Nicola Vetrini wrote: >> On 07/08/2023 11:15, Jan Beulich wrote: >>> On 07.08.2023 11:01, Julien Grall wrote: >>>> On 07/08/2023 09:14, Jan Beulich wrote: >>>>> On 04.08.2023 17:27, Nicola Vetrini wrote: >>>>>> --- a/xen/include/xen/delay.h >>>>>> +++ b/xen/include/xen/delay.h >>>>>> @@ -5,6 +5,6 @@ >>>>>> >>>>>> #include <asm/delay.h> >>>>>> #define mdelay(n) (\ >>>>>> - {unsigned long msec=(n); while (msec--) udelay(1000);}) >>>>>> + {unsigned long msec_=(n); while (msec_--) udelay(1000);}) >>>>> >>>>> As elsewhere, please also adjust style while touching the line, at >>>>> least as far as the obviously wrong case goes: >>>>> >>>>> #define mdelay(n) (\ >>>>> {unsigned long msec_ = (n); while (msec_--) udelay(1000);}) >>>>> >>>>> Even better would be >>>>> >>>>> #define mdelay(n) ({ \ >>>>> unsigned long msec_ = (n); while (msec_--) udelay(1000); \ >>>>> }) >>>> >>>> If you are touching the style, about converting to a staging inline >>>> and >>>> also splitting the line in multiple one? >>> >>> I'd be happy about this being done, but I wouldn't want to go as far >>> with >>> on-commit adjustments. Nicola, are you up to doing so in v2? >> >> I'm afraid I don't understand what "staging inline" refers to. Other >> than that, sure thing. > > Surely Julien meant static inline. Yes. That was a typo. Sorry for that. Cheers,
diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h index 9d70ef035f..f2d9270e83 100644 --- a/xen/include/xen/delay.h +++ b/xen/include/xen/delay.h @@ -5,6 +5,6 @@ #include <asm/delay.h> #define mdelay(n) (\ - {unsigned long msec=(n); while (msec--) udelay(1000);}) + {unsigned long msec_=(n); while (msec_--) udelay(1000);}) #endif /* defined(_LINUX_DELAY_H) */
The variable 'msec' shadows the local variable in 'ehci_dbgp_bios_handoff', but to prevent any future clashes, the one in the macro gains a suffix. No functional change. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- Unless that file should remain as-is, because it's clearly taken from linux, but this does not prevent future name clashes with msec. --- xen/include/xen/delay.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)