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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.07.2022 15:12, Daniel P. Smith wrote:
> 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.

All fine, but then such a change wants doing on its own, not in the middle
of pretty involved refactoring work.

Jan