| Message ID | 20220726170920.15929-4-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [01/10] manager: unregister nl80211 config watch | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-ci-gitlint | fail | [04/10] wiphy: fix runtime error from bit shift 7: B3 Line contains hard tab characters (\t): " type 'int'" |
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
On Tue, 2022-07-26 at 15:57 -0500, Denis Kenzior wrote: > 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? Only to avoid passing the wiphy pointer to all these print functions. Otherwise yes, the original could be used as-is.
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) { - 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); @@ -728,6 +727,11 @@ static char **wiphy_get_supported_iftypes(struct wiphy *wiphy, uint16_t mask) return ret; } +static char **wiphy_get_supported_iftypes(struct wiphy *wiphy, uint16_t mask) +{ + return wiphy_iftype_mask_to_str(wiphy->supported_iftypes & mask); +} + bool wiphy_supports_iftype(struct wiphy *wiphy, uint32_t iftype) { if (iftype > sizeof(wiphy->supported_iftypes) * 8) @@ -951,38 +955,14 @@ static void wiphy_print_mcs_info(const uint8_t *mcs_map, static void wiphy_print_he_capabilities(struct band *band, const struct band_he_capabilities *he_cap) { - int i; - char type_buf[128]; - char *s = type_buf; + _auto_(l_strv_free) char **iftypes = NULL; + _auto_(l_free) char *joined = NULL; uint8_t width_set = bit_field(he_cap->he_phy_capa[0], 1, 7); - for (i = 0; i < 32; i++) { - if (!(he_cap->iftypes & (1 << i))) - continue; - - if (L_WARN_ON(s >= type_buf + sizeof(type_buf))) - return; - - switch (i) { - case NETDEV_IFTYPE_ADHOC: - s += sprintf(s, "%s ", "Ad-Hoc"); - break; - case NETDEV_IFTYPE_STATION: - s += sprintf(s, "%s ", "Station"); - break; - case NETDEV_IFTYPE_AP: - s += sprintf(s, "%s ", "AP"); - break; - case NETDEV_IFTYPE_P2P_CLIENT: - s += sprintf(s, "%s ", "P2P Client"); - break; - case NETDEV_IFTYPE_P2P_GO: - s += sprintf(s, "%s ", "P2P GO"); - break; - } - } + iftypes = wiphy_iftype_mask_to_str(he_cap->iftypes); + joined = l_strjoinv(iftypes, ' '); - l_info("\t\t\tInterface Types: %s", type_buf); + l_info("\t\t\tInterface Types: %s", joined); switch (band->freq) { case BAND_FREQ_2_4_GHZ: