On 5/21/2025 12:47 PM, Johannes Berg wrote: > On Tue, 2025-04-29 at 09:30 +0530, Roopni Devanathan wrote: >> >> #define NL80211_WIPHY_NAME_MAXLEN 64 >> +#define NL80211_WIPHY_RADIO_ID_DEFAULT -1 > > I pointed this out before, that doesn't belong to the API. > > https://lore.kernel.org/linux-wireless/bef48349364854ba2ec42262e91b747028310a1c.camel@xxxxxxxxxxxxxxxx/ > > Please address comments or tell me why you disagree. > As discussed, I'll remove the define in next version. >> + if (info->attrs[NL80211_ATTR_WIPHY_RTS_THRESHOLD]) { >> + rts_threshold = nla_get_u32( >> + info->attrs[NL80211_ATTR_WIPHY_RTS_THRESHOLD]); >> + changed |= WIPHY_PARAM_RTS_THRESHOLD; >> + } >> + >> + if (changed) { >> + old_rts_threshold = rdev->wiphy.radio_cfg[radio_id].rts_threshold; >> + >> + rdev->wiphy.radio_cfg[radio_id].rts_threshold = rts_threshold; >> + >> + result = rdev_set_wiphy_params(rdev, radio_id, changed); >> + if (result) >> + rdev->wiphy.radio_cfg[radio_id].rts_threshold = old_rts_threshold; >> + } >> + >> + return result; > > nit: you could invert the "changed" test and save some indentation, > those lines are pretty long. Then you can also not initialize "result=0" > and "return 0" for the !changed case, which is more readable. > Sure, will do this in next version. >> + old_radio_rts_threshold = kcalloc(rdev->wiphy.n_radio, >> + sizeof(u32), >> + GFP_KERNEL); >> + if (!old_radio_rts_threshold) { >> + kfree(old_radio_rts_threshold); >> + return -ENOMEM; > > Hmm? > > Also doesn't that leak? >I'll remove kfree() from within the if() condition and free old_radio_rts_threshold towards the end of its usage. > johannes
