Re: [PATCH 1/2] staging: rtl8192u: use max macro instead of ternary operator

[Dan Carpenter]

On Wed, Apr 06, 2022 at 07:09:43PM -0500, Rebecca Mckeever wrote:
> Replace ternary operator with max macro to increase readability
> and conform to Linux kernel coding style.
> Found with minmax coccinelle script.
> 
> Signed-off-by: Rebecca Mckeever <remckee0@xxxxxxxxx>
> ---
>  drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> index 78cc8f357bbc..a10c1303695b 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> @@ -470,7 +470,7 @@ int ieee80211_wx_get_encode(struct ieee80211_device *ieee,
>  		return 0;
>  	}
>  	len = crypt->ops->get_key(keybuf, SCM_KEY_LEN, NULL, crypt->priv);
> -	erq->length = (len >= 0 ? len : 0);
> +	erq->length = max(len, 0);

Neither before nor after is really readable.  It would be better to
write it as:

	len = crypt->ops->get_key(keybuf, SCM_KEY_LEN, NULL, crypt->priv);
	if (len < 0)
		len = 0;
	erq->length = len;

There are few rules/patterns that apply here:
1) Do error handling right away.
2) Do error handling not success handling.
3) Try to keep the error path and the success path separate.
4) Try to keep the success path indented one tab and the error path
   indented two tabs.  It looks like this to the reader:

	a = frob();
	if (fail)
		cleanup;
	b = frob();
	if (fail)
		cleanup;


	success
	check
		fail
	success
	check
		fail

So then if you want to understand what a function does at a high level
you can just skim the code which is indented one tab and ignore the
error handling code.

regards,
dan carpenter