[waffle] [PATCH 05/10] waffle: remove found_platform from waffle_init_parse_attrib_list

[Emil Velikov]

On 11/06/14 05:46, Chad Versace wrote:
> On Fri, Jun 06, 2014 at 12:24:06PM +0100, Emil Velikov wrote:
>> On 06/06/14 07:25, Chad Versace wrote:
>>> On Sat, May 31, 2014 at 03:22:03AM +0100, Emil Velikov wrote:
>>>> Whenever a platform is missing a case statement, the default will
>>>> kick in throwing an error and exiting the function.
>>>
>>> Ah, but that's not what 'found_platform' is checking for...
>>>
>>>> -    if (!found_platform) {
>>>> -        wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE,
>>>> -                     "attribute list is missing WAFFLE_PLATFORM");
>>>> -        return false;
>>>> -    }
>>>
>>> ... waffle_init() uses 'found_platform' to check if the user provided
>>> WAFFLE_PLATFORM.
>>>
>> AFAICS that is handled by the "default" switch case.
>>
>>> I admit that waffle_init_parse_attrib_list() is a clumsily written
>>> function. It was one of the very first functions in Waffle's codebase.
>>>
>> IMHO the code bails out if we've missed (partially or fully) any platform
>> and/or if the user has provided a invalid WAFFLE_PLATFORM. Thus we will reach
>> the if (!found_platform)... only when the variable is already set (via the
>> CASE_DEFINED_PLATFORM macro).
>>
>> Perhaps this code is targeting some elaborate use-case which I'm failing to
>> see here?
> 
> I think you're failing to see the use-case because it's extremely
> *unelaborate*.
> 
> This block...
> 
>     if (!found_platform) {
>         wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE,
>                      "attribute list is missing WAFFLE_PLATFORM");
>         return false;
>     }
> 
> ... catches the case when the user supplies *no* attribute. That is, it
> catches these three erroneous cases:
> 
>     waffle_init(NULL);
>     waffle_init((int32_t[]){});
>     waffle_init((int32_t[]){0});
> 
> In other words, 'found_platform' detects when codeflow altogether skips
> the switch statement.
> 
To be honest I've not considered this case at all. Thanks for keeping up with
my silly assumptions.

-Emil

/me puts the brown bag on

_______________________________________________
> waffle mailing list
> waffle at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/waffle
>