Re: [waffle] [PATCH 05/10] waffle: remove found_platform from waffle_init_parse_attrib_list
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. ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 05/10] waffle: remove found_platform from waffle_init_parse_attrib_list
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. 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. ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 05/10] waffle: remove found_platform from waffle_init_parse_attrib_list
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? -Emil ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
[waffle] [PATCH 05/10] waffle: remove found_platform from waffle_init_parse_attrib_list
Whenever a platform is missing a case statement, the default will kick in throwing an error and exiting the function. Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- src/waffle/api/waffle_init.c | 9 - 1 file changed, 9 deletions(-) diff --git a/src/waffle/api/waffle_init.c b/src/waffle/api/waffle_init.c index b45c3ba..9baf271 100644 --- a/src/waffle/api/waffle_init.c +++ b/src/waffle/api/waffle_init.c @@ -40,8 +40,6 @@ waffle_init_parse_attrib_list( const int32_t attrib_list[], int *platform) { -bool found_platform = false; - for (const int32_t *i = attrib_list; *i != 0; i += 2) { const int32_t attr = i[0]; const int32_t value = i[1]; @@ -51,7 +49,6 @@ waffle_init_parse_attrib_list( switch (value) { #define CASE_DEFINED_PLATFORM(name) \ case WAFFLE_PLATFORM_##name : \ -found_platform = true; \ *platform = value; \ break; @@ -116,12 +113,6 @@ waffle_init_parse_attrib_list( } } -if (!found_platform) { -wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE, - attribute list is missing WAFFLE_PLATFORM); -return false; -} - return true; } -- 1.9.3 ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle