On 12/12/22 10:17 AM, John Garry wrote:
> On 09/12/2022 18:47, Bart Van Assche wrote:
>>>> around it then we have to do a WARN/BUG. We do the macro approach now
>>>> so we can do the BUILD_BUG_ON.
>>>
>>> Maybe we have to switch to a WARN/BUG.
>>>
>>> It looks like some compilers don't like:
>>>
>>> const struct scsi_exec_args exec_args = {
>>> .sshdr = &sshdr,
>>> };
>>>
>>> scsi_execute_args(.... exec_args);
>>>
>>> and will hit the:
>>>
>>> #define scsi_execute_args(sdev, cmd, opf, buffer, bufflen, timeout, \
>>> retries, args) \
>>> ({ \
>>> BUILD_BUG_ON(args.sense && \
>>> args.sense_len != SCSI_SENSE_BUFFERSIZE); \
>>>
>>> because the args's sense and sense_len are not cleared yet.
>>
>> My understanding is that the __scsi_execute() macro was introduced to prevent that every single scsi_execute() caller would have to be modified. I'm fine with removing the BUILD_BUG_ON(sense_len != SCSI_SENSE_BUFFER_SIZE) check and replacing it with a WARN_ON_ONCE() statement, e.g. inside __scsi_execute().
>
> Another option is to have __scsi_execute() allocate the sense buf by kmemdup, and hold the sense pointer as unsigned char ** in struct scsi_exec_args; but then the caller needs to kfree the allocated sense buf, which I suppose is less than ideal. However there is only single driver which uses this AFAICS.
I did the WARN_ON_ONCE.
