[ovs-dev] [leaks 5/5] Rearrange structures to better fit valgrind's memory leak heuristics.

Ben Pfaff blp at nicira.com
Wed Mar 28 21:58:34 UTC 2012


valgrind's memory leak detector considers a pointer to the head of a memory
block to be "definitely" a pointer to that memory block but a pointer to
the interior of a memory block only "possibly" a pointer to that memory
block.  Open vSwitch hmap_node and list data structures can go anywhere
inside a structure; if they are in the middle of a structure then valgrind
considers pointers to them to be possible leaks.  Therefore, this commit
moves some of these from the middle of data structures to the head, to
reduce valgrind's uncertainty.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ofproto/ofproto-dpif.c     |    2 +-
 ofproto/ofproto-provider.h |    6 +++---
 ovsdb/jsonrpc-server.c     |    4 ++--
 ovsdb/row.h                |    4 ++--
 ovsdb/server.h             |   12 ++++++------
 vswitchd/bridge.c          |    2 +-
 6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f2b9339..51b847f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -145,8 +145,8 @@ static void update_mirror_stats(struct ofproto_dpif *ofproto,
                                 uint64_t packets, uint64_t bytes);
 
 struct ofbundle {
-    struct ofproto_dpif *ofproto; /* Owning ofproto. */
     struct hmap_node hmap_node; /* In struct ofproto's "bundles" hmap. */
+    struct ofproto_dpif *ofproto; /* Owning ofproto. */
     void *aux;                  /* Key supplied by ofproto's client. */
     char *name;                 /* Identifier for log messages. */
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index e540850..26904ef 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -36,10 +36,10 @@ struct ofputil_flow_mod;
  * With few exceptions, ofproto implementations may look at these fields but
  * should not modify them. */
 struct ofproto {
+    struct hmap_node hmap_node; /* In global 'all_ofprotos' hmap. */
     const struct ofproto_class *ofproto_class;
     char *type;                 /* Datapath type. */
     char *name;                 /* Datapath name. */
-    struct hmap_node hmap_node; /* In global 'all_ofprotos' hmap. */
 
     /* Settings. */
     uint64_t fallback_dpid;     /* Datapath ID if no better choice found. */
@@ -94,8 +94,8 @@ struct ofport *ofproto_get_port(const struct ofproto *, uint16_t ofp_port);
  * With few exceptions, ofproto implementations may look at these fields but
  * should not modify them. */
 struct ofport {
-    struct ofproto *ofproto;    /* The ofproto that contains this port. */
     struct hmap_node hmap_node; /* In struct ofproto's "ports" hmap. */
+    struct ofproto *ofproto;    /* The ofproto that contains this port. */
     struct netdev *netdev;
     struct ofputil_phy_port pp;
     uint16_t ofp_port;          /* OpenFlow port number. */
@@ -156,8 +156,8 @@ struct oftable {
  * With few exceptions, ofproto implementations may look at these fields but
  * should not modify them. */
 struct rule {
-    struct ofproto *ofproto;     /* The ofproto that contains this rule. */
     struct list ofproto_node;    /* Owned by ofproto base code. */
+    struct ofproto *ofproto;     /* The ofproto that contains this rule. */
     struct cls_rule cr;          /* In owning ofproto's classifier. */
 
     struct ofoperation *pending; /* Operation now in progress, if nonnull. */
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 9e6ed25..1848bb9 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira Networks
+/* Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -295,9 +295,9 @@ ovsdb_jsonrpc_server_wait(struct ovsdb_jsonrpc_server *svr)
 /* JSON-RPC database server session. */
 
 struct ovsdb_jsonrpc_session {
+    struct list node;           /* Element in remote's sessions list. */
     struct ovsdb_session up;
     struct ovsdb_jsonrpc_remote *remote;
-    struct list node;           /* Element in remote's sessions list. */
 
     /* Triggers. */
     struct hmap triggers;       /* Hmap of "struct ovsdb_jsonrpc_trigger"s. */
diff --git a/ovsdb/row.h b/ovsdb/row.h
index 2fdc72e..306a56d 100644
--- a/ovsdb/row.h
+++ b/ovsdb/row.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira Networks
+/* Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,8 +43,8 @@ struct ovsdb_weak_ref {
 
 /* A row in a database table. */
 struct ovsdb_row {
-    struct ovsdb_table *table;     /* Table to which this belongs. */
     struct hmap_node hmap_node;    /* Element in ovsdb_table's 'rows' hmap. */
+    struct ovsdb_table *table;     /* Table to which this belongs. */
     struct ovsdb_txn_row *txn_row; /* Transaction that row is in, if any. */
 
     /* Weak references. */
diff --git a/ovsdb/server.h b/ovsdb/server.h
index a9285f7..17e4222 100644
--- a/ovsdb/server.h
+++ b/ovsdb/server.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2011 Nicira Networks
+/* Copyright (c) 2011, 2012 Nicira Networks
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,9 +39,9 @@ struct ovsdb_lock_waiter *ovsdb_session_get_lock_waiter(
  * A lock always has one or more "lock waiters" kept on a list.  The waiter at
  * the head of the list owns the lock. */
 struct ovsdb_lock {
+    struct hmap_node hmap_node;  /* In ovsdb_server's "locks" hmap. */
     struct ovsdb_server *server; /* The containing server. */
     char *name;                  /* Unique name. */
-    struct hmap_node hmap_node;  /* In ovsdb_server's "locks" hmap. */
     struct list waiters;         /* Contains "struct ovsdb_lock_waiter"s. */
 };
 
@@ -55,14 +55,14 @@ enum ovsdb_lock_mode {
 
 /* A session's request for a database lock. */
 struct ovsdb_lock_waiter {
+    struct hmap_node session_node; /* In ->session->locks's hmap. */
+    struct ovsdb_lock *lock;    /* The lock being waited for. */
+
     enum ovsdb_lock_mode mode;
     char *lock_name;
 
-    struct ovsdb_lock *lock;    /* The lock being waited for. */
-    struct list lock_node;      /* In ->lock->waiters's list. */
-
     struct ovsdb_session *session;
-    struct hmap_node session_node; /* In ->session->locks's hmap. */
+    struct list lock_node;      /* In ->lock->waiters's list. */
 };
 
 struct ovsdb_session *ovsdb_lock_waiter_remove(struct ovsdb_lock_waiter *);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index e15d57b..adc3b47 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -87,8 +87,8 @@ struct mirror {
 };
 
 struct port {
-    struct bridge *bridge;
     struct hmap_node hmap_node; /* Element in struct bridge's "ports" hmap. */
+    struct bridge *bridge;
     char *name;
 
     const struct ovsrec_port *cfg;
-- 
1.7.2.5




More information about the dev mailing list