This looks good to me, although I'm not too familiar with the code.

Tested-by: Joe Stringer <joestrin...@nicira.com>


On 13 June 2014 14:06, Simon Horman <ho...@verge.net.au> wrote:

> dd51dae29bccca3 ("ofproto: Move logic for collecting read-only rules into
> rule_criteria.") modifies modify_flows__ such that the variable error,
> the return value, may be uninitialised if either of the following is true:
>
> 1. ofproto->ofproto_class->rule_premodify_actions is NULL
> 2. rules->n is zero
>
> It appears for the "bfd - Verify tunnel down detection" test
> in the testsuite the first condition is true and the test fails.
>
> This patch assumes that modify_flows__() may succeed without directly
> checking that rules are modifiable in the second for loop, which was part
> of the change introduced by dd51dae29bccca3 ("ofproto: Move logic for
> collecting read-only rules into rule_criteria."). I am assuming that the
> implication is that it is no longer necessary.
>
> As a result of this assumption this patch simply initialises the
> return value to 0.
>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
>
> ---
>
> I noticed this while testing Ben's proposed modifications
> to "ofproto-dpif: MPLS recirculation".
> ---
>  ofproto/ofproto.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index a3ab276..83a9c35 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -4099,7 +4099,7 @@ modify_flows__(struct ofproto *ofproto, struct
> ofputil_flow_mod *fm,
>  {
>      struct list dead_cookies = LIST_INITIALIZER(&dead_cookies);
>      enum nx_flow_update_event event;
> -    enum ofperr error;
> +    enum ofperr error = 0;
>      size_t i;
>
>      if (ofproto->ofproto_class->rule_premodify_actions) {
> --
> 2.0.0.rc2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to