From f937ccc2321d9bcb946fca249ec718531553a9bc Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Tue, 4 Oct 2011 09:26:14 -0700
Subject: [PATCH] check-structs: Add check that OFP_ASSERT is checking the
 right structures.

This avoids a fairly common issue in which a developer cuts and pastes a
structure definition and forgets to update the structure name inside the
OFP_ASSERT, so that the new structure's size doesn't really get checked at
all.
---
 build-aux/check-structs |  7 ++++++-
 tests/check-structs.at  | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/build-aux/check-structs b/build-aux/check-structs
index 152c6a21..0849fcf1 100755
--- a/build-aux/check-structs
+++ b/build-aux/check-structs
@@ -187,6 +187,7 @@ def parseStruct():
             warn("%s needs %d bytes of tail padding" % (structName, shortage))
         size += shortage
     types[structName] = {"size": size, "alignment": alignment}
+    return structName
 
 def checkStructs():
     if len(sys.argv) < 2:
@@ -223,6 +224,7 @@ header files without extensions.''' % {"argv0": argv0}
         global lineNumber
         inputFile = open(fileName)
         lineNumber = 0
+        lastStruct = None
         while getToken():
             if token in ("#ifdef", "#ifndef", "#include",
                          "#endif", "#elif", "#else"):
@@ -243,12 +245,15 @@ header files without extensions.''' % {"argv0": argv0}
                 while token != ';':
                     getToken()
             elif token in ('struct', 'union'):
-                parseStruct()
+                lastStruct = parseStruct()
             elif match('OFP_ASSERT') or match('BOOST_STATIC_ASSERT'):
                 forceMatch('(')
                 forceMatch('sizeof')
                 forceMatch('(')
                 typeName = parseTypeName()
+                if typeName != lastStruct:
+                    warn("checking size of %s but %s was most recently defined"
+                         % (typeName, lastStruct))
                 forceMatch(')')
                 forceMatch('=')
                 forceMatch('=')
diff --git a/tests/check-structs.at b/tests/check-structs.at
index 52e92ec0..a926a0fb 100644
--- a/tests/check-structs.at
+++ b/tests/check-structs.at
@@ -2,7 +2,8 @@ AT_BANNER([struct alignment checker unit tests])
 
 m4_define([check_structs], [$top_srcdir/build-aux/check-structs])
 m4_define([RUN_STRUCT_CHECKER], 
-  [AT_SKIP_IF([test $HAVE_PYTHON = no])
+  [AT_KEYWORDS([check-structs])
+   AT_SKIP_IF([test $HAVE_PYTHON = no])
    AT_DATA([test.h], [$1
 ])
    AT_CHECK_UNQUOTED([$PYTHON check_structs test.h], [$2], [$3], [$4])])
@@ -39,3 +40,17 @@ OFP_ASSERT(sizeof(struct wibble) == 12);
   [test.h:4: warning: struct wibble is 8 bytes long but declared as 12
 ])
 AT_CLEANUP
+
+AT_SETUP([check wrong struct's declared size])
+RUN_STRUCT_CHECKER(
+[struct moo {
+    uint64_t bar;
+};
+OFP_ASSERT(sizeof(struct moo) == 8);
+struct wibble {
+    uint64_t z;
+};
+OFP_ASSERT(sizeof(struct moo) == 8);
+], [1], [], [test.h:8: warning: checking size of struct moo but struct wibble was most recently defined
+])
+AT_CLEANUP
-- 
2.30.2