On Thu, 2017-08-24 at 11:11 +0200, Christoph Hellwig wrote:
> On Wed, Aug 23, 2017 at 02:40:07PM -0700, Bart Van Assche wrote:
> > Avoid that the following compiler warning is reported when building
> > with W=1:
> > 
> > drivers/scsi/scsi_transport_srp.c:92:19: warning: comparison is always 
> > false due to limited range of data type [-Wtype-limits]
> > 
> > Signed-off-by: Bart Van Assche <bart.vanass...@wdc.com>
> > Cc: Christoph Hellwig <h...@lst.de>
> > Cc: Hannes Reinecke <h...@suse.de>
> > Cc: Johannes Thumshirn <jthumsh...@suse.de>
> > ---
> >  drivers/scsi/scsi_transport_srp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/scsi_transport_srp.c 
> > b/drivers/scsi/scsi_transport_srp.c
> > index 698cc4681706..b8f5e4c47579 100644
> > --- a/drivers/scsi/scsi_transport_srp.c
> > +++ b/drivers/scsi/scsi_transport_srp.c
> > @@ -89,7 +89,7 @@ int srp_tmo_valid(int reconnect_delay, int 
> > fast_io_fail_tmo, int dev_loss_tmo)
> >     if (fast_io_fail_tmo < 0 &&
> >         dev_loss_tmo > SCSI_DEVICE_BLOCK_MAX_TIMEOUT)
> >             return -EINVAL;
> > -   if (dev_loss_tmo >= LONG_MAX / HZ)
> > +   if (dev_loss_tmo + 0UL >= LONG_MAX / HZ)
> >             return -EINVAL;
> 
> That's a weird change..  If we can to promote dev_loss_tmo to
> long we should just cast it.  And of course we should always ask
> us why we got this warning.  If long is 64-bit and int is 32-bit
> the warning makes sense, as for every reasonable comparism
> the 32-bit timeout can't be larger than LONG_MAX divided by 100 or 100.
> 
> But for 32-bit longs it can, so we should keep it, maybe with a comment.

Hello Christoph,

The purpose of that check is to avoid that dev_loss_tmo * HZ can overflow.
That check is only needed on 32-bit systems since only on these systems
sizeof(long) == sizeof(int). How about changing the type of the dev_loss_tmo
argument from int to long such that no explicit cast is needed?

Thanks,

Bart.

Reply via email to