[Mesa-dev] [PATCH 01/18] gallium/util: clear up that debug_get_flags_option returns a 64-bit mask

[Marek Olšák]

On Tue, Jul 28, 2015 at 6:35 PM, Kai Wasserbäch
<kai at dev.carbon-project.org> wrote:
> Marek Olšák wrote on 28.07.2015 12:05:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> ---
>>  src/gallium/auxiliary/util/u_debug.c | 8 ++++----
>>  src/gallium/auxiliary/util/u_debug.h | 6 +++---
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_debug.c b/src/gallium/auxiliary/util/u_debug.c
>> index cf6eca7..b4503de 100644
>> --- a/src/gallium/auxiliary/util/u_debug.c
>> +++ b/src/gallium/auxiliary/util/u_debug.c
>> @@ -256,12 +256,12 @@ static boolean str_has_option(const char *str, const char *name)
>>     return FALSE;
>>  }
>>
>> -unsigned long
>> +uint64_t
>>  debug_get_flags_option(const char *name,
>>                         const struct debug_named_value *flags,
>> -                       unsigned long dfault)
>> +                       uint64_t dfault)
>
> Since you already touch this, maybe fix the typo as well? (s/dfault/default/)
>
>>  {
>> -   unsigned long result;
>> +   uint64_t result;
>>     const char *str;
>>     const struct debug_named_value *orig = flags;
>>     unsigned namealign = 0;
>> @@ -276,7 +276,7 @@ debug_get_flags_option(const char *name,
>>           namealign = MAX2(namealign, strlen(flags->name));
>>        for (flags = orig; flags->name; ++flags)
>>           _debug_printf("| %*s [0x%0*lx]%s%s\n", namealign, flags->name,
>> -                      (int)sizeof(unsigned long)*CHAR_BIT/4, flags->value,
>> +                      (int)sizeof(uint64_t)*CHAR_BIT/4, flags->value,
>>                        flags->desc ? " " : "", flags->desc ? flags->desc : "");
>>     }
>>     else {
>> diff --git a/src/gallium/auxiliary/util/u_debug.h b/src/gallium/auxiliary/util/u_debug.h
>> index b4286d3..926063a 100644
>> --- a/src/gallium/auxiliary/util/u_debug.h
>> +++ b/src/gallium/auxiliary/util/u_debug.h
>> @@ -269,7 +269,7 @@ void _debug_assert_fail(const char *expr,
>>  struct debug_named_value
>>  {
>>     const char *name;
>> -   unsigned long value;
>> +   uint64_t value;
>>     const char *desc;
>>  };
>>
>> @@ -377,10 +377,10 @@ debug_get_bool_option(const char *name, boolean dfault);
>>  long
>>  debug_get_num_option(const char *name, long dfault);
>>
>> -unsigned long
>> +uint64_t
>>  debug_get_flags_option(const char *name,
>>                         const struct debug_named_value *flags,
>> -                       unsigned long dfault);
>> +                       uint64_t dfault);
>>
>>  #define DEBUG_GET_ONCE_BOOL_OPTION(sufix, name, dfault) \
>>  static boolean \
>
> The receiving variables in the callers should probably be fixed up as well,
> right? Most of them seem to be unsigned long or just unsigned. (E.g. in
> debug_flags in r600_common_screen is an unsigned)
> But maybe that can be ignored?
>
> Though somebody with deeper knowledge of the code should give you a R-b, you can
> have mine, for what it is worth, with the callers fixed:
>         Reviewed-by: Kai Wasserbäch <kai at dev.carbon-project.org>

The type of r600_common_screen::debug_flags is changed by a later patch.

I don't think the callers need updating if they don't use the higher
bits. The behavior remains the same. For radeon, the first patch using
the higher bits updates the type as well.

Marek