* [PATCH] staging: comedi: serial2002: bare unsigned and unneeded cast styling issues
@ 2016-06-08 19:59 Chris Cesare
2016-06-09 9:27 ` Ian Abbott
0 siblings, 1 reply; 2+ messages in thread
From: Chris Cesare @ 2016-06-08 19:59 UTC (permalink / raw)
To: abbotti; +Cc: devel, linux-kernel, Chris Cesare
checkpatch.pl reported two warnings: A bare "unsigned" and an
unnecessary cast. Fixed both.
Signed-off-by: Chris Cesare <chris.cesare@gmail.com>
---
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++;
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] staging: comedi: serial2002: bare unsigned and unneeded cast styling issues
2016-06-08 19:59 [PATCH] staging: comedi: serial2002: bare unsigned and unneeded cast styling issues Chris Cesare
@ 2016-06-09 9:27 ` Ian Abbott
0 siblings, 0 replies; 2+ messages in thread
From: Ian Abbott @ 2016-06-09 9:27 UTC (permalink / raw)
To: Chris Cesare; +Cc: devel, linux-kernel
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@gmail.com>
> ---
> 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@mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-06-09 9:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08 19:59 [PATCH] staging: comedi: serial2002: bare unsigned and unneeded cast styling issues Chris Cesare
2016-06-09 9:27 ` Ian Abbott
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).