[waffle] [PATCH 05/10] waffle: remove found_platform from waffle_init_parse_attrib_list
Emil Velikov
emil.l.velikov at gmail.com
Wed Jun 11 05:07:46 PDT 2014
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
>
More information about the waffle
mailing list