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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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

Thanks, Roger.