Re: [PATCH v1 05/18] x86: refactor xen cmdline into general framework

["Daniel P. Smith" <dpsmith@apertussolutions.com>]

On 7/19/22 09:26, Jan Beulich wrote:
> On 06.07.2022 23:04, Daniel P. Smith wrote:
>> --- a/xen/include/xen/bootinfo.h
>> +++ b/xen/include/xen/bootinfo.h
>> @@ -53,6 +53,17 @@ struct __packed boot_info {
>>  
>>  extern struct boot_info *boot_info;
>>  
>> +static inline char *bootinfo_prepare_cmdline(struct boot_info *bi)
>> +{
>> +    bi->cmdline = arch_bootinfo_prepare_cmdline(bi->cmdline, bi->arch);
>> +
>> +    if ( *bi->cmdline == ' ' )
>> +        printk(XENLOG_WARNING "%s: leading whitespace left on cmdline\n",
>> +               __func__);
> 
> Just a remark and a question on this one: I don't view the use of
> __func__ here (and in fact in many other cases as well) as very
> useful. And why do we need such a warning all of the sudden in the
> first place?

This started as just a debug print, thus why __func__ is in place, but
later decided to leave it. This is because after this point, the code
assumes that all leading space was stripped, but there was never a check
that logic did the job correct. I don't believe a failure to do so
warranted a halt to the boot process, but at least provide a warning
into the log should the trimming fail. Doing so would allow an admin to
have a clue should an unexpected behavior occur as a result of leading
space making it through and breaking the fully trimmed assumption made
elsewhere.

v/r,
dps