On 3/20/17, 8:27 AM, "ovs-dev-boun...@openvswitch.org on behalf of Eric Garver" 
<ovs-dev-boun...@openvswitch.org on behalf of e...@erig.me> wrote:

    On Sun, Mar 19, 2017 at 10:11:09AM -0700, Darrell Ball wrote:
    > In file included from /usr/include/string.h:640:0,
    >                  from ./lib/string.h:20,
    >                  from /usr/include/netinet/icmp6.h:22,
    >                  from ../lib/flow.h:21,
    >                  from ../lib/flow.c:18:
    > In function 'memset',
    >     inlined from 'flow_push_vlan_uninit' at ../lib/flow.c:2188:19:
    > /usr/include/x86_64-linux-gnu/bits/string3.h:81:30: error:
    > call to '__warn_memset_zero_len' declared with attribute warning:
    > memset used with constant zero length parameter; this could be
    > due to transposed parameters [-Werror]
    >        __warn_memset_zero_len ();
    >                               ^
    > cc1: all warnings being treated as errors
    > make[2]: *** [lib/flow.lo] Error 1
    > 
    > Fixes: f0fb825a3785 ("Add support for 802.1ad (QinQ tunneling)")
    > Signed-off-by: Darrell Ball <dlu...@gmail.com>
    > ---
    >  lib/flow.c | 4 +++-
    >  1 file changed, 3 insertions(+), 1 deletion(-)
    > 
    > diff --git a/lib/flow.c b/lib/flow.c
    > index f628526..0a67455 100644
    > --- a/lib/flow.c
    > +++ b/lib/flow.c
    > @@ -2184,7 +2184,9 @@ flow_push_vlan_uninit(struct flow *flow, struct 
flow_wildcards *wc)
    >  {
    >      if (wc) {
    >          int n = flow_count_vlan_headers(flow);
    > -        memset(wc->masks.vlans, 0xff, sizeof(union flow_vlan_hdr) * n);
    > +        if (n) {
    
    Better to use (n > 0) since n is of type int.
    
    Thanks!

ahh , it is “int”; I auto-translated to something else.
The called API flow_count_vlan_headers cannot return negative values.
so it is cosmetic, but I agree; I also added some similar checks in the
code associated with the same commit, as it looks better to the eye.

I would prefer not using signed values for number of vlans, but this code
has been reviewed and seems correct in its usage of same, so I won’t
press the matter.

    
    > +            memset(wc->masks.vlans, 0xff, sizeof(union flow_vlan_hdr) * 
n);
    > +        }
    >      }
    >      memmove(&flow->vlans[1], &flow->vlans[0],
    >              sizeof(union flow_vlan_hdr) * (FLOW_MAX_VLAN_HEADERS - 1));
    > -- 
    > 1.9.1
    _______________________________________________
    dev mailing list
    d...@openvswitch.org
    
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=N-gxJn1peieH0LmA-VwtdBswybNu6Qtqax8G0Yv25zQ&s=SVWxmtMJQZigGxQK8y8vqH2PlJZgxK8CIRotUzF9Kfc&e=
 
    

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to