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

[Emil Velikov]

On 19 November 2015 at 00:16, Nanley Chery <nanleychery at gmail.com> wrote:
> 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 []’
>

>> >> > +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.
>
Ouch you're spot-on for both things.

Thanks for calling me out, and pardon for the noise.

-Emil