On Tue, Oct 08, 2013 at 12:26:22PM +0300, Alexandru Copot wrote:
> Signed-off-by: Alexandru Copot <[email protected]>
> Cc: Daniel Baluta <[email protected]>
Thanks for the patch.
"git am" says:
Applying: lib/ofp-util: add decoder for role status asynchronous message
/home/blp/ovs/.git/rebase-apply/patch:27: trailing whitespace.
const struct ofp14_role_status *r = b.l3;
warning: 1 line adds whitespace errors.
GCC says:
../lib/ofp-util.c: In function 'ofputil_decode_role_status':
../lib/ofp-util.c:3939:5: error: ISO C90 forbids mixed declarations and
code [-Werror=declaration-after-statement]
> +enum ofperr
> +ofputil_decode_role_status(const struct ofp_header *oh,
> + struct ofputil_role_status *rs)
> +{
> + struct ofpbuf b;
> + enum ofpraw raw;
> +
> + ofpbuf_use_const(&b, oh, ntohs(oh->length));
> + raw = ofpraw_pull_assert(&b);
We always put {} around 'if' blocks. But I would just use
ovs_assert() here instead of an 'if':
> + if (raw != OFPRAW_OFPT14_ROLE_STATUS)
> + NOT_REACHED();
> +
> + const struct ofp14_role_status *r = b.l3;
> + if (r->role != htonl(OFPCR12_ROLE_NOCHANGE) &&
> + r->role != htonl(OFPCR12_ROLE_EQUAL) &&
> + r->role != htonl(OFPCR12_ROLE_MASTER) &&
> + r->role != htonl(OFPCR12_ROLE_SLAVE)) {
This should only be indented 4 spaces beyond the 'if', as usual:
> + return OFPERR_OFPRRFC_BAD_ROLE;
> + }
> +
> + rs->role = ntohl(r->role);
> + rs->generation_id = ntohll(r->generation_id);
> +
> + return 0;
> +}
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
