* [PATCH v3] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS
@ 2020-11-17 17:18 Aditya Srivastava
2020-11-20 10:41 ` Aditya
0 siblings, 1 reply; 7+ messages in thread
From: Aditya Srivastava @ 2020-11-17 17:18 UTC (permalink / raw)
To: joe; +Cc: yashsri421, linux-kernel-mentees, linux-kernel, lukas.bulwahn
Currently, checkpatch warns us if an assignment operator is placed
at the start of a line and not at the end of previous line.
E.g., running checkpatch on commit 8195b1396ec8 ("hv_netvsc: fix
deadlock on hotplug") reports:
CHECK: Assignment operator '=' should be on the previous line
+ struct netvsc_device *nvdev
+ = container_of(w, struct netvsc_device, subchan_work);
Provide a simple fix by appending assignment operator to the previous
line and removing from the current line, if both the lines are additions
(ie start with '+')
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
Changes in v2:
add check if both the lines are additions (ie start with '+')
Changes in v3:
quote $operator; test with division assignment operator ('/=')
scripts/checkpatch.pl | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c9aaaa443265..d5bc4d8e4f6c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3542,8 +3542,14 @@ sub process {
# check for assignments on the start of a line
if ($sline =~ /^\+\s+($Assignment)[^=]/) {
- CHK("ASSIGNMENT_CONTINUATIONS",
- "Assignment operator '$1' should be on the previous line\n" . $hereprev);
+ my $operator = "$1";
+ if (CHK("ASSIGNMENT_CONTINUATIONS",
+ "Assignment operator '$1' should be on the previous line\n" . $hereprev) &&
+ $fix && $prevrawline =~ /^\+/) {
+ # add assignment operator to the previous line, remove from current line
+ $fixed[$fixlinenr - 1] .= " $operator";
+ $fixed[$fixlinenr] =~ s/$operator\s*//;
+ }
}
# check for && or || at the start of a line
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS
2020-11-17 17:18 [PATCH v3] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS Aditya Srivastava
@ 2020-11-20 10:41 ` Aditya
2020-11-20 17:26 ` Joe Perches
0 siblings, 1 reply; 7+ messages in thread
From: Aditya @ 2020-11-20 10:41 UTC (permalink / raw)
To: joe; +Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn
On 17/11/20 10:48 pm, Aditya Srivastava wrote:
> Currently, checkpatch warns us if an assignment operator is placed
> at the start of a line and not at the end of previous line.
>
> E.g., running checkpatch on commit 8195b1396ec8 ("hv_netvsc: fix
> deadlock on hotplug") reports:
>
> CHECK: Assignment operator '=' should be on the previous line
> + struct netvsc_device *nvdev
> + = container_of(w, struct netvsc_device, subchan_work);
>
> Provide a simple fix by appending assignment operator to the previous
> line and removing from the current line, if both the lines are additions
> (ie start with '+')
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> Changes in v2:
> add check if both the lines are additions (ie start with '+')
>
> Changes in v3:
> quote $operator; test with division assignment operator ('/=')
>
> scripts/checkpatch.pl | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index c9aaaa443265..d5bc4d8e4f6c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3542,8 +3542,14 @@ sub process {
>
> # check for assignments on the start of a line
> if ($sline =~ /^\+\s+($Assignment)[^=]/) {
> - CHK("ASSIGNMENT_CONTINUATIONS",
> - "Assignment operator '$1' should be on the previous line\n" . $hereprev);
> + my $operator = "$1";
> + if (CHK("ASSIGNMENT_CONTINUATIONS",
> + "Assignment operator '$1' should be on the previous line\n" . $hereprev) &&
> + $fix && $prevrawline =~ /^\+/) {
> + # add assignment operator to the previous line, remove from current line
> + $fixed[$fixlinenr - 1] .= " $operator";
> + $fixed[$fixlinenr] =~ s/$operator\s*//;
> + }
> }
>
> # check for && or || at the start of a line
>
Hi Joe
This patch probably got missed. Please review :)
Thanks
Aditya
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS
2020-11-20 10:41 ` Aditya
@ 2020-11-20 17:26 ` Joe Perches
2020-11-20 18:02 ` Aditya
0 siblings, 1 reply; 7+ messages in thread
From: Joe Perches @ 2020-11-20 17:26 UTC (permalink / raw)
To: Aditya; +Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn
On Fri, 2020-11-20 at 16:11 +0530, Aditya wrote:
> On 17/11/20 10:48 pm, Aditya Srivastava wrote:
> > Currently, checkpatch warns us if an assignment operator is placed
> > at the start of a line and not at the end of previous line.
> >
> > E.g., running checkpatch on commit 8195b1396ec8 ("hv_netvsc: fix
> > deadlock on hotplug") reports:
> >
> > CHECK: Assignment operator '=' should be on the previous line
> > + struct netvsc_device *nvdev
> > + = container_of(w, struct netvsc_device, subchan_work);
> >
> > Provide a simple fix by appending assignment operator to the previous
> > line and removing from the current line, if both the lines are additions
> > (ie start with '+')
> >
> > Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> > ---
> > Changes in v2:
> > add check if both the lines are additions (ie start with '+')
> >
> > Changes in v3:
> > quote $operator; test with division assignment operator ('/=')
> >
> > scripts/checkpatch.pl | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index c9aaaa443265..d5bc4d8e4f6c 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -3542,8 +3542,14 @@ sub process {
> >
> >
> > # check for assignments on the start of a line
> > if ($sline =~ /^\+\s+($Assignment)[^=]/) {
> > - CHK("ASSIGNMENT_CONTINUATIONS",
> > - "Assignment operator '$1' should be on the previous line\n" . $hereprev);
> > + my $operator = "$1";
> > + if (CHK("ASSIGNMENT_CONTINUATIONS",
> > + "Assignment operator '$1' should be on the previous line\n" . $hereprev) &&
> > + $fix && $prevrawline =~ /^\+/) {
> > + # add assignment operator to the previous line, remove from current line
> > + $fixed[$fixlinenr - 1] .= " $operator";
> > + $fixed[$fixlinenr] =~ s/$operator\s*//;
> > + }
> > }
> >
> >
> > # check for && or || at the start of a line
> >
>
> Hi Joe
> This patch probably got missed. Please review :)
Did you look at $Assignment? Did you see it can be /= ?
If it is, what happens in the $fixed[$fixlinenr] line?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS
2020-11-20 17:26 ` Joe Perches
@ 2020-11-20 18:02 ` Aditya
2020-11-21 11:50 ` Aditya
0 siblings, 1 reply; 7+ messages in thread
From: Aditya @ 2020-11-20 18:02 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn
On 20/11/20 10:56 pm, Joe Perches wrote:
> On Fri, 2020-11-20 at 16:11 +0530, Aditya wrote:
>> On 17/11/20 10:48 pm, Aditya Srivastava wrote:
>>> Currently, checkpatch warns us if an assignment operator is placed
>>> at the start of a line and not at the end of previous line.
>>>
>>> E.g., running checkpatch on commit 8195b1396ec8 ("hv_netvsc: fix
>>> deadlock on hotplug") reports:
>>>
>>> CHECK: Assignment operator '=' should be on the previous line
>>> + struct netvsc_device *nvdev
>>> + = container_of(w, struct netvsc_device, subchan_work);
>>>
>>> Provide a simple fix by appending assignment operator to the previous
>>> line and removing from the current line, if both the lines are additions
>>> (ie start with '+')
>>>
>>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>>> ---
>>> Changes in v2:
>>> add check if both the lines are additions (ie start with '+')
>>>
>>> Changes in v3:
>>> quote $operator; test with division assignment operator ('/=')
>>>
>>> scripts/checkpatch.pl | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index c9aaaa443265..d5bc4d8e4f6c 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -3542,8 +3542,14 @@ sub process {
>>>
>>>
>>> # check for assignments on the start of a line
>>> if ($sline =~ /^\+\s+($Assignment)[^=]/) {
>>> - CHK("ASSIGNMENT_CONTINUATIONS",
>>> - "Assignment operator '$1' should be on the previous line\n" . $hereprev);
>>> + my $operator = "$1";
>>> + if (CHK("ASSIGNMENT_CONTINUATIONS",
>>> + "Assignment operator '$1' should be on the previous line\n" . $hereprev) &&
>>> + $fix && $prevrawline =~ /^\+/) {
>>> + # add assignment operator to the previous line, remove from current line
>>> + $fixed[$fixlinenr - 1] .= " $operator";
>>> + $fixed[$fixlinenr] =~ s/$operator\s*//;
>>> + }
>>> }
>>>
>>>
>>> # check for && or || at the start of a line
>>>
>>
>> Hi Joe
>> This patch probably got missed. Please review :)
>
> Did you look at $Assignment? Did you see it can be /= ?
>
Yes, I tested the patch with '/=' operator as well.
As I could not find any occurrences in the past(over 4.13..5.8), I
created an example for myself by modifying the above mentioned commit
example i.e. commit 8195b1396ec8 ("hv_netvsc: fix deadlock on
hotplug") as:
+ struct netvsc_device *nvdev
+ /= container_of(w, struct netvsc_device, subchan_work);
[For Line 144 and 145(where the warning was reported for '=' earlier)]
The fix changes these lines to:
+ struct netvsc_device *nvdev /=
+ container_of(w, struct netvsc_device, subchan_work);
On retesting the patch with checkpatch.pl, it did not give this CHECK,
nor did we add any new warning/error.
> If it is, what happens in the $fixed[$fixlinenr] line?
>
In $fixed[$fixlinenr], we are just getting rid of the operator and any
space(s) following it.
What do you think?
Thanks
Aditya
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS
2020-11-20 18:02 ` Aditya
@ 2020-11-21 11:50 ` Aditya
2020-11-21 12:04 ` [PATCH v4] " Aditya Srivastava
0 siblings, 1 reply; 7+ messages in thread
From: Aditya @ 2020-11-21 11:50 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn
On 20/11/20 11:32 pm, Aditya wrote:
> On 20/11/20 10:56 pm, Joe Perches wrote:
>> On Fri, 2020-11-20 at 16:11 +0530, Aditya wrote:
>>> On 17/11/20 10:48 pm, Aditya Srivastava wrote:
>>>> Currently, checkpatch warns us if an assignment operator is placed
>>>> at the start of a line and not at the end of previous line.
>>>>
>>>> E.g., running checkpatch on commit 8195b1396ec8 ("hv_netvsc: fix
>>>> deadlock on hotplug") reports:
>>>>
>>>> CHECK: Assignment operator '=' should be on the previous line
>>>> + struct netvsc_device *nvdev
>>>> + = container_of(w, struct netvsc_device, subchan_work);
>>>>
>>>> Provide a simple fix by appending assignment operator to the previous
>>>> line and removing from the current line, if both the lines are additions
>>>> (ie start with '+')
>>>>
>>>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>>>> ---
>>>> Changes in v2:
>>>> add check if both the lines are additions (ie start with '+')
>>>>
>>>> Changes in v3:
>>>> quote $operator; test with division assignment operator ('/=')
>>>>
>>>> scripts/checkpatch.pl | 10 ++++++++--
>>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>>> index c9aaaa443265..d5bc4d8e4f6c 100755
>>>> --- a/scripts/checkpatch.pl
>>>> +++ b/scripts/checkpatch.pl
>>>> @@ -3542,8 +3542,14 @@ sub process {
>>>>
>>>>
>>>> # check for assignments on the start of a line
>>>> if ($sline =~ /^\+\s+($Assignment)[^=]/) {
>>>> - CHK("ASSIGNMENT_CONTINUATIONS",
>>>> - "Assignment operator '$1' should be on the previous line\n" . $hereprev);
>>>> + my $operator = "$1";
>>>> + if (CHK("ASSIGNMENT_CONTINUATIONS",
>>>> + "Assignment operator '$1' should be on the previous line\n" . $hereprev) &&
>>>> + $fix && $prevrawline =~ /^\+/) {
>>>> + # add assignment operator to the previous line, remove from current line
>>>> + $fixed[$fixlinenr - 1] .= " $operator";
>>>> + $fixed[$fixlinenr] =~ s/$operator\s*//;
>>>> + }
>>>> }
>>>>
>>>>
>>>> # check for && or || at the start of a line
>>>>
>>>
>>> Hi Joe
>>> This patch probably got missed. Please review :)
>>
>> Did you look at $Assignment? Did you see it can be /= ?
>>
>
> Yes, I tested the patch with '/=' operator as well.
> As I could not find any occurrences in the past(over 4.13..5.8), I
> created an example for myself by modifying the above mentioned commit
> example i.e. commit 8195b1396ec8 ("hv_netvsc: fix deadlock on
> hotplug") as:
>
> + struct netvsc_device *nvdev
> + /= container_of(w, struct netvsc_device, subchan_work);
>
> [For Line 144 and 145(where the warning was reported for '=' earlier)]
>
> The fix changes these lines to:
> + struct netvsc_device *nvdev /=
> + container_of(w, struct netvsc_device, subchan_work);
>
> On retesting the patch with checkpatch.pl, it did not give this CHECK,
> nor did we add any new warning/error.
>
>> If it is, what happens in the $fixed[$fixlinenr] line?
>>
>
> In $fixed[$fixlinenr], we are just getting rid of the operator and any
> space(s) following it.
>
> What do you think?
>
I think I understood quoting incorrectly here. I had to use \Q.
I'll resend the modified patch with desired changes.
Sorry for inconvenience.
Thanks
Aditya
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS
2020-11-21 11:50 ` Aditya
@ 2020-11-21 12:04 ` Aditya Srivastava
2020-11-21 12:10 ` Joe Perches
0 siblings, 1 reply; 7+ messages in thread
From: Aditya Srivastava @ 2020-11-21 12:04 UTC (permalink / raw)
To: joe; +Cc: yashsri421, lukas.bulwahn, linux-kernel-mentees, linux-kernel
Currently, checkpatch warns us if an assignment operator is placed
at the start of a line and not at the end of previous line.
E.g., running checkpatch on commit 8195b1396ec8 ("hv_netvsc: fix
deadlock on hotplug") reports:
CHECK: Assignment operator '=' should be on the previous line
+ struct netvsc_device *nvdev
+ = container_of(w, struct netvsc_device, subchan_work);
Provide a simple fix by appending assignment operator to the previous
line and removing from the current line, if both the lines are additions
(ie start with '+')
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
Changes in v2:
add check if both the lines are additions (ie start with '+')
Changes in v3:
quote $operator; test with division assignment operator ('/=')
Changes in v4:
fix incorrect use of quote
scripts/checkpatch.pl | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2749f32dffe9..d4c8d42cb13e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3533,8 +3533,14 @@ sub process {
# check for assignments on the start of a line
if ($sline =~ /^\+\s+($Assignment)[^=]/) {
- CHK("ASSIGNMENT_CONTINUATIONS",
- "Assignment operator '$1' should be on the previous line\n" . $hereprev);
+ my $operator = $1;
+ if (CHK("ASSIGNMENT_CONTINUATIONS",
+ "Assignment operator '$1' should be on the previous line\n" . $hereprev) &&
+ $fix && $prevrawline =~ /^\+/) {
+ # add assignment operator to the previous line, remove from current line
+ $fixed[$fixlinenr - 1] .= " $operator";
+ $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//;
+ }
}
# check for && or || at the start of a line
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS
2020-11-21 12:04 ` [PATCH v4] " Aditya Srivastava
@ 2020-11-21 12:10 ` Joe Perches
0 siblings, 0 replies; 7+ messages in thread
From: Joe Perches @ 2020-11-21 12:10 UTC (permalink / raw)
To: Aditya Srivastava, Andrew Morton
Cc: lukas.bulwahn, linux-kernel-mentees, linux-kernel
On Sat, 2020-11-21 at 17:34 +0530, Aditya Srivastava wrote:
> Currently, checkpatch warns us if an assignment operator is placed
> at the start of a line and not at the end of previous line.
Right, thanks.
Acked-by: Joe Perches <joe@perches.com>
>
> E.g., running checkpatch on commit 8195b1396ec8 ("hv_netvsc: fix
> deadlock on hotplug") reports:
>
> CHECK: Assignment operator '=' should be on the previous line
> + struct netvsc_device *nvdev
> + = container_of(w, struct netvsc_device, subchan_work);
>
> Provide a simple fix by appending assignment operator to the previous
> line and removing from the current line, if both the lines are additions
> (ie start with '+')
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> Changes in v2:
> add check if both the lines are additions (ie start with '+')
>
> Changes in v3:
> quote $operator; test with division assignment operator ('/=')
>
> Changes in v4:
> fix incorrect use of quote
>
> scripts/checkpatch.pl | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2749f32dffe9..d4c8d42cb13e 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3533,8 +3533,14 @@ sub process {
>
>
> # check for assignments on the start of a line
> if ($sline =~ /^\+\s+($Assignment)[^=]/) {
> - CHK("ASSIGNMENT_CONTINUATIONS",
> - "Assignment operator '$1' should be on the previous line\n" . $hereprev);
> + my $operator = $1;
> + if (CHK("ASSIGNMENT_CONTINUATIONS",
> + "Assignment operator '$1' should be on the previous line\n" . $hereprev) &&
> + $fix && $prevrawline =~ /^\+/) {
> + # add assignment operator to the previous line, remove from current line
> + $fixed[$fixlinenr - 1] .= " $operator";
> + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//;
> + }
> }
>
>
> # check for && or || at the start of a line
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-21 12:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 17:18 [PATCH v3] checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS Aditya Srivastava
2020-11-20 10:41 ` Aditya
2020-11-20 17:26 ` Joe Perches
2020-11-20 18:02 ` Aditya
2020-11-21 11:50 ` Aditya
2020-11-21 12:04 ` [PATCH v4] " Aditya Srivastava
2020-11-21 12:10 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).