From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758537AbcLOP3b (ORCPT ); Thu, 15 Dec 2016 10:29:31 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:36505 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755300AbcLOP3a (ORCPT ); Thu, 15 Dec 2016 10:29:30 -0500 Date: Thu, 15 Dec 2016 16:29:30 +0100 From: Johan Hovold To: Geert Uytterhoeven Cc: Johan Hovold , "Ji-Ze Hong (Peter Hong)" , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] USB: serial: f81534: Detect errors from f81534_logic_to_phy_port() Message-ID: <20161215152930.GQ6085@localhost> References: <1481714333-14014-1-git-send-email-geert@linux-m68k.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1481714333-14014-1-git-send-email-geert@linux-m68k.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 14, 2016 at 12:18:53PM +0100, Geert Uytterhoeven wrote: > With gcc 4.1.2: > > drivers/usb/serial/f81534.c: In function ‘f81534_port_probe’: > drivers/usb/serial/f81534.c:1250: warning: comparison is always false due to limited range of data type > > f81534_logic_to_phy_port() may return a negative error value, which is > ignored by assigning it to u8 f81534_port_private.phy_num. Thanks for fixing this. > Use an intermediate variable of type int to fix this. > While at it, forward the actual error code instead of converting it to > -ENODEV. > > Fixes: 0c9bd6004d258d46 ("USB: serial: add Fintek F81532/534 driver") > Signed-off-by: Geert Uytterhoeven > --- > drivers/usb/serial/f81534.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c > index 8282a6a18fee83f6..2acad268f9d15f43 100644 > --- a/drivers/usb/serial/f81534.c > +++ b/drivers/usb/serial/f81534.c > @@ -1237,6 +1237,7 @@ static int f81534_attach(struct usb_serial *serial) > static int f81534_port_probe(struct usb_serial_port *port) > { > struct f81534_port_private *port_priv; > + int ret; > > port_priv = devm_kzalloc(&port->dev, sizeof(*port_priv), GFP_KERNEL); > if (!port_priv) > @@ -1246,10 +1247,14 @@ static int f81534_port_probe(struct usb_serial_port *port) > mutex_init(&port_priv->mcr_mutex); > > /* Assign logic-to-phy mapping */ > - port_priv->phy_num = f81534_logic_to_phy_port(port->serial, port); > - if (port_priv->phy_num < 0 || port_priv->phy_num >= F81534_NUM_PORT) > + ret = f81534_logic_to_phy_port(port->serial, port); > + if (ret < 0) > + return ret; > + > + if (ret >= F81534_NUM_PORT) > return -ENODEV; How about dropping the redundant >= F81534_NUM_PORT test as well as the helper function will never return anything that large? > > + port_priv->phy_num = ret; > usb_set_serial_port_data(port, port_priv); > dev_dbg(&port->dev, "%s: port_number: %d, phy_num: %d\n", __func__, > port->port_number, port_priv->phy_num); Thanks, Johan