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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > ---
> > 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?
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 <[email protected]>
>>> ---
>>> 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
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 <[email protected]>
>>>> ---
>>>> 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
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 <[email protected]>
---
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
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 <[email protected]>
>
> 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 <[email protected]>
> ---
> 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