[ovs-dev] [PATCH] check-structs: Add check that OFP_ASSERT is checking the right structures.

Ben Pfaff blp at nicira.com
Tue Oct 4 16:26:35 UTC 2011


Thanks, I'll push this in a minute.  I added a unit test.

On Mon, Oct 03, 2011 at 10:16:02PM -0700, Justin Pettit wrote:
> This is a great idea.  Looks good to me.
> 
> --Justin
> 
> 
> On Oct 3, 2011, at 1:49 PM, Ben Pfaff wrote:
> 
> > 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 ++++++-
> > 1 files changed, 6 insertions(+), 1 deletions(-)
> > 
> > diff --git a/build-aux/check-structs b/build-aux/check-structs
> > index 152c6a2..0849fcf 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('=')
> > -- 
> > 1.7.4.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 



More information about the dev mailing list