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
