| Message ID | 20170728144511.tf7nc672hwo35rnn@mwanda |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Dan Carpenter <dan.carpenter@oracle.com> Date: Fri, 28 Jul 2017 17:45:11 +0300 > - __u16 rval; > + __u16 rval = -1; Fixing a bogus warning by assigning a signed constant to an unsigned variable doesn't really make me all that happy. I don't think I'll apply this, sorry.
On Sat, Jul 29, 2017 at 11:28:55PM -0700, David Miller wrote: > From: Dan Carpenter <dan.carpenter@oracle.com> > Date: Fri, 28 Jul 2017 17:45:11 +0300 > > > - __u16 rval; > > + __u16 rval = -1; > > Fixing a bogus warning by assigning a signed constant to an > unsigned variable doesn't really make me all that happy. > > I don't think I'll apply this, sorry. There's no guarantee that small kmallocs will always succeed in future kernels so it's not *totally* bogus. regards, dan carpenter
From: Dan Carpenter <dan.carpenter@oracle.com> Date: Mon, 31 Jul 2017 10:41:40 +0300 > On Sat, Jul 29, 2017 at 11:28:55PM -0700, David Miller wrote: >> From: Dan Carpenter <dan.carpenter@oracle.com> >> Date: Fri, 28 Jul 2017 17:45:11 +0300 >> >> > - __u16 rval; >> > + __u16 rval = -1; >> >> Fixing a bogus warning by assigning a signed constant to an >> unsigned variable doesn't really make me all that happy. >> >> I don't think I'll apply this, sorry. > > There's no guarantee that small kmallocs will always succeed in future > kernels so it's not *totally* bogus. Perhaps the burdon of initializing the value belongs in mcs_get_reg(), and you can set it properly to 0xffff instead of -1. Ok?
On Mon, Jul 31, 2017 at 10:37:16AM -0700, David Miller wrote: > From: Dan Carpenter <dan.carpenter@oracle.com> > Date: Mon, 31 Jul 2017 10:41:40 +0300 > > > On Sat, Jul 29, 2017 at 11:28:55PM -0700, David Miller wrote: > >> From: Dan Carpenter <dan.carpenter@oracle.com> > >> Date: Fri, 28 Jul 2017 17:45:11 +0300 > >> > >> > - __u16 rval; > >> > + __u16 rval = -1; > >> > >> Fixing a bogus warning by assigning a signed constant to an > >> unsigned variable doesn't really make me all that happy. > >> > >> I don't think I'll apply this, sorry. > > > > There's no guarantee that small kmallocs will always succeed in future > > kernels so it's not *totally* bogus. > > Perhaps the burdon of initializing the value belongs in > mcs_get_reg(), and you can set it properly to 0xffff > instead of -1. > > Ok? Sure. I will resend. thanks, dan carpenter
diff --git a/drivers/net/irda/mcs7780.c b/drivers/net/irda/mcs7780.c index 765de3bedb88..64b29880e534 100644 --- a/drivers/net/irda/mcs7780.c +++ b/drivers/net/irda/mcs7780.c @@ -585,7 +585,7 @@ static int mcs_speed_change(struct mcs_cb *mcs) int rst = 0; int cnt = 0; __u16 nspeed; - __u16 rval; + __u16 rval = -1; nspeed = mcs_speed_set[(mcs->new_speed >> 8) & 0x0f];
My static checker complains that, if the allocation in mcs_get_reg() fails, it means we use "rval" without initializing it. Small allocations never fail in current kernels so it's not a major concern but it's simple enough to silence the warning. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>