Re: [PATCH 17/19] scsi_transport_srp: Suppress a W=1 compiler warning

[Bart Van Assche <Bart.VanAssche@wdc.com>]

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.vanassche@wdc.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Hannes Reinecke <hare@suse.de>
> > Cc: Johannes Thumshirn <jthumshirn@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.