* [PATCH] ata: libata-core: fix sloppy parameter type in ata_exec_internal[_sg]()
@ 2022-06-13 18:47 Sergey Shtylyov
2022-06-14 5:24 ` Damien Le Moal
0 siblings, 1 reply; 5+ messages in thread
From: Sergey Shtylyov @ 2022-06-13 18:47 UTC (permalink / raw)
To: Damien Le Moal, linux-ide
Make the 'timeout' parameter to ata_exec_internal_sg() *unsigned int* as
msecs_to_jiffies() that it calls takes just *unsigned int* for the time in
milliseconds. Then follow the suit with ata_exec_internal(), its only
caller; also fix up ata_dev_set_feature(), the only ata_exec_internal()'s
caller that explicitly passes *unsigned long* variable for timeout...
Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
This patch is against the 'for-next' branch of Damien's 'libata.git' repo.
drivers/ata/libata-core.c | 6 +++---
drivers/ata/libata.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
Index: libata/drivers/ata/libata-core.c
===================================================================
--- libata.orig/drivers/ata/libata-core.c
+++ libata/drivers/ata/libata-core.c
@@ -1472,7 +1472,7 @@ static void ata_qc_complete_internal(str
unsigned ata_exec_internal_sg(struct ata_device *dev,
struct ata_taskfile *tf, const u8 *cdb,
int dma_dir, struct scatterlist *sgl,
- unsigned int n_elem, unsigned long timeout)
+ unsigned int n_elem, unsigned int timeout)
{
struct ata_link *link = dev->link;
struct ata_port *ap = link->ap;
@@ -1647,7 +1647,7 @@ unsigned ata_exec_internal_sg(struct ata
unsigned ata_exec_internal(struct ata_device *dev,
struct ata_taskfile *tf, const u8 *cdb,
int dma_dir, void *buf, unsigned int buflen,
- unsigned long timeout)
+ unsigned int timeout)
{
struct scatterlist *psg = NULL, sg;
unsigned int n_elem = 0;
@@ -4339,7 +4339,7 @@ unsigned int ata_dev_set_feature(struct
{
struct ata_taskfile tf;
unsigned int err_mask;
- unsigned long timeout = 0;
+ unsigned int timeout = 0;
/* set up set-features taskfile */
ata_dev_dbg(dev, "set features - SATA features\n");
Index: libata/drivers/ata/libata.h
===================================================================
--- libata.orig/drivers/ata/libata.h
+++ libata/drivers/ata/libata.h
@@ -52,11 +52,11 @@ extern u64 ata_tf_read_block(const struc
extern unsigned ata_exec_internal(struct ata_device *dev,
struct ata_taskfile *tf, const u8 *cdb,
int dma_dir, void *buf, unsigned int buflen,
- unsigned long timeout);
+ unsigned int timeout);
extern unsigned ata_exec_internal_sg(struct ata_device *dev,
struct ata_taskfile *tf, const u8 *cdb,
int dma_dir, struct scatterlist *sg,
- unsigned int n_elem, unsigned long timeout);
+ unsigned int n_elem, unsigned int timeout);
extern int ata_wait_ready(struct ata_link *link, unsigned long deadline,
int (*check_ready)(struct ata_link *link));
extern int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ata: libata-core: fix sloppy parameter type in ata_exec_internal[_sg]()
2022-06-13 18:47 [PATCH] ata: libata-core: fix sloppy parameter type in ata_exec_internal[_sg]() Sergey Shtylyov
@ 2022-06-14 5:24 ` Damien Le Moal
2022-06-14 18:43 ` Sergey Shtylyov
0 siblings, 1 reply; 5+ messages in thread
From: Damien Le Moal @ 2022-06-14 5:24 UTC (permalink / raw)
To: Sergey Shtylyov, linux-ide
On 6/14/22 03:47, Sergey Shtylyov wrote:
> Make the 'timeout' parameter to ata_exec_internal_sg() *unsigned int* as
> msecs_to_jiffies() that it calls takes just *unsigned int* for the time in
> milliseconds. Then follow the suit with ata_exec_internal(), its only
> caller; also fix up ata_dev_set_feature(), the only ata_exec_internal()'s
> caller that explicitly passes *unsigned long* variable for timeout...
>
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
Since you are changing this function signature, can you also make it
static since it is only used in libata-core.c ? The declaration in
drivers/ata/libata.h is useless.
>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>
> ---
> This patch is against the 'for-next' branch of Damien's 'libata.git' repo.
>
> drivers/ata/libata-core.c | 6 +++---
> drivers/ata/libata.h | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> Index: libata/drivers/ata/libata-core.c
> ===================================================================
> --- libata.orig/drivers/ata/libata-core.c
> +++ libata/drivers/ata/libata-core.c
> @@ -1472,7 +1472,7 @@ static void ata_qc_complete_internal(str
> unsigned ata_exec_internal_sg(struct ata_device *dev,
> struct ata_taskfile *tf, const u8 *cdb,
> int dma_dir, struct scatterlist *sgl,
> - unsigned int n_elem, unsigned long timeout)
> + unsigned int n_elem, unsigned int timeout)
> {
> struct ata_link *link = dev->link;
> struct ata_port *ap = link->ap;
> @@ -1647,7 +1647,7 @@ unsigned ata_exec_internal_sg(struct ata
> unsigned ata_exec_internal(struct ata_device *dev,
> struct ata_taskfile *tf, const u8 *cdb,
> int dma_dir, void *buf, unsigned int buflen,
> - unsigned long timeout)
> + unsigned int timeout)
> {
> struct scatterlist *psg = NULL, sg;
> unsigned int n_elem = 0;
> @@ -4339,7 +4339,7 @@ unsigned int ata_dev_set_feature(struct
> {
> struct ata_taskfile tf;
> unsigned int err_mask;
> - unsigned long timeout = 0;
> + unsigned int timeout = 0;
>
> /* set up set-features taskfile */
> ata_dev_dbg(dev, "set features - SATA features\n");
> Index: libata/drivers/ata/libata.h
> ===================================================================
> --- libata.orig/drivers/ata/libata.h
> +++ libata/drivers/ata/libata.h
> @@ -52,11 +52,11 @@ extern u64 ata_tf_read_block(const struc
> extern unsigned ata_exec_internal(struct ata_device *dev,
> struct ata_taskfile *tf, const u8 *cdb,
> int dma_dir, void *buf, unsigned int buflen,
> - unsigned long timeout);
> + unsigned int timeout);
> extern unsigned ata_exec_internal_sg(struct ata_device *dev,
> struct ata_taskfile *tf, const u8 *cdb,
> int dma_dir, struct scatterlist *sg,
> - unsigned int n_elem, unsigned long timeout);
> + unsigned int n_elem, unsigned int timeout);
> extern int ata_wait_ready(struct ata_link *link, unsigned long deadline,
> int (*check_ready)(struct ata_link *link));
> extern int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ata: libata-core: fix sloppy parameter type in ata_exec_internal[_sg]()
2022-06-14 5:24 ` Damien Le Moal
@ 2022-06-14 18:43 ` Sergey Shtylyov
2022-06-14 23:46 ` [PATCH] ata: libata-core: [_sg]fix sloppy parameter type in ata_exec_internal() Damien Le Moal
0 siblings, 1 reply; 5+ messages in thread
From: Sergey Shtylyov @ 2022-06-14 18:43 UTC (permalink / raw)
To: Damien Le Moal, linux-ide
On 6/14/22 8:24 AM, Damien Le Moal wrote:
[...]
>> Make the 'timeout' parameter to ata_exec_internal_sg() *unsigned int* as
>> msecs_to_jiffies() that it calls takes just *unsigned int* for the time in
>> milliseconds. Then follow the suit with ata_exec_internal(), its only
>> caller; also fix up ata_dev_set_feature(), the only ata_exec_internal()'s
>> caller that explicitly passes *unsigned long* variable for timeout...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>
> Since you are changing this function signature, can you also make it
> static since it is only used in libata-core.c ? The declaration in
> drivers/ata/libata.h is useless.
Hopefully you don't mean I should do it in the same patch? :-)
[...]
MBR, Sergey
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ata: libata-core: [_sg]fix sloppy parameter type in ata_exec_internal()
2022-06-14 18:43 ` Sergey Shtylyov
@ 2022-06-14 23:46 ` Damien Le Moal
2022-06-15 17:09 ` Sergei Shtylyov
0 siblings, 1 reply; 5+ messages in thread
From: Damien Le Moal @ 2022-06-14 23:46 UTC (permalink / raw)
To: Sergey Shtylyov, linux-ide
On 6/15/22 03:43, Sergey Shtylyov wrote:
> On 6/14/22 8:24 AM, Damien Le Moal wrote:
> [...]
>>> Make the 'timeout' parameter to ata_exec_internal_sg() *unsigned int* as
>>> msecs_to_jiffies() that it calls takes just *unsigned int* for the time in
>>> milliseconds. Then follow the suit with ata_exec_internal(), its only
>>> caller; also fix up ata_dev_set_feature(), the only ata_exec_internal()'s
>>> caller that explicitly passes *unsigned long* variable for timeout...
>>>
>>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>>> analysis tool.
>>
>> Since you are changing this function signature, can you also make it
>> static since it is only used in libata-core.c ? The declaration in
>> drivers/ata/libata.h is useless.
>
> Hopefully you don't mean I should do it in the same patch? :-)
The patch you sent is changing the function signature. So yes, doing it in
the same patch is fine with me. I do not see the need for 2 patches for
that. Instead of "fix sloppy parameter type in ata_exec_internal_sg",
rename the patch "fix ata_exec_internal_sg signature" and then all changes
naturally belong to the same patch.
>
> [...]
>
> MBR, Sergey
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ata: libata-core: [_sg]fix sloppy parameter type in ata_exec_internal()
2022-06-14 23:46 ` [PATCH] ata: libata-core: [_sg]fix sloppy parameter type in ata_exec_internal() Damien Le Moal
@ 2022-06-15 17:09 ` Sergei Shtylyov
0 siblings, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2022-06-15 17:09 UTC (permalink / raw)
To: Damien Le Moal, linux-ide; +Cc: Sergey Shtylyov
Hello!
Replying from my Gmail address, the corporate IMAP server is still AWOL... :-(
Not sure what happened to the subject: "[_sg]" somehow got moved before "fix"...
On 6/15/22 2:46 AM, Damien Le Moal wrote:
[...]
>>>> Make the 'timeout' parameter to ata_exec_internal_sg() *unsigned int* as
>>>> msecs_to_jiffies() that it calls takes just *unsigned int* for the time in
>>>> milliseconds. Then follow the suit with ata_exec_internal(), its only
>>>> caller; also fix up ata_dev_set_feature(), the only ata_exec_internal()'s
>>>> caller that explicitly passes *unsigned long* variable for timeout...
>>>>
>>>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>>>> analysis tool.
>>>
>>> Since you are changing this function signature, can you also make it
>>> static since it is only used in libata-core.c ? The declaration in
>>> drivers/ata/libata.h is useless.
>>
>> Hopefully you don't mean I should do it in the same patch? :-)
>
> The patch you sent is changing the function signature.
Actually, it changes 2 function signatures...
> So yes, doing it in the same patch is fine with me.
But not with me. Adding a "drove by" signature change doesn't go together
well with the "found by SVACE" note, to start with...
Moreover, I've created 2 patches already, with the 1st patch making
ata_exec_internal_sg() *static*...
> I do not see the need for 2 patches for that.
This patch as is already does enough to violate the "do one thing per
patch" rule, to add even more violations. :-)
> Instead of "fix sloppy parameter type in ata_exec_internal_sg",
> rename the patch "fix ata_exec_internal_sg signature" and then all changes
> naturally belong to the same patch.
Thanks, no -- I'd like to keep the emphasis on "sloppy parameter type".
MBR, Sergey
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-15 17:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 18:47 [PATCH] ata: libata-core: fix sloppy parameter type in ata_exec_internal[_sg]() Sergey Shtylyov
2022-06-14 5:24 ` Damien Le Moal
2022-06-14 18:43 ` Sergey Shtylyov
2022-06-14 23:46 ` [PATCH] ata: libata-core: [_sg]fix sloppy parameter type in ata_exec_internal() Damien Le Moal
2022-06-15 17:09 ` Sergei Shtylyov
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.