Re: [PATCH wireless-next v4 09/21] wifi: nl80211/cfg80211: add support for SMD context programming

[Johannes Berg]

On Fri, 2026-11-06 at 13:28 +0530, Pooventhiran G wrote:
> 
> +static int
> +nl80211_parse_smd_ctx_ba_params(struct nlattr *tb,
> +				struct ieee80211_smd_ctx_ba *ba_tids)
> +{
> +	struct nlattr *ba_tb[NL80211_SMD_CTX_BA_ATTR_MAX + 1];
> +	struct ieee80211_smd_ctx_ba *ba;
> +	struct nlattr *ba_attr;
> +	int n_tids = 0;
> +	int rem, tid;
> +
> +	nla_for_each_nested(ba_attr, tb, rem) {
> +		tid = nla_type(ba_attr) - 1;
> +		if (tid < 0 || tid >= IEEE80211_SMD_CTX_NUM_TIDS)
> +			return -EINVAL;
> +
> +		if (nla_parse_nested(ba_tb, NL80211_SMD_CTX_BA_ATTR_MAX,
> +				     ba_attr, nl80211_smd_ctx_ba_policy,
> +				     NULL))

if you set up the policies correctly (and you should, but I didn't go
back to check now) then you don't need the policy here

> +		ba->amsdu_supported =
> +			!!ba_tb[NL80211_SMD_CTX_BA_ATTR_AMSDU_SUPPORT];

it's not super useful to use !! to assign to a bool, and if you wanted
to be fancy you'd use nla_flag(), right? ;)

> +		if (nla_parse_nested(dl_tb, NL80211_SMD_CTX_DL_ATTR_MAX,
> +				     tb[NL80211_SMD_CTX_ATTR_DL],
> +				     nl80211_smd_ctx_dl_policy, NULL)) {

same here, and then it kind of cannot fail, I think

> +			nla_for_each_nested(sn_attr,
> +					    dl_tb[NL80211_SMD_CTX_DL_ATTR_SN],
> +					    rem) {
> +				tid = nla_type(sn_attr) - 1;
> +				if (tid < 0 ||
> +				    tid >= IEEE80211_SMD_CTX_NUM_TIDS) {
> +					GENL_SET_ERR_MSG(info,
> +							 "invalid DL SN tid");
> +					goto err_free_ctx;
> +				}
> +
> +				if (nla_len(sn_attr) != sizeof(u16)) {
> +					GENL_SET_ERR_MSG(info,
> +							 "invalid DL SN len");
> +					goto err_free_ctx;
> +				}

You've got to be _kidding_ me, why do I even have to get see such
obviously redundant code?

Please read your own code.

johannes