Re: [PATCH 04/10] wiphy: fix runtime error from bit shift

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 7/26/22 12:09, James Prestwood wrote:
> The compiler treated the '1' as an int type which was not big enough
> to hold a bit shift of 31:
> 
> runtime error: left shift of 1 by 31 places cannot be represented in
> 		type 'int'
> 
> Instead of doing the iftype check manually, refactor
> wiphy_get_supported_iftypes by adding a subroutine which just parses
> out iftypes from a mask into a char** list. This removes the need to
> case each iftype into a string.
> ---
>   src/wiphy.c | 48 ++++++++++++++----------------------------------
>   1 file changed, 14 insertions(+), 34 deletions(-)
> 
> diff --git a/src/wiphy.c b/src/wiphy.c
> index 09b99fb2..98bd3aa4 100644
> --- a/src/wiphy.c
> +++ b/src/wiphy.c
> @@ -707,17 +707,16 @@ bool wiphy_constrain_freq_set(const struct wiphy *wiphy,
>   	return true;
>   }
>   
> -static char **wiphy_get_supported_iftypes(struct wiphy *wiphy, uint16_t mask)
> +static char **wiphy_iftype_mask_to_str(uint16_t mask)

I'm still unsure why you introduce this? Can't you use 
wiphy_get_supported_iftypes() as is?

>   {
> -	uint16_t supported_mask = wiphy->supported_iftypes & mask;
> -	char **ret = l_new(char *, __builtin_popcount(supported_mask) + 1);
> +	char **ret = l_new(char *, __builtin_popcount(mask) + 1);
>   	unsigned int i;
>   	unsigned int j;
>   
> -	for (j = 0, i = 0; i < sizeof(supported_mask) * 8; i++) {
> +	for (j = 0, i = 0; i < sizeof(mask) * 8; i++) {
>   		const char *str;
>   
> -		if (!(supported_mask & (1 << i)))
> +		if (!(mask & (1 << i)))
>   			continue;
>   
>   		str = netdev_iftype_to_string(i + 1);

Also note that since valid iftypes start at 1, we actually subtract 1 when 
parsing them for the purposes of the mask.  So get_iftypes() might need to be 
modified to do the same.

Regards,
-Denis