| Message ID | 20170316142232.25959-1-e@erig.me |
|---|---|
| State | Accepted |
| Headers | show |
Eric Garver <e@erig.me> writes: > We need to use == instead of the is operator. If you're unlucky it may > fail because they're not exactly the same object, but hold the same > value. > > Example false positive: > > E(120): Inappropriate bracing around statement > > + if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING]) I count an unbalanced brace. There may be something wrong, but at least from the example, I don't see it. Can you include a bit more context? Perhaps the hunk that failed? -Aaron
On Thu, Mar 16, 2017 at 10:37 AM, Aaron Conole <aconole@redhat.com> wrote: > Eric Garver <e@erig.me> writes: > >> We need to use == instead of the is operator. If you're unlucky it may >> fail because they're not exactly the same object, but hold the same >> value. >> >> Example false positive: >> >> E(120): Inappropriate bracing around statement >> >> + if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING]) > > I count an unbalanced brace. There may be something wrong, but at least > from the example, I don't see it. > > Can you include a bit more context? Perhaps the hunk that failed? I applied this to master before seeing your reply and request for more info. Sorry about that. I applied this because I think the change is correct, but I agree that the example appears to indeed be imbalanced.
Russell Bryant <russell@ovn.org> writes: > On Thu, Mar 16, 2017 at 10:37 AM, Aaron Conole <aconole@redhat.com> wrote: >> Eric Garver <e@erig.me> writes: >> >>> We need to use == instead of the is operator. If you're unlucky it may >>> fail because they're not exactly the same object, but hold the same >>> value. >>> >>> Example false positive: >>> >>> E(120): Inappropriate bracing around statement >>> >>> + if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING]) >> >> I count an unbalanced brace. There may be something wrong, but at least >> from the example, I don't see it. >> >> Can you include a bit more context? Perhaps the hunk that failed? > > I applied this to master before seeing your reply and request for more > info. Sorry about that. > > I applied this because I think the change is correct, but I agree that > the example appears to indeed be imbalanced. +1 - I do agree with the change, fwiw :)
On Thu, Mar 16, 2017 at 10:37:00AM -0400, Aaron Conole wrote: > Eric Garver <e@erig.me> writes: > > > We need to use == instead of the is operator. If you're unlucky it may > > fail because they're not exactly the same object, but hold the same > > value. > > > > Example false positive: > > > > E(120): Inappropriate bracing around statement > > > > + if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING]) > > I count an unbalanced brace. There may be something wrong, but at least > from the example, I don't see it. > > Can you include a bit more context? Perhaps the hunk that failed? It's from a multi-line if statement. if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING]) || 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_COLLECT_METADATA]) || 1 != nl_attr_get_u8(vxlan[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) || (tnl_cfg->dst_port != nl_attr_get_be16(vxlan[IFLA_VXLAN_PORT]))) { err = EINVAL; } In some cases the check if letter is '(': evaluates to false and balanced_parens() unexpectedly returns true, as such if_and_for_end_with_bracket_check() returns False because there is no { at the end of the line. I could not reproduce this manually in the interpreter. But adding print statements inside the check for '(' results in zero prints. So it must have something to do with string/char caching. $ python --version Python 2.7.5
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index dfe6f9c7621e..638ac97746b6 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -144,11 +144,11 @@ def if_and_for_end_with_bracket_check(line): """This is a rather naive counter - it won't deal with quotes""" balance = 0 for letter in line: - if letter is '(': + if letter == '(': balance += 1 - elif letter is ')': + elif letter == ')': balance -= 1 - return balance is 0 + return balance == 0 if __regex_is_for_if_single_line_bracket.search(line) is not None: if not balanced_parens(line):
We need to use == instead of the is operator. If you're unlucky it may fail because they're not exactly the same object, but hold the same value. Example false positive: E(120): Inappropriate bracing around statement + if (0 != nl_attr_get_u8(vxlan[IFLA_VXLAN_LEARNING]) Fixes: 30c7ffd5ac46 ("utilities/checkpatch.py: Check for appropriate bracing") Signed-off-by: Eric Garver <e@erig.me> --- utilities/checkpatch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)