Re: [PATCH 2/9] Fix size capping in write_pieced_value

[Yao Qi <qiyaoltc at gmail dot com>]

Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

>> Hi Andreas,
>> The description to the logic can go to comments, so that we don't need
>> to do "git blame/log" to understand the code.
>
> Right, I'll add some general explanation and a diagram about the various
> bits and offsets (as requested below).
>
> However, most of the commit message explains a specific bug in a piece
> of code that won't exist any more.  This aspect doesn't make sense to be
> included in the comments, I think.
>

OK, no problem.

>>>>> logic in write_pieced_value for handling this is flawed when there are
>>>>> actually bits to skip at the beginning of the first piece: it truncates
>>>>> the piece size towards the end *before* accounting for the skipped bits
>>>>> at the beginning instead of the other way around.
>>>>>
>>>>> Note that the same bug was already found in read_pieced_value and fixed
>>>>> there (but not in write_pieced_value), see PR 15391.
>>>>
>>>> Can we share the code in write_pieced_value and read_pieced_value?  The
>>>> code computing offsets and bits should be shared.
>>>
>>> Yes.  I have another patch (not posted yet) that merges these two
>>> functions.  I moved that towards the end of the patch series, so the
>>> individual fixes can be incremental.
>>>
>>
>> I'd like to merge the code first, then don't need to fix the same
>> problem in two functions read_pieced_value and write_pieced_value (your
>> patch 4/9 ~ 9/9 touches both two functions).
>
> Not sure I understand.  Do you mean to merge the functions first while
> preserving existing logic, including all the bugs and differences?  I

What I meant is that 1) make two parts identical (but not introducing
new bugs) 2) merge the code to a single function, 3) fix the rest of
bugs in the single function,

> had started along this path and gave up on it, because I found it too
> complicated.  From that attempt I've concluded that the current approach
> is much less error-prone and easier to follow.

There may be some complexity I didn't realize.  I don't want to make
your life harder.  If you are comfortable on this approach, fine by me.

-- 
Yao (齐尧)