On Mon, Mar 20, 2017 at 06:39:15PM +0000, Darrell Ball wrote: > > > 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.
100% agree. This patch made me look at changing to use size_t for flow_count_vlan_headers(), but it requires other changes. So I'll attempt it separately. Thanks for pointing it out. > > > > + 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://mail.openvswitch.org/mailman/listinfo/ovs-dev