diff mbox series

[ovs-dev,v2] ovs-tcpdump: Fix an undefined variable

Message ID 20190201013908.4894-1-hyonkim@cisco.com
State Accepted
Headers show
Series [ovs-dev,v2] ovs-tcpdump: Fix an undefined variable | expand

Commit Message

Li,Rongqing via dev Feb. 1, 2019, 1:39 a.m. UTC
Run ovs-tcpdump without --span, and it throws the following
exception. Define mirror_select_all to avoid the error.

Traceback (most recent call last):
  File "/usr/local/bin/ovs-tcpdump", line 488, in <module>
    main()
  File "/usr/local/bin/ovs-tcpdump", line 454, in main
    mirror_select_all)
UnboundLocalError: local variable 'mirror_select_all' referenced before assignment

Fixes: 0475db71c650 ("ovs-tcpdump: Add --span to mirror all ports on bridge.")

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
---
v2:
* fix a typo: with -> without
* resend after subscribing to dev to avoid ovs-dev in From:

 utilities/ovs-tcpdump.in | 1 +
 1 file changed, 1 insertion(+)

Comments

0-day Robot Feb. 1, 2019, 1:59 a.m. UTC | #1
Bleep bloop.  Greetings Hyong Youb Kim via dev, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author should not be mailing list.
Lines checked: 38, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Li,Rongqing via dev Feb. 1, 2019, 2:10 a.m. UTC | #2
On Thu, Jan 31, 2019 at 08:59:31PM -0500, 0-day Robot wrote:
> Bleep bloop.  Greetings Hyong Youb Kim via dev, I am a robot and I have tried out your patch.
> Thanks for your contribution.
> 
> I encountered some error that I wasn't expecting.  See the details below.
> 
> 
> checkpatch:
> ERROR: Author should not be mailing list.
> Lines checked: 38, Warnings: 0, Errors: 1

Hmm, I am lost :-) I do not understand why the list is rewriting
"From:".  Never had this issue with other lists..

These are the headers in the sent/CC'd email.

From: Hyong Youb Kim <hyonkim@cisco.com>
To: dev@openvswitch.org
Cc: Martin Fong <mwfong@csl.sri.com>, Hyong Youb Kim <hyonkim@cisco.com>
Subject: [PATCH v2] ovs-tcpdump: Fix an undefined variable
[...]

-Hyong
Ilya Maximets Feb. 1, 2019, 3:37 p.m. UTC | #3
> Run ovs-tcpdump without --span, and it throws the following
> exception. Define mirror_select_all to avoid the error.
> 
> Traceback (most recent call last):
>   File "/usr/local/bin/ovs-tcpdump", line 488, in <module>
>     main()
>   File "/usr/local/bin/ovs-tcpdump", line 454, in main
>     mirror_select_all)
> UnboundLocalError: local variable 'mirror_select_all' referenced before assignment
> 
> Fixes: 0475db71c650 ("ovs-tcpdump: Add --span to mirror all ports on bridge.")
> 
> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> ---
> v2:
> * fix a typo: with -> without
> * resend after subscribing to dev to avoid ovs-dev in From:
> 
>  utilities/ovs-tcpdump.in | 1 +
>  1 file changed, 1 insertion(+)

Thanks for the fix,
Acked-by: Ilya Maximets <i.maximets@samsung.com>

Regarding checkpatch. Looks like it's a patchwork issue.
It has mail-list email in a form field. Mail-list itself shows correct e-mail.
I hope, maintainers will use correct one while applying the patch, if needed.

Another option (workaround): You may add "From: Name <e-mail>" as a first line
of the commit-message. This should be correctly treated by git in any case.

> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index 22f249f58..269c252f8 100755
> --- a/utilities/ovs-tcpdump.in
> +++ b/utilities/ovs-tcpdump.in
> @@ -379,6 +379,7 @@  def main():
>  
>      skip_next = False
>      mirror_interface = None
> +    mirror_select_all = False
>      dump_cmd = 'tcpdump'
>  
>      for cur, nxt in argv_tuples(sys.argv[1:]):
>
Aaron Conole Feb. 1, 2019, 3:47 p.m. UTC | #4
Ilya Maximets <i.maximets@samsung.com> writes:

>> Run ovs-tcpdump without --span, and it throws the following
>> exception. Define mirror_select_all to avoid the error.
>> 
>> Traceback (most recent call last):
>>   File "/usr/local/bin/ovs-tcpdump", line 488, in <module>
>>     main()
>>   File "/usr/local/bin/ovs-tcpdump", line 454, in main
>>     mirror_select_all)
>> UnboundLocalError: local variable 'mirror_select_all' referenced before assignment
>> 
>> Fixes: 0475db71c650 ("ovs-tcpdump: Add --span to mirror all ports on bridge.")
>> 
>> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
>> ---
>> v2:
>> * fix a typo: with -> without
>> * resend after subscribing to dev to avoid ovs-dev in From:
>> 
>>  utilities/ovs-tcpdump.in | 1 +
>>  1 file changed, 1 insertion(+)
>
> Thanks for the fix,
> Acked-by: Ilya Maximets <i.maximets@samsung.com>
>
> Regarding checkpatch. Looks like it's a patchwork issue.
> It has mail-list email in a form field. Mail-list itself shows correct e-mail.
> I hope, maintainers will use correct one while applying the patch, if needed.

I don't see that the email address is correct.  Actually in my email
client, it does say:

   From: Hyong Youb Kim via dev <ovs-dev@openvswitch.org>

I'm not sure what causes this (again, I'm going from the email I
received), but we've seen it before.

> Another option (workaround): You may add "From: Name <e-mail>" as a first line
> of the commit-message. This should be correctly treated by git in any case.

Also, you can try attaching the patch to an email to the list (I think
that can work), or submitting a pull request on github and sending the
list an email with the information.

Very strange, though.

>> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
>> index 22f249f58..269c252f8 100755
>> --- a/utilities/ovs-tcpdump.in
>> +++ b/utilities/ovs-tcpdump.in
>> @@ -379,6 +379,7 @@  def main():
>>  
>>      skip_next = False
>>      mirror_interface = None
>> +    mirror_select_all = False
>>      dump_cmd = 'tcpdump'
>>  
>>      for cur, nxt in argv_tuples(sys.argv[1:]):
>>
diff mbox series

Patch

diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index 22f249f58..269c252f8 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -379,6 +379,7 @@  def main():
 
     skip_next = False
     mirror_interface = None
+    mirror_select_all = False
     dump_cmd = 'tcpdump'
 
     for cur, nxt in argv_tuples(sys.argv[1:]):