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 <chris.cesare@xxxxxxxxx> --- 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: <abbotti@xxxxxxxxx> )=- -=( Web: http://www.mev.co.uk/ )=- _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel