Re: [PATCH] xen/arm: Initialise memory type for struct kernel_info

[Michal Orzel <michal.orzel@xxxxxxx>]

On 23/12/2024 11:45, Julien Grall wrote:
> 
> 
> Hi,
> 
> On 23/12/2024 10:42, Michal Orzel wrote:
>>
>>
>> On 23/12/2024 11:06, Julien Grall wrote:
>>>
>>>
>>> Hi Michal,
>>>
>>> On 23/12/2024 07:58, Michal Orzel wrote:
>>>>
>>>>
>>>> On 20/12/2024 16:19, Luca Fancellu wrote:
>>>>>
>>>>>
>>>>> Commit a14593e3995a ("xen/device-tree: Allow region overlapping with
>>>>> /memreserve/ ranges") introduced a type in the 'struct membanks_hdr'
>>>>> but forgot to update the 'struct kernel_info' initialiser, while
>>>>> it doesn't lead to failures because the field is not currently
>>>>> used while managing kernel_info structures, it's good to have it
>>>>> for completeness.
>>>> The last part "good to have it" does not sound like we need a Fixes tag.
>>>
>>> Reading the discussion, it sounds like ".type" is not always set and
>>> this is not properly documented. This means that in the future we may
>>> write a patch that requires to use ".type" and needs backported.
>>>
>>> But this would depend on this patch which was not tagged appropriately.
>>> Therefore, I would argue it needs a fixes tag (even though this is a
>>> latent bug) or at least a backport request.
>> Setting explicitly a type for structures not requiring it, does not seem 
>> beneficial for me.
> 
> I have to disagree. If we set type everywhere, then the developer
> doesn't need to think whether ".type" is garbagge or not.
I see, I have nothing against this proposal. But in that case I do think that 
the same needs to apply to max_banks
member.

~Michal