Looks good.
Ethan
On Mon, Aug 8, 2011 at 16:02, Ben Pfaff <b...@nicira.com> wrote:
> On Mon, Aug 08, 2011 at 03:35:39PM -0700, Ethan Jackson wrote:
>> The indentation is slightly messed up in the man page (when viewed
>> with "man"). The first occurrence of "ovs-vswitchd will respond
>> with extensive" should be indented further like the second occurrence
>> is.
>
> Thanks, fixed.
>
> (Have I mentioned just how much "nroff" syntax blows?)
>
>> Also, I noticed another problem in ofproto_unixctl_trace() which I
>> don't think was introduced by this patch. trace_format_rule()
>> dereferences the result of rule_dpif_lookup() before checking if it's
>> NULL. Seems like trace_format_rule should really take a rule_dpif
>> instead.
>
> You're right.
>
> Here's a fix:
>
> --8<--------------------------cut here-------------------------->8--
>
> From: Ben Pfaff <b...@nicira.com>
> Date: Mon, 8 Aug 2011 16:01:05 -0700
> Subject: [PATCH] ofproto-dpif: Fix pointer arithmetic on null pointer.
>
> rule_dpif_lookup() can return a null pointer as 'rule' so we shouldn't
> try to calculate '&rule->up' before it's been found to be nonnull.
>
> This doesn't appear to fix a real bug because 'up' is the first member
> of 'rule' so when rule is NULL then &rule->up is also NULL.
>
> Reported-by: Ethan Jackson <et...@nicira.com>
> ---
> ofproto/ofproto-dpif.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 2aea4e1..4cad7af 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3920,7 +3920,7 @@ struct ofproto_trace {
> };
>
> static void
> -trace_format_rule(struct ds *result, int level, const struct rule *rule)
> +trace_format_rule(struct ds *result, int level, const struct rule_dpif *rule)
> {
> ds_put_char_multiple(result, '\t', level);
> if (!rule) {
> @@ -3929,13 +3929,13 @@ trace_format_rule(struct ds *result, int level, const
> struct rule *rule)
> }
>
> ds_put_format(result, "Rule: cookie=%#"PRIx64" ",
> - ntohll(rule->flow_cookie));
> - cls_rule_format(&rule->cr, result);
> + ntohll(rule->up.flow_cookie));
> + cls_rule_format(&rule->up.cr, result);
> ds_put_char(result, '\n');
>
> ds_put_char_multiple(result, '\t', level);
> ds_put_cstr(result, "OpenFlow ");
> - ofp_print_actions(result, rule->actions, rule->n_actions);
> + ofp_print_actions(result, rule->up.actions, rule->up.n_actions);
> ds_put_char(result, '\n');
> }
>
> @@ -3962,7 +3962,7 @@ trace_resubmit(struct action_xlate_ctx *ctx, struct
> rule_dpif *rule)
>
> ds_put_char(result, '\n');
> trace_format_flow(result, ctx->recurse + 1, "Resubmitted flow", trace);
> - trace_format_rule(result, ctx->recurse + 1, &rule->up);
> + trace_format_rule(result, ctx->recurse + 1, rule);
> }
>
> static void
> @@ -4050,7 +4050,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const
> char *args_,
> ds_put_char(&result, '\n');
>
> rule = rule_dpif_lookup(ofproto, &flow);
> - trace_format_rule(&result, 0, &rule->up);
> + trace_format_rule(&result, 0, rule);
> if (rule) {
> struct ofproto_trace trace;
> struct ofpbuf *odp_actions;
> --
> 1.7.4.4
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
