From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424270AbcFIJ17 (ORCPT ); Thu, 9 Jun 2016 05:27:59 -0400 Received: from smtp105.iad3a.emailsrvr.com ([173.203.187.105]:49170 "EHLO smtp105.iad3a.emailsrvr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161019AbcFIJ15 (ORCPT ); Thu, 9 Jun 2016 05:27:57 -0400 X-Auth-ID: abbotti@mev.co.uk X-Sender-Id: abbotti@mev.co.uk Subject: Re: [PATCH] staging: comedi: serial2002: bare unsigned and unneeded cast styling issues To: Chris Cesare References: <1465415971-30203-1-git-send-email-chris.cesare@gmail.com> Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org From: Ian Abbott Message-ID: <5759369A.2060509@mev.co.uk> Date: Thu, 9 Jun 2016 10:27:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: <1465415971-30203-1-git-send-email-chris.cesare@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/06/16 20:59, Chris Cesare wrote: > checkpatch.pl reported two warnings: A bare "unsigned" and an > unnecessary cast. Fixed both. > > Signed-off-by: Chris Cesare > --- > drivers/staging/comedi/drivers/serial2002.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/comedi/drivers/serial2002.c b/drivers/staging/comedi/drivers/serial2002.c > index 7a1defc..d3c2743 100644 > --- a/drivers/staging/comedi/drivers/serial2002.c > +++ b/drivers/staging/comedi/drivers/serial2002.c > @@ -95,7 +95,7 @@ struct serial_data { > #define S2002_CFG_SIGN(x) (((x) >> 13) & 0x1) > #define S2002_CFG_BASE(x) (((x) >> 14) & 0xfffff) > > -static long serial2002_tty_ioctl(struct file *f, unsigned op, > +static long serial2002_tty_ioctl(struct file *f, unsigned int op, > unsigned long param) > { > if (f->f_op->unlocked_ioctl) > @@ -379,7 +379,8 @@ static int serial2002_setup_subdevice(struct comedi_subdevice *s, > range_table_list[chan] = > (const struct comedi_lrange *)&range[j]; > } > - maxdata_list[chan] = ((long long)1 << cfg[j].bits) - 1; > + maxdata_list[chan] = (1 << cfg[j].bits) - 1; > + > chan++; > } > } > Logically, the cfg[j].bits value can range from 0 to 63. Practically, I guess it has some fixed set of non-zero values up to a maximum of 32. A shift value of 32 is too large for both a plain, signed int, and an unsigned int (assuming 32-bit int), so the change is a bit dodgy. I propose splitting the patch into 2, fixing the bare unsigned in patch 1, and fixing the cast styling issue in patch 2. Since maxdata_list[chan] is only 32 bits wide, the safest change would be something like this, clamping the value if cfg[j].bits happens to be larger than 32: if (cfg[j].bits < 32) maxdata_list[chan] = (1u << cfg[j].bits) - 1; else maxdata_list[chan] = 0xffffffff; -- -=( Ian Abbott @ MEV Ltd. E-mail: )=- -=( Web: http://www.mev.co.uk/ )=-