From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3174EC4338F for ; Tue, 3 Aug 2021 05:14:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1390060FC2 for ; Tue, 3 Aug 2021 05:14:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234123AbhHCFOO (ORCPT ); Tue, 3 Aug 2021 01:14:14 -0400 Received: from mail.kernel.org ([198.145.29.99]:60424 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233737AbhHCFOJ (ORCPT ); Tue, 3 Aug 2021 01:14:09 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 24BA160EE7; Tue, 3 Aug 2021 05:13:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1627967638; bh=xa4VRMf9MHhvAN8b2TBx4R7NfCpyIy5U5JlFqM/1uNE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=S8P8vaGLwYMlwvmup3upXwU24mcWEGu0OPG2JbwWw6crE1YM/9PHqg2+0oCTThMjD /Vp/V2SSevpk0kSCYZOaIHagSTH6YGEXyweYO4PSWbcqrrLkE14aXLMOrNVfw7vKQo k/lPh15+fJcfzvJRSQ0xGxW2SyAKUouVwqkdYvHs= Date: Tue, 3 Aug 2021 07:13:56 +0200 From: Greg Kroah-Hartman To: Pete Zaitcev Cc: Salah Triki , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] usb: class: usblp: replace conditional statement with min() Message-ID: References: <20210803002806.GA1541734@pc> <20210802205022.357279fc@suzdal.zaitcev.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210802205022.357279fc@suzdal.zaitcev.lan> Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Mon, Aug 02, 2021 at 08:50:22PM -0500, Pete Zaitcev wrote: > On Tue, 3 Aug 2021 01:28:06 +0100 > Salah Triki wrote: > > > Replace conditional statement with min() in order to make code cleaner. Issue > > found by coccinelle. > > > +++ b/drivers/usb/class/usblp.c > > request, !!dir, recip, value, index, len, retval); > > - return retval < 0 ? retval : 0; > > + return min(retval, 0); > > } > > I'm very much against this change. The function min() is there > for numeric values. But here we have a situation where we > do one thing if there was an error, and another thing if > there was no error. > > This sort of abuse is exactly why blindly clicking heels > whenever a tool tells you is not optimal. > > If the objective is to shut the tool up, please consider > the following instead: > > diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c > index 9596e4279294..bbcbcf199fa9 100644 > --- a/drivers/usb/class/usblp.c > +++ b/drivers/usb/class/usblp.c > @@ -264,7 +264,9 @@ static int usblp_ctrl_msg(struct usblp *usblp, int request, int type, int dir, i > dev_dbg(&usblp->intf->dev, > "usblp_control_msg: rq: 0x%02x dir: %d recip: %d value: %d idx: %d len: %#x result: %d\n", > request, !!dir, recip, value, index, len, retval); > - return retval < 0 ? retval : 0; > + if (retval < 0) > + return retval; > + return 0; > } I agree with Pete here, this is the "correct" fix for this, using min() is not ok here. thanks, greg k-h