Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Fri, 8 Apr 2022 09:31:33 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Joe Perches <joe@perches.com>
Cc:     Julia Lawall <julia.lawall@inria.fr>,
        Rebecca Mckeever <remckee0@gmail.com>,
        outreachy@lists.linux.dev,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with
 if and assignment
Message-ID: <20220408063133.GP12805@kadam>
References: <cover.1649378587.git.remckee0@gmail.com>
 <36059ec66a2f3d58a8e339aa4f262772eabd3ef0.1649378587.git.remckee0@gmail.com>
 <alpine.DEB.2.22.394.2204080614400.2199@hadrien>
 <20220408055732.GO3293@kadam>
 <34d0ad73f44ff4990f6bee49105ac49bb55352a5.camel@perches.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <34d0ad73f44ff4990f6bee49105ac49bb55352a5.camel@perches.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ecSGDiklPhu3R8G7OPzyXITzVOXYOnDNHFhMah757eoLwJGpEtMU89tLknf5?=
 =?us-ascii?Q?fVymcket/4OEIIW8ptUzMgQ6hNWnj7GsQBDXsCgRKxImpQ76ZdxTuCWv8sE2?=
 =?us-ascii?Q?EvHvJT8hQlNieDejKSOdx97rleO8HvNlvOvRwT3XqQp3ODI/0yfzuTBoBEDS?=
 =?us-ascii?Q?oCX421La0EEmG9jjf5H4x5SKUceqXLRd2L6iPp/KbErHORP+FeDEqsfwwEGu?=
 =?us-ascii?Q?Sg4qpOIf9URA0t02cYt7SQJZPNhct4Xoc5ZOO5rfJQOEVfDst0ACDps3V0QI?=
 =?us-ascii?Q?TuS7FQDS7E3CqHBGrvS8c6SRngDoPVvTKkUDgIuEdWIP4RY/dgMGIK1OfbUq?=
 =?us-ascii?Q?29HWdwdiqOCabvrbX50Rw6Vt2G/raoNZ74fJI38WFVYdsHxxWiOrdhm3UR4D?=
 =?us-ascii?Q?pOxGujPuQ0C+j3SyuWNjqwJFAZzUjM2l2136UyfXFsATXtnRQIZMm4SeWXwn?=
 =?us-ascii?Q?4+XP1XFWsPJnVOLZJ1fVaih/Upl3JYVUl/T79YEBcNYj3vH5vTfGqhfU476O?=
 =?us-ascii?Q?HsKQHHy/w+KLNJvV6E4HPjVDaxYhcW1DaweZOyeCq2k55MkrJmKjK1xogtoD?=
 =?us-ascii?Q?CkWPPzhOejmUb2ltd2Jt85gM0gV8dWv69TYRPAyE2ccChwMCkLA1uKiTVsVz?=
 =?us-ascii?Q?nTpPcu9bmfaD7DqVmocSGb2D0YDK4qZ9sEVPdg9ukY/eM5KxnorDoOatjeRB?=
 =?us-ascii?Q?KaGAdRe2e7hkmb245LfNuU1YxwxnO+h8fL5Q+wcnmTmMyefgo6sNzDy+fbok?=
 =?us-ascii?Q?cVlsBm5vaMmCxdY/tUUyQYm7dDUfrI8PCZdsuHm4GIzjQT5lB/REpH8QsCNR?=
 =?us-ascii?Q?bNNdRL5UJ5difaIaUMtAvwcQpHkW4BEgjM/xjogaRn/DVdBVMbDGHXRVfpnK?=
 =?us-ascii?Q?/U7vWYSELckrEK++nLhzWSqZzUBFfb3g0c2lyprckmowQtW9kTGyiFWGvdg8?=
 =?us-ascii?Q?UDHjxnmEJPMVRt2CMuU8vU9dOUgmEfKGndAattkfbxcBxnG0XGwRKBu6Sbe3?=
 =?us-ascii?Q?mBr3URISTNlCGgyeOjff0CPaSzsDA2kvYX8wShgDUEXAXrLyLCrgZ+2O+Ytr?=
 =?us-ascii?Q?C0ltDjMZ0bAaQ1FrJztaTBZDnbxiTsjl2p5quz6Hcv2PY130uKnP006lPHpQ?=
 =?us-ascii?Q?pZt2XePEgSQLKaelgsIJbKSRZMvXMaBbJhuI9OOLevy/8mnCdoKJmmXVy8Mb?=
 =?us-ascii?Q?u67le08dA6VCn+exfMunUqfgI83bGObdcm9347R0Sk9Ve/+jDng2wV4w11+n?=
 =?us-ascii?Q?7JYNaZFxOdKY/edb2wVfICH8ZuFqpEGweZJY2THkwd0Op6ix1hzVcNtY6wG+?=
 =?us-ascii?Q?wjI6WuVkH1qhwqgHTo2dv6uszdxXFko6AEXPFqEgHhwlHTfgxxQWXAciNxHk?=
 =?us-ascii?Q?/WPNdTirsF+QCtEicqu/Van+oLdGS3lYjzUbOsA1GDHGguk/zUFrOJLe7vcX?=
 =?us-ascii?Q?9gOwxZOlYepZLFWvzvjKTNUYZQks4UM8omBfw5MSTBmoh9BxV19MW2iFiDsi?=
 =?us-ascii?Q?fHwxdepbyDQD4Q7bOXJsdXEK/RP29aLV2oI7niYtMH4eFbRxgtU4wcabLPiX?=
 =?us-ascii?Q?AlufooUmafrGTzhQNFOCO+Yr7CRv3AcQc55Ex0+zbkAuKjA5rsiBq1pbVEOO?=
 =?us-ascii?Q?XARArjgCkpg3eBWrc09cU3vo8U+u0CQGTwI1EliuT5Yrmp3cVClWrliWEDva?=
 =?us-ascii?Q?BzVXBrPXHn781c280oh4cbfzbOY98VM1666tXFEiPZYTiyxyraa0TiGWmuZD?=
 =?us-ascii?Q?klFl+cPXfdlfCobtmTJ8pvje722U8Fc=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3a42832a-8791-423d-ba27-08da19297bdf
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2022 06:31:59.0209
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: o0VZzN1X3rVFGT2bMHfPHtGf/PiO2aEOZRE+AsTjDj0pmZOf9PhS8b8ye1SjuyhcaIDlSsMcsHRdMQdsXoOyGdt6/EaQTINaHOEqRlwGsrI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR10MB5522
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-04-07_01:2022-04-07,2022-04-07 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=891 malwarescore=0
 mlxscore=0 suspectscore=0 bulkscore=0 adultscore=0 spamscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2204070064
X-Proofpoint-GUID: c33nNitzHFYa1iQdGikKXAvRQq7E1iBg
X-Proofpoint-ORIG-GUID: c33nNitzHFYa1iQdGikKXAvRQq7E1iBg
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Apr 07, 2022 at 11:14:51PM -0700, Joe Perches wrote:
> On Fri, 2022-04-08 at 08:57 +0300, Dan Carpenter wrote:
> > On Fri, Apr 08, 2022 at 06:15:14AM +0200, Julia Lawall wrote:
> > > On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
> > > 
> > > > Replace ternary statement with an if statement followed by an assignment
> > > > to increase readability and make error handling more obvious.
> > > > Found with minmax coccinelle script.
> []
> > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> []
> > > > @@ -470,7 +470,9 @@ 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);
> > > > +	if (len < 0)
> > > > +		len = 0;
> > > > +	erq->length = len;
> > > 
> > > Maybe you could use max here?
> > 
> > Initially Rebecca did use max() but I NAKed it.  It's really not less
> > readable.  Better to handle the error explicitly.  Keep the error path
> > indented two tabs.  Separate from the success path.
> 
> A comment would be useful as it's not obvious it's an 'error' path.
> One has to read all 3 get_key functions to determine that.
> 

I'm so confused.  Negative error codes are the common case in the
kernel.  We don't need to comment it.

Obviously in an ideal world the get_key() functions would return -EINVAL
instead of -1...

regards,
dan carpenter