On 6/23/22 05:57, Sergey Shtylyov wrote:
> While ata_ioc32() returns 'int', its result gets assigned to and compared
> with the 'unsigned long' variable 'val' in ata_sas_scsi_ioctl(), its only
> caller, which implies a problematic implicit cast -- fix that by returning
> 'bool' instead (actually, the object code doesn't seem to change, probably
> because the function is always inlined).
Looks good. I would though prefer to change this commit message to simply
say that ata_ioc32() return value is clearly a bool. The implicit cast to
unsigned long from int is not really an issue at all (the reverse cast
would be an issue). And keep mentioning that the object code does not change.
By the way, does your statis analyzer stop complaining after this change ?
Because we still have an implicit cast in the user site.
>
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
>
> ---
> This patch is against the 'for-next' branch of Damien's 'libata.git' repo.
>
> Changes in version 2:
> - changed the result type of ata_ioc32() to 'bool', updating the 'return'
> statements as well;
> - dropped "sloppy" from the patch subject;
> - added a note about the object code to the patch description;
> - changed * to ' in the patch description.
>
> drivers/ata/libata-scsi.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> Index: libata/drivers/ata/libata-scsi.c
> ===================================================================
> --- libata.orig/drivers/ata/libata-scsi.c
> +++ libata/drivers/ata/libata-scsi.c
> @@ -539,13 +539,13 @@ int ata_task_ioctl(struct scsi_device *s
> return rc;
> }
>
> -static int ata_ioc32(struct ata_port *ap)
> +static bool ata_ioc32(struct ata_port *ap)
> {
> if (ap->flags & ATA_FLAG_PIO_DMA)
> - return 1;
> + return true;
> if (ap->pflags & ATA_PFLAG_PIO32)
> - return 1;
> - return 0;
> + return true;
> + return false;
> }
>
> /*
--
Damien Le Moal
Western Digital Research
