Re: [PATCH v2 03/35] scsi: Add struct for args to execution functions

Bart Van Assche
2022-09-30T17:30:22Z

On 9/29/22 19:43, Mike Christie wrote:
> On 9/29/22 7:12 PM, Bart Van Assche wrote:
>> On 9/28/22 19:53, Mike Christie wrote:
>>> +int __scsi_exec_req(struct scsi_exec_args *args)
>>
>> Has it been considered to change the argument list into "const struct
>> scsi_exec_args *args"?
>
> Yeah I meant to ask you about this. We do end up updating resid, sense
> buf, and sshdr, but because those are pointers I can make it
> "const struct scsi_exec_args *args" and it's fine since we are not
> updating fields like buf_len.
>
> I was thinking you wanted fields like cmd const though. So do you want
>
> 1. "const struct scsi_exec_args *args"
>
> plus
>
> 2. pointers on that struct that we don't modify like cmd and sdev also
> const.
>
> ?
Hi Mike,

I care more about (1) than about (2). The following code:

__scsi_exec_req(&((struct scsi_exec_args){...}));

creates a temporary struct on the stack and passes the address of that
temporary to __scsi_exec_req(). Changing the argument type of
__scsi_exec_req() from struct scsi_exec_args *args into const struct
scsi_exec_args *args ensures that __scsi_exec_req() cannot modify *args
if 'args' points at a temporary data structure on the stack.

Thanks,

Bart.