On 3/20/17, 12:32 PM, "[email protected] on behalf of Eric
Garver" <[email protected] on behalf of [email protected]> wrote:
On Mon, Mar 20, 2017 at 11:24:49AM -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
>
> At the same time, fix some similar bounds checks related to
> the same commit.
>
> Signed-off-by: Darrell Ball <[email protected]>
> Fixes: f0fb825a3785 ("Add support for 802.1ad (QinQ tunneling)")
> CC: Eric Garver <[email protected]>
> ---
>
> v1->v2: Change some cosmetic bounds checks.
>
> lib/flow.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/lib/flow.c b/lib/flow.c
> index f628526..8e7d3ee 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -2167,10 +2167,10 @@ void
> flow_pop_vlan(struct flow *flow, struct flow_wildcards *wc)
> {
> int n = flow_count_vlan_headers(flow);
> - if (n == 0) {
> + if (n <= 0) {
> return;
> }
> - if (wc) {
> + if ((wc) && (n > 1)) {
This is redundant because of the if (n <= 0) return above, but it's
fine.
It is just for consistency w.r.t. memset size as per other gcc/glibc
warning, even though memset is safe with zero size.
i.e. your code is functionally correct.
> memset(&wc->masks.vlans[1], 0xff,
> sizeof(union flow_vlan_hdr) * (n - 1));
> }
> @@ -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 > 0) {
> + 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
>
Thanks for the clean up!
Acked-by: Eric Garver <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=94Xk72jLRHCUW_F-65jVvIes5pnWIgOdCNVKzAALs5I&s=HtjN2Mk985ZmYSnekbzOffPHRwhE2FZH-cCaZzdnoiQ&e=
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
