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

[Chad Versace]

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.