Re: [PATCH] lib/sort: adjust types

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.12.2020 18:05, Andrew Cooper wrote:
> On 22/12/2020 16:49, Jan Beulich wrote:
>> First and foremost do away with the use of plain int for sizes or size-
>> derived values. Use size_t, despite this requiring some adjustment to
>> the logic. Also replace u32 by uint32_t.
>>
>> While not directly related also drop a leftover #ifdef from x86's
>> swap_ex - this was needed only back when 32-bit Xen was still a thing.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks.

>> --- a/xen/lib/sort.c
>> +++ b/xen/lib/sort.c
>> @@ -6,14 +6,15 @@
>>  
>>  #include <xen/types.h>
>>  
>> -static void u32_swap(void *a, void *b, int size)
>> +static void u32_swap(void *a, void *b, size_t size)
>>  {
>> -    u32 t = *(u32 *)a;
>> -    *(u32 *)a = *(u32 *)b;
>> -    *(u32 *)b = t;
>> +    uint32_t t = *(uint32_t *)a;
>> +
>> +    *(uint32_t *)a = *(uint32_t *)b;
>> +    *(uint32_t *)b = t;
>>  }
>>  
>> -static void generic_swap(void *a, void *b, int size)
>> +static void generic_swap(void *a, void *b, size_t size)
>>  {
>>      char t;
>>  
>> @@ -43,18 +44,18 @@ static void generic_swap(void *a, void *
>>  
>>  void sort(void *base, size_t num, size_t size,
>>            int (*cmp)(const void *, const void *),
>> -          void (*swap)(void *, void *, int size))
>> +          void (*swap)(void *, void *, size_t size))
>>  {
>>      /* pre-scale counters for performance */
>> -    int i = (num / 2 - 1) * size, n = num * size, c, r;
>> +    size_t i = (num / 2) * size, n = num * size, c, r;
>>  
>>      if ( !swap )
>>          swap = (size == 4 ? u32_swap : generic_swap);
>>  
>>      /* heapify */
>> -    for ( ; i >= 0; i -= size )
>> +    while ( i > 0 )
>>      {
>> -        for ( r = i; r * 2 + size < n; r  = c )
>> +        for ( r = i -= size; r * 2 + size < n; r  = c )
> 
> Aren't some compilers going to complain at the lack of brackets for this
> setup of r?

I've never seen any warning on this type of construct. It's no
different from "a = b = c;".

> Also as you're editing the line, the "r  = c" part should lose one space.

Oh, indeed.

Jan