Rearrange structures to better fit valgrind's memory leak heuristics.

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@nicira.com>

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f2b933963952be5c151ec088d51c4491c797ee88..51b847f24d1a1829d3870f435e04f61ffe6326bb 100644 (file)
--- 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 e540850f27fa8ff043d5939c1eb2dd153161c892..26904ef021a1adeb7d9bf551536de1317c87e152 100644 (file)
--- 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 9e6ed257a662108da7d2f89346565388399010fc..1848bb989363994f5dc1786acf473212ab985703 100644 (file)
--- 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 2fdc72ef5eb79b22dc019d0727a0f83117240ba3..306a56da650ef3b6659df4796f2a03b206a1eacc 100644 (file)
--- 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 a9285f79ee08e6b25be9d6bb0f8ebf2149639b74..17e4222195f9ab6cbcc6825d54e44a7b805d9759 100644 (file)
--- 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 e15d57b683170c812dba5c41aa2e704b7c25c058..adc3b47cc9b524e087d5ddf755adc7476e9ca864 100644 (file)
--- 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;