| Message ID | 1c5c93ecbd8c46a338b22a4ef52e51648e333c01.1702746240.git.marcelo.schmitt1@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add support for AD7091R-2/-4/-8 | expand |
On Sat, 2023-12-16 at 14:45 -0300, Marcelo Schmitt wrote: > Checkpatch presumes attributes marked with __aligned(alignment) are part > of a function declaration and throws a warning stating that those > compiler attributes should have an identifier name which is not correct. > Add __aligned compiler attributes to the list of attribute notes > so they don't cause warnings anymore. > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > --- > Any expression that evaluates to an integer that is a power of 2 can be > within __aligned parenthesis. > I can't see how we could use a regex to check code meets such constraint (if possible). You can't because if a #define is used for the alignment value it is not necessarily available to a patch fragment or even a file if the #define is in an #include. > Some additional exotic uses of __aligned are: > drivers/net/wireless/quantenna/qtnfmac/bus.h:72: char bus_priv[] __aligned(sizeof(void *)); > include/linux/netdevice.h:225:} __aligned(4 * sizeof(unsigned long)); Right, then there are random uses like: drivers/firmware/qcom/qcom_qseecom_uefisecapp.c: size_t __aligned; \ drivers/firmware/qcom/qcom_qseecom_uefisecapp.c: *__offset = __aligned; \ so there's always some false positive/negative issue with checkpatch. Anyway: Acked-by: Joe Perches <joe@perches.com> > > The regex > __aligned\s*\(.*\) > seems to match all use cases. > > We might not catch invalid arguments to __aligned, but it looks like making > checkpath confidently report those wouldn't be feasible anyway. > > The patch that would trigger the mentioned warning in now > patch number 13 (iio: adc: Add support for AD7091R-8). > > scripts/checkpatch.pl | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 25fdb7fda112..d56c98146da3 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -512,6 +512,7 @@ our $Attribute = qr{ > __ro_after_init| > __kprobes| > $InitAttribute| > + __aligned\s*\(.*\)| > ____cacheline_aligned| > ____cacheline_aligned_in_smp| > ____cacheline_internodealigned_in_smp|
On Sat, 16 Dec 2023 10:07:24 -0800 Joe Perches <joe@perches.com> wrote: > On Sat, 2023-12-16 at 14:45 -0300, Marcelo Schmitt wrote: > > Checkpatch presumes attributes marked with __aligned(alignment) are part > > of a function declaration and throws a warning stating that those > > compiler attributes should have an identifier name which is not correct. > > Add __aligned compiler attributes to the list of attribute notes > > so they don't cause warnings anymore. > > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > > --- > > Any expression that evaluates to an integer that is a power of 2 can be > > within __aligned parenthesis. > > I can't see how we could use a regex to check code meets such constraint (if possible). > > You can't because if a #define is used for the alignment > value it is not necessarily available to a patch fragment > or even a file if the #define is in an #include. > > > Some additional exotic uses of __aligned are: > > drivers/net/wireless/quantenna/qtnfmac/bus.h:72: char bus_priv[] __aligned(sizeof(void *)); > > include/linux/netdevice.h:225:} __aligned(4 * sizeof(unsigned long)); > > Right, then there are random uses like: > > drivers/firmware/qcom/qcom_qseecom_uefisecapp.c: size_t __aligned; \ > drivers/firmware/qcom/qcom_qseecom_uefisecapp.c: *__offset = __aligned; \ > > so there's always some false positive/negative issue > with checkpatch. > > Anyway: > > Acked-by: Joe Perches <joe@perches.com> Given this should cut down on false postives I've picked this one up now. Applied to the togreg branch of iio.git and pushed out briefly as testing to see what blows up. I'll see if I can pick up some more parts of this series to avoid us going round again with quite so many patches. Jonathan > > > > > The regex > > __aligned\s*\(.*\) > > seems to match all use cases. > > > > We might not catch invalid arguments to __aligned, but it looks like making > > checkpath confidently report those wouldn't be feasible anyway. > > > > The patch that would trigger the mentioned warning in now > > patch number 13 (iio: adc: Add support for AD7091R-8). > > > > scripts/checkpatch.pl | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 25fdb7fda112..d56c98146da3 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -512,6 +512,7 @@ our $Attribute = qr{ > > __ro_after_init| > > __kprobes| > > $InitAttribute| > > + __aligned\s*\(.*\)| > > ____cacheline_aligned| > > ____cacheline_aligned_in_smp| > > ____cacheline_internodealigned_in_smp| > >
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 25fdb7fda112..d56c98146da3 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -512,6 +512,7 @@ our $Attribute = qr{ __ro_after_init| __kprobes| $InitAttribute| + __aligned\s*\(.*\)| ____cacheline_aligned| ____cacheline_aligned_in_smp| ____cacheline_internodealigned_in_smp|
Checkpatch presumes attributes marked with __aligned(alignment) are part of a function declaration and throws a warning stating that those compiler attributes should have an identifier name which is not correct. Add __aligned compiler attributes to the list of attribute notes so they don't cause warnings anymore. Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> --- Any expression that evaluates to an integer that is a power of 2 can be within __aligned parenthesis. I can't see how we could use a regex to check code meets such constraint (if possible). Some additional exotic uses of __aligned are: drivers/net/wireless/quantenna/qtnfmac/bus.h:72: char bus_priv[] __aligned(sizeof(void *)); include/linux/netdevice.h:225:} __aligned(4 * sizeof(unsigned long)); The regex __aligned\s*\(.*\) seems to match all use cases. We might not catch invalid arguments to __aligned, but it looks like making checkpath confidently report those wouldn't be feasible anyway. The patch that would trigger the mentioned warning in now patch number 13 (iio: adc: Add support for AD7091R-8). scripts/checkpatch.pl | 1 + 1 file changed, 1 insertion(+)