Re: [PATCH 2/2] staging: r8188eu: convert rtw_set_802_11_add_wep error code semantics

[Dan Carpenter]

On Fri, Jul 29, 2022 at 12:11:50AM +0100, Phillip Potter wrote:
> -u8 rtw_set_802_11_add_wep(struct adapter *padapter, struct ndis_802_11_wep *wep)
> +int rtw_set_802_11_add_wep(struct adapter *padapter, struct ndis_802_11_wep *wep)
>  {
>  	int		keyid, res;
>  	struct security_priv *psecuritypriv = &padapter->securitypriv;
> -	u8		ret = _SUCCESS;
> +	int		ret = 0;
>  
>  	keyid = wep->KeyIndex & 0x3fffffff;
>  
>  	if (keyid >= 4) {
> -		ret = false;
> +		ret = -EOPNOTSUPP;
>  		goto exit;
>  	}
>  
> @@ -424,7 +424,7 @@ u8 rtw_set_802_11_add_wep(struct adapter *padapter, struct ndis_802_11_wep *wep)
>  	res = rtw_set_key(padapter, psecuritypriv, keyid, 1);
>  
>  	if (res == _FAIL)
> -		ret = false;
> +		ret = -ENOMEM;
>  exit:
>  
>  	return ret;

No, this isn't right.  This now returns 1 on success and negative
error codes on error.

There are a couple anti-patterns here:

1) Do nothing gotos
2) Mixing error paths and success paths.

If you avoid mixing error paths and success paths then you get a pattern
called: "Solid return zero."  This is where the end of the function has
a very chunky "return 0;" to mark that it is successful.  You want that.
Some people do a "if (ret == 0) return ret;".  Nope.  "return ret;" is
not chunky.

regards,
dan carpenter