On 10/9/24 11:53, Jan Beulich wrote:
> On 06.10.2024 23:49, Daniel P. Smith wrote:
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -54,8 +54,24 @@ struct boot_info {
>> struct boot_module mods[MAX_NR_BOOTMODS + 1];
>> };
>>
>> -#endif /* __XEN_X86_BOOTINFO_H__ */
>> +static inline int __init next_boot_module_index(
>> + const struct boot_info *bi, enum bootmod_type t, int offset)
>
> Instead of "offset" maybe better "start" or "from"? Further, plain int
> (as also used ...
Will change to start.
>> +{
>> + int i;
>
> ... here) isn't really liked for ...
>
>> + for ( i = offset; i < bi->nr_modules; i++ )
>> + {
>> + if ( bi->mods[i].type == t )
>
> ... array indexing. Perhaps the function itself would better have
> unsigned int return type as well, ...
>
>> + return i;
>> + }
>> +
>> + return -1;
>
> ... using UINT_MAX or some other suitable constant here instead?
I was initially going to disagree as returning a value less than zero is
much more natural/reasonable than UINIT_MAX. But then thinking about it,
another natural value to reflect an error/not found is a value larger
than MAX_NR_BOOTMODS.
Will switch to unsigned and add code comment that larger than
MAX_NR_BOOTMODS is error/not found condition.
v/r,
dps