<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Wed, Nov 18, 2015 at 4:08 PM, Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 18 November 2015 at 23:40, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: > On Wed, Nov 18, 2015 at 3:07 PM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a>> 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. <div><div> </div></div></blockquote><div> </div><div>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 []’ </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div> >> On Wed, Nov 18, 2015 at 6:01 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> >> wrote: >> > From: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com" target="_blank">nanley.g.chery@intel.com</a>> >> > >> > Enable developers to know if the table's alphabetical sorting >> > is maintained or lost. >> > >> > Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com" target="_blank">nanley.g.chery@intel.com</a>> >> > --- >> > 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); </div></div>Am I day dreaming or something's wrong here. On the last iteration you'll end up comparing the final extension name against ... null ? </blockquote><div> </div><div>The last iteration compares the 2nd-to-last extension name against the last extension name. </div><div>Regards, </div><div>Nanley </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> -Emil </blockquote></div> </div></div>