Re: [PATCH v4 19/28] pc-bios/s390-ccw: Refactor zipl_load_segment function

[Collin Walling <walling@linux.ibm.com>]

[...]

>>> @@ -662,6 +661,9 @@ static int zipl_load_segment(ComponentEntry *entry)
>>>                   */
>>>                  break;
>>>              }
>>> +
>>> +            comp_len += bprs->size * (bprs[i].blockct + 1);
>>> +
>>
>> I'm confused by the arithmetic here.  Why is size multiplied by the
>> block count?  Won't that artificially inflate the value representing the
>> size of the component?  What's the reason that comp_len += bprs->size
>> isn't sufficient?
>>
> 
> A component table entry points to a segment table, which holds pointers
> to code segments loaded into contiguous memory.
> 
> Since segments can vary in length, the block count field may be nonzero.
> 
> Block size indicates the number of bytes in a single logical block, so
> the total component size is the block size multiplied by the block count.
> 

Okay.  After referencing both your explanation and our disk layout docs,
this makes more sense to me.  Thanks!

More nitpicking: I think it reads better to put this line of code as the
last line of the for loop.  That will keep all disk-reading code
together and make it a little more clear that this variable isn't used
elsewhere in the inner loop.

Functionally, I think the patch is fine :)

[...]

-- 
Regards,
  Collin