[ovs-dev] [bug4769 3/4] ofpbuf: Make ofpbufs initialized with ofpbuf_use_stack() not expandable.

Ben Pfaff blp at nicira.com
Wed Mar 2 22:30:59 UTC 2011


My original intent for ofpbufs initialized with ofpbuf_use_stack() was that
the caller was providing enough space on the stack for the common case,
with dynamic allocation as a fallback.  But in practice, none of the
clients actually do this.  Instead, all of them actually know that the
stack-allocated buffer is big enough and, since they don't want to bother
with having to call ofpbuf_delete(), they instead assert that the buffer
wasn't reallocated.

Since this is a bit of a pain, this commit changes the semantics of
ofpbuf_use_stack() to be that the stack-allocated buffer cannot be
reallocated at all.  This is more convenient for the existing clients.
---
 lib/dpif-netdev.c |    1 -
 lib/ofpbuf.c      |   16 +++++-----------
 lib/ofpbuf.h      |    3 +--
 ofproto/ofproto.c |    2 --
 4 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 762d24b..486ba48 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -911,7 +911,6 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *state_,
 
         ofpbuf_use_stack(&buf, &state->keybuf, sizeof state->keybuf);
         odp_flow_key_from_flow(&buf, &flow->key);
-        assert(buf.base == &state->keybuf);
 
         *key = buf.data;
         *key_len = buf.size;
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
index 34aad93..0eac121 100644
--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -47,9 +47,9 @@ ofpbuf_use(struct ofpbuf *b, void *base, size_t allocated)
 
 /* Initializes 'b' as an empty ofpbuf that contains the 'allocated' bytes of
  * memory starting at 'base'.  'base' should point to a buffer on the stack.
- * If 'b' is resized, new memory will be allocated with malloc() and 'base'
- * will not be freed.  This is useful when a small stack-allocated buffer is
- * normally appropriate but sometimes it must be expanded.
+ *
+ * An ofpbuf operation that requires reallocating data will assert-fail if this
+ * function was used to initialize it.
  *
  * 'base' should be appropriately aligned.  Using an array of uint32_t or
  * uint64_t for the buffer is a reasonable way to ensure appropriate alignment
@@ -74,7 +74,7 @@ ofpbuf_use_stack(struct ofpbuf *b, void *base, size_t allocated)
 void
 ofpbuf_use_const(struct ofpbuf *b, const void *data, size_t size)
 {
-    ofpbuf_use__(b, (void *) data, size, OFPBUF_CONST);
+    ofpbuf_use__(b, (void *) data, size, OFPBUF_STACK);
     b->size = size;
 }
 
@@ -225,12 +225,6 @@ ofpbuf_resize__(struct ofpbuf *b, size_t new_headroom, size_t new_tailroom)
         break;
 
     case OFPBUF_STACK:
-        new_base = xmalloc(new_allocated);
-        ofpbuf_copy__(b, new_base, new_headroom, new_tailroom);
-        b->source = OFPBUF_MALLOC;
-        break;
-
-    case OFPBUF_CONST:
         NOT_REACHED();
 
     default:
diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
index 442c90c..3c45328 100644
--- a/lib/ofpbuf.h
+++ b/lib/ofpbuf.h
@@ -28,8 +28,7 @@ extern "C" {
 
 enum ofpbuf_source {
     OFPBUF_MALLOC,              /* Obtained via malloc(). */
-    OFPBUF_STACK,               /* Stack space or static buffer. */
-    OFPBUF_CONST                /* Must not be expanded. */
+    OFPBUF_STACK                /* Stack space or static buffer. */
 };
 
 /* Buffer for holding arbitrary data.  An ofpbuf is automatically reallocated
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index d1ac197..aac9edd 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2345,7 +2345,6 @@ facet_put__(struct ofproto *ofproto, struct facet *facet,
 
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
     odp_flow_key_from_flow(&key, &facet->flow);
-    assert(key.base == &keybuf);
 
     return dpif_flow_put(ofproto->dpif, flags, key.data, key.size,
                          actions, actions_len, stats);
@@ -2395,7 +2394,6 @@ facet_uninstall(struct ofproto *p, struct facet *facet)
 
         ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
         odp_flow_key_from_flow(&key, &facet->flow);
-        assert(key.base == &keybuf);
 
         if (!dpif_flow_del(p->dpif, key.data, key.size, &stats)) {
             facet_update_stats(p, facet, &stats);
-- 
1.7.1





More information about the dev mailing list