Re: -Wsizeof-array-div in mm/hugetlb.c

[Mike Kravetz <mike.kravetz@oracle.com>]

On 9/17/19 12:34 AM, Nathan Chancellor wrote:
> Hi all,
> 
> Clang recently added a new diagnostic in r371605, -Wsizeof-array-div,
> that tries to warn when sizeof(X) / sizeof(Y) does not compute the
> number of elements in an array X (i.e., sizeof(Y) is wrong). See that
> commit for more details:
> 
> https://github.com/llvm/llvm-project/commit/3240ad4ced0d3223149b72a4fc2a4d9b67589427
> 
> There is a warning in mm/hugetlb.c in hugetlb_fault_mutex_hash:
> 
> mm/hugetlb.c:4055:40: warning: expression does not compute the number of
> elements in this array; element type is 'unsigned long', not 'u32' (aka
> 'unsigned int') [-Wsizeof-array-div]
>         hash = jhash2((u32 *)&key, sizeof(key)/sizeof(u32), 0);
>                                           ~~~ ^
> mm/hugetlb.c:4049:16: note: array 'key' declared here
>         unsigned long key[2];
>                       ^
> 1 warning generated.
> 
> Should this warning be silenced? What is the reasoning behind having key
> be an array of unsigned longs but representing it as an array of u32s?

Well, the second argument to jhash2 is "the number of u32's in the key".
This is the reason for the sizeof(key)/sizeof(u32) calculation.  It certainly
is not trying to calculate the number of elements in the array as suggested by
the warning.

> Would it be better to avoid the cast and have it just be an array of
> u32s directly?

I did not write this code, but it is much easier to do the assignments (below)
to build the key if the array is unsigned long as opposed to u32.

struct address_space *mapping;
pgoff_t idx;
unsigned long key[2];

        key[0] = (unsigned long) mapping;
        key[1] = idx;

> u32s directly? I am not familiar with this code so I may be naive for
> asking such questions but we'd like to get these warnings cleaned up so
> that this warning can be useful down the road.

I suppose it would be possible to change 'key' to be something else besides
an array (such as struct or union) to eliminate the warning.  But, I would
prefer to have some type of directive to indicate the code is ok as is.  It
is not trying to calculate the number of elements in the array as suspected
by the clang diagnostic.

-- 
Mike Kravetz