On Thu, Jul 19, 2012 at 05:14:57PM +0900, Isaku Yamahata wrote:
> On Thu, Jul 19, 2012 at 04:31:38PM +0900, Simon Horman wrote:
> > On Thu, Jul 05, 2012 at 11:12:29PM -0700, Ben Pfaff wrote:
> > > OpenFlow headers are not as uniform as they could be, with size,
> > > alignment,
> > > and numbering changes from one version to another and across varieties
> > > (e.g. ordinary messages vs. "stats" messages). Until now the Open vSwitch
> > > internal APIs haven't done a good job of abstracting those differences in
> > > header formats. This commit changes that; from this commit forward very
> > > little code actually needs to understand the header format or numbering.
> > > Instead, it can just encode or decode, or pull or put, the header using
> > > a more abstract API using the ofpraw_, ofptype_, and other APIs in the
> > > new ofp-msgs module.
> > >
> > > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > > ---
> > > v3: New commit. This isn't complete yet: in particular the program for
> > > extracting the numbers from the ofp-msgs.h header file isn't written,
> > > although there's a skeleton. That means that nothing has been tested or
> > > carefully looked over, either.
> > >
> > > v3->v4: Almost ready. The ofp-msgs.c functions need some comments
> > > and careful review.
> > >
> > > v4->v5: Ready for review.
> >
> > Not a proper review, but I noticed a compile-time warning.
> >
> > [snip]
> >
> > > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > > index 7b0b220..425448e 100644
> >
> > [snip]
> >
> > > @@ -620,18 +608,21 @@ fetch_port_by_stats(const char *vconn_name,
> > > run(vconn_recv_block(vconn, &reply), "OpenFlow packet receive
> > > failed");
> > > recv_xid = ((struct ofp_header *) reply->data)->xid;
> > > if (send_xid == recv_xid) {
> > > - const struct ofputil_msg_type *type;
> > > - struct ofp_stats_msg *osm;
> > > -
> > > - ofputil_decode_msg_type(reply->data, &type);
> > > - if (ofputil_msg_type_code(type) !=
> > > OFPUTIL_OFPST_PORT_DESC_REPLY) {
> > > + struct ofp_header *oh;
> > > + enum ofptype type;
> > > + struct ofpbuf b;
> > > + uint16_t flags;
> > > +
> > > + ofpbuf_use_const(&b, oh, ntohs(oh->length));
> >
> > gcc tells me that oh is used uninitialised here and that does seem
> > to be the case to me too.
> >
> > Should it be initialised to ((struct ofp_header *) reply->data) ?
>
> How about this patch?
>
> commit 3da23e05619d5b21b057b20dc078ca03e522da4c
> Author: Isaku Yamahata <yamah...@valinux.co.jp>
> Date: Thu Jul 19 17:13:33 2012 +0900
>
> ovs-ofctl: uninitialized variable oh
>
> This patch fixes the following warning.
>
> > openvswitch/utilities/ovs-ofctl.c: In function 'fetch_ofputil_phy_port':
> > openvswitch/utilities/ovs-ofctl.c:616:89: warning: 'oh' may be used
> uninitialized in this function [-Wuninitialized]
>
> Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp>
>
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 7bf8e0f..b6679f7 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -608,7 +608,7 @@ fetch_port_by_stats(const char *vconn_name,
> run(vconn_recv_block(vconn, &reply), "OpenFlow packet receive
> failed");
> recv_xid = ((struct ofp_header *) reply->data)->xid;
> if (send_xid == recv_xid) {
> - struct ofp_header *oh;
> + struct ofp_header *oh = reply->data;
> enum ofptype type;
> struct ofpbuf b;
> uint16_t flags;
>
>
> --
> yamahata
Thanks, but I'd prefer to just fix it in the initial commit rather
than with a followup patch.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
