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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
