* linux-4.8-rc1/drivers/scsi/sd.c:317: pointless test ?
@ 2016-08-08 13:46 David Binderman
2016-08-08 20:17 ` Laurence Oberman
0 siblings, 1 reply; 2+ messages in thread
From: David Binderman @ 2016-08-08 13:46 UTC (permalink / raw)
To: jejb, martin.petersen, linux-scsi, Linux Kernel Mailing List, dcb314
Hello there,
linux-4.8-rc1/drivers/scsi/sd.c:317]: (style) Unsigned variable 'val'
can't be negative so it is unnecessary to test it.
Source code is
if (val >= 0 && val <= SD_DIF_TYPE3_PROTECTION)
but
unsigned int val;
Suggest new code
if (val <= SD_DIF_TYPE3_PROTECTION)
Regards
David Binderman
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: linux-4.8-rc1/drivers/scsi/sd.c:317: pointless test ?
2016-08-08 13:46 linux-4.8-rc1/drivers/scsi/sd.c:317: pointless test ? David Binderman
@ 2016-08-08 20:17 ` Laurence Oberman
0 siblings, 0 replies; 2+ messages in thread
From: Laurence Oberman @ 2016-08-08 20:17 UTC (permalink / raw)
To: David Binderman
Cc: jejb, martin petersen, linux-scsi, Linux Kernel Mailing List, dcb314
----- Original Message -----
> From: "David Binderman" <linuxdev.baldrick@gmail.com>
> To: jejb@linux.vnet.ibm.com, "martin petersen" <martin.petersen@oracle.com>, linux-scsi@vger.kernel.org, "Linux
> Kernel Mailing List" <linux-kernel@vger.kernel.org>, dcb314@hotmail.com
> Sent: Monday, August 8, 2016 9:46:53 AM
> Subject: linux-4.8-rc1/drivers/scsi/sd.c:317: pointless test ?
>
> Hello there,
>
> linux-4.8-rc1/drivers/scsi/sd.c:317]: (style) Unsigned variable 'val'
> can't be negative so it is unnecessary to test it.
>
> Source code is
>
> if (val >= 0 && val <= SD_DIF_TYPE3_PROTECTION)
>
> but
>
> unsigned int val;
>
> Suggest new code
>
> if (val <= SD_DIF_TYPE3_PROTECTION)
>
> Regards
>
> David Binderman
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Hello
Declaring val as unsigned does not prevent it from being assigned a negative integer.
This means there is a possibility it could be negative.
However looking at the code it seems that we are converting char *buf (string) to an int to set val here.
Also the kernel code shows we cannot have a - sign anyway so I expect your suggestion should be fine to change this.
static ssize_t
protection_type_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct scsi_disk *sdkp = to_scsi_disk(dev);
unsigned int val;
int err;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
err = kstrtouint(buf, 10, &val); *****
if (err)
return err;
if (val >= 0 && val <= SD_DIF_TYPE3_PROTECTION)
sdkp->protection_type = val;
Reviewed-by
Laurence Oberman <loberman@redhat.com>
return count;
}
/**
* kstrtouint - convert a string to an unsigned int
* @s: The start of the string. The string must be null-terminated, and may also
* include a single newline before its terminating null. The first character
* may also be a plus sign, but not a minus sign.
* @base: The number base to use. The maximum supported base is 16. If base is
* given as 0, then the base of the string is automatically detected with the
* conventional semantics - If it begins with 0x the number will be parsed as a
* hexadecimal (case insensitive), if it otherwise begins with 0, it will be
* parsed as an octal number. Otherwise it will be parsed as a decimal.
* @res: Where to write the result of the conversion on success.
*
* Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
* Used as a replacement for the obsolete simple_strtoull. Return code must
* be checked.
*/
int kstrtouint(const char *s, unsigned int base, unsigned int *res)
{
unsigned long long tmp;
int rv;
rv = kstrtoull(s, base, &tmp);
if (rv < 0)
return rv;
if (tmp != (unsigned long long)(unsigned int)tmp)
return -ERANGE;
*res = tmp;
return 0;
}
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-08-08 20:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-08 13:46 linux-4.8-rc1/drivers/scsi/sd.c:317: pointless test ? David Binderman
2016-08-08 20:17 ` Laurence Oberman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.