Re: [PATCH v6 08/25] spl: Refactor spl_load_info->read to use units of bytes

[Sean Anderson <seanga2@gmail.com>]

On 11/7/23 03:49, Xavier Drudis Ferran wrote:
> El Mon, Nov 06, 2023 at 08:54:03AM -0500, Sean Anderson deia:
>> On 11/6/23 07:35, Xavier Drudis Ferran wrote:
>>> Thanks for your work. I'm still reading... but...
>>>
>>>
>>> El Sun, Nov 05, 2023 at 09:25:46PM -0500, Sean Anderson deia:
>>>> diff --git a/include/spl.h b/include/spl.h
>>>> index 951e136b9ea..ecfc50e0095 100644
>>>> --- a/include/spl.h
>>>> +++ b/include/spl.h
>>>> @@ -297,10 +297,10 @@ struct spl_load_info {
>>>>    	 * read() - Read from device
>>>>    	 *
>>>>    	 * @load: Information about the load state
>>>> -	 * @sector: Sector number to read from (each @load->bl_len bytes)
>>>> -	 * @count: Number of sectors to read
>>>> +	 * @offset: Offset to read from in bytes, in multiples of @load->bl_len
>>>> +	 * @count: Number of bytes to read, in multiples of @load->bl_len
>>>
>>> I'm no native English speaker, but would it be easier to understand? :
>>>
>>> +	 * @offset: Offset to read from in bytes, a multiple of @load->bl_len
>>> +	 * @count: Number of bytes to read, a multiple of @load->bl_len
>>>
>>>
>>>
>>
>> I think it would have to be worded
>>
>> @offset: Offset to read from in bytes, as a multiple of @load->bl_len
>>
>> but to me these both mean the same thing.
>>
>> --Sean
> 
> Ah, OK. I doubted on whether it should be a comma or "as". Apparently
> it was both.
> 
> They may mean the same, but I got confused by having "in bytes", and
> "in multiples" so close.
> 
> "in bytes" means the value should be the correct answer to the
> question "how many bytes" (not a number of bits, not a number of
> sectors)
> 
> but "in multiples of..." doesn't mean the value should be the correct
> answer to "how many of multiples". Should the value be a number of
> sectors (first multiple of bl_len) a number of dozens of sectors
> (another multiple of bl_len) or ...?
> 
> In the first part "in" introduces a unit of measure, but in the second
> the same word introduces a constraint on a value. That was confusing
> to me.
> 
> Not saying it doesn't mean the same or even that you should change it,
> just elaborating why I said it.
> 

OK, what about

@offset: Offset to read from in bytes. This must be a multiple of @load->bl_len.

--Sean