[Mesa-dev] [PATCH 2/2] mesa: Add test for sorted extension table

[Nanley Chery]

On Wed, Nov 18, 2015 at 4:08 PM, Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> On 18 November 2015 at 23:40, Nanley Chery <nanleychery at gmail.com> wrote:
> > On Wed, Nov 18, 2015 at 3:07 PM, Ilia Mirkin <imirkin at alum.mit.edu>
> wrote:
> >>
> >> Why have the count as an entry in the enum? Can't you just do
> >> ARRAY_SIZE(_mesa_extension_table) ?
> >>
> >
> > The compiler doesn't know the size of the array by just looking at the
> > header. To use ARRAY_SIZE, I'd have to modify the declaration of
> > _mesa_extension_table to specify a size of MESA_EXTENSION_COUNT.
> >
> Afaict there is no declaration, only a definition of the array. That
> aside I'm not sure what makes you think that - we have dozens of cases
> in mesa which use this approach.
>
>
The array is declared in extensions.h and defined in extensions.c.

I compiled the test using the ARRAY_SIZE macro and got the following error:
mesa_extensions.cpp:40:53: error: invalid application of ‘sizeof’ to
incomplete type ‘const mesa_extension []’



> >> On Wed, Nov 18, 2015 at 6:01 PM, Nanley Chery <nanleychery at gmail.com>
> >> wrote:
> >> > From: Nanley Chery <nanley.g.chery at intel.com>
> >> >
> >> > Enable developers to know if the table's alphabetical sorting
> >> > is maintained or lost.
> >> >
> >> > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> >> > ---
> >> >  src/mesa/main/extensions.h              |  1 +
> >> >  src/mesa/main/tests/Makefile.am         |  1 +
> >> >  src/mesa/main/tests/mesa_extensions.cpp | 47
> >> > +++++++++++++++++++++++++++++++++
> >> >  3 files changed, 49 insertions(+)
> >> >  create mode 100644 src/mesa/main/tests/mesa_extensions.cpp
> >> >
> >> > diff --git a/src/mesa/main/extensions.h b/src/mesa/main/extensions.h
> >> > index 1615e1c..7114c96 100644
> >> > --- a/src/mesa/main/extensions.h
> >> > +++ b/src/mesa/main/extensions.h
> >> > @@ -85,6 +85,7 @@ enum {
> >> >  #define EXT(name_str, ...) MESA_EXTENSION_##name_str,
> >> >  #include "extensions_table.h"
> >> >  #undef EXT
> >> > +   MESA_EXTENSION_COUNT
> >> >  };
> >> >
> >> >
> >> > diff --git a/src/mesa/main/tests/Makefile.am
> >> > b/src/mesa/main/tests/Makefile.am
> >> > index bd7ab73..d6977e2 100644
> >> > --- a/src/mesa/main/tests/Makefile.am
> >> > +++ b/src/mesa/main/tests/Makefile.am
> >> > @@ -27,6 +27,7 @@ AM_CPPFLAGS += -DHAVE_SHARED_GLAPI
> >> >  main_test_SOURCES +=                   \
> >> >         dispatch_sanity.cpp             \
> >> >         mesa_formats.cpp                        \
> >> > +       mesa_extensions.cpp                     \
> >> >         program_state_string.cpp
> >> >
> >> >  main_test_LDADD += \
> >> > diff --git a/src/mesa/main/tests/mesa_extensions.cpp
> >> > b/src/mesa/main/tests/mesa_extensions.cpp
> >> > new file mode 100644
> >> > index 0000000..5154ae1
> >> > --- /dev/null
> >> > +++ b/src/mesa/main/tests/mesa_extensions.cpp
> >> > @@ -0,0 +1,47 @@
> >> > +/*
> >> > + * Copyright © 2015 Intel Corporation
> >> > + *
> >> > + * Permission is hereby granted, free of charge, to any person
> >> > obtaining a
> >> > + * copy of this software and associated documentation files (the
> >> > "Software"),
> >> > + * to deal in the Software without restriction, including without
> >> > limitation
> >> > + * the rights to use, copy, modify, merge, publish, distribute,
> >> > sublicense,
> >> > + * and/or sell copies of the Software, and to permit persons to whom
> >> > the
> >> > + * Software is furnished to do so, subject to the following
> conditions:
> >> > + *
> >> > + * The above copyright notice and this permission notice (including
> the
> >> > next
> >> > + * paragraph) shall be included in all copies or substantial portions
> >> > of the
> >> > + * Software.
> >> > + *
> >> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >> > EXPRESS OR
> >> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> > MERCHANTABILITY,
> >> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
> >> > SHALL
> >> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR
> >> > OTHER
> >> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >> > ARISING
> >> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER
> >> > + * DEALINGS IN THE SOFTWARE.
> >> > + */
> >> > +
> >> > +/**
> >> > + * \name mesa_extensions.cpp
> >> > + *
> >> > + * Verify that the extensions table is sorted.
> >> > + *
> >> > + */
> >> > +
> >> > +#include <gtest/gtest.h>
> >> > +#include "main/mtypes.h"
> >> > +#include "main/extensions.h"
> >> > +
> >> > +/**
> >> > + * Debug/test: verify the extension table is alphabetically sorted.
> >> > + */
> >> > +TEST(MesaExtensionsTest, AlphabeticallySorted)
> >> > +{
> >> > +   for (int i = 0; i < MESA_EXTENSION_COUNT - 1; ++i) {
> >> > +      const char * current_str = _mesa_extension_table[i].name;
> >> > +      const char * next_str = _mesa_extension_table[i+1].name;
> >> > +
> >> > +      /* We expect the extension table to be alphabetically sorted */
> >> > +      ASSERT_LT(strcmp(current_str, next_str), 0);
> Am I day dreaming or something's wrong here. On the last iteration
> you'll end up comparing the final extension name against ... null ?
>
>
The last iteration compares the 2nd-to-last extension name against the last
extension name.

Regards,
Nanley


> -Emil
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151118/be4db409/attachment.html>