Re: [PATCH v3 4/7] x86/time: introduce probing logic for the wallclock

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.09.2024 14:30, Roger Pau Monné wrote:
> On Wed, Sep 04, 2024 at 01:49:36PM +0200, Jan Beulich wrote:
>> On 04.09.2024 12:58, Roger Pau Monné wrote:
>>> I had it that way originally, but then it seemed the extra
>>> indentation made it less readable.  Will see how can I adjust it, my
>>> preference would be for:
>>>
>>>     panic("No usable wallclock found, probed:%s%s%s\n%s",
>>>           !cmos_rtc_probe && !efi_enabled(EFI_RS) ? " None" : "",
>>>           cmos_rtc_probe ? " CMOS" : "",
>>>           efi_enabled(EFI_RS) ? " EFI" : "",
>>>           !cmos_rtc_probe ? "Try with command line option 
>>> \"cmos-rtc-probe\"\n"
>>>                           : !efi_enabled(EFI_RS) ? "System must be booted 
>>> from EFI\n"
>>>                                                  : "");
>>>
>>> But that exceeds the 80 columns limit.
>>
>> Right, formally the above would be my preference, too. Here two shorter-
>> lines alternatives:
>>
>>     panic("No usable wallclock found, probed:%s%s%s\n%s",
>>           !cmos_rtc_probe && !efi_enabled(EFI_RS) ? " None" : "",
>>           cmos_rtc_probe ? " CMOS" : "",
>>           efi_enabled(EFI_RS) ? " EFI" : "",
>>           !cmos_rtc_probe
>>           ? "Try with command line option \"cmos-rtc-probe\"\n"
>>           : !efi_enabled(EFI_RS) ? "System must be booted from EFI\n"
>>                                  : "");
>>
>>     panic("No usable wallclock found, probed:%s%s%s\n%s",
>>           !cmos_rtc_probe && !efi_enabled(EFI_RS) ? " None" : "",
>>           cmos_rtc_probe ? " CMOS" : "",
>>           efi_enabled(EFI_RS) ? " EFI" : "",
>>           !cmos_rtc_probe
>>               ? "Try with command line option \"cmos-rtc-probe\"\n"
>>               : !efi_enabled(EFI_RS)
>>                   ? "System must be booted from EFI\n"
>>                   : "");
>>
>> Either of these or anything more or less similar will do imo, just as
>> long as the ? vs : alignment is there.
> 
> I think I prefer the second variant, as indentation is clearer there.
> 
>>
>> One thing I notice only now: The trailing %s will be a little odd if
>> the "" variant is used in the last argument. That'll produce "(XEN) "
>> with nothing following in the log. Which usually is a sign of some
>> strange breakage.
> 
> I've tested this and it doesn't produce an extra newline if the string
> parameter is "".  IOW:
> 
> printk("FOO\n%s", "");
> 
> Results in:
> 
> (XEN) [    2.230603] TSC deadline timer enabled
> (XEN) [    2.235654] FOO
> (XEN) [    2.238682] Wallclock source: EFI

Oh, my mistake. Format string processing of course comes before the
determination of line breaks within what is to be output.

Jan