Re: [PATCH v4 0/6] Fix printf string specifiers, otherwise kexec doesn't work on my laptop

[Jeremy Linton <jeremy.linton@arm.com>]

Hi,


First, thanks for fixing this!

So, I've been trying to find a failure beyond the printouts being wrong 
for the past couple days, and largely failed to duplicate a functional 
issue.

That said, this does fix a couple printouts I checked on 32-bit 686 
debian 12. Generally I think the code is ok as well and have reviewed it 
but i'm going to withhold the review by tag because I still think the 
commit messages could use some cleanup.


Tested-by: Jeremy Linton <jeremy.linton@arm.com>

Thanks again.




On 8/19/25 4:30 PM, Askar Safin wrote:
> TL;DR: this patchset fixes regression, introduced in aecc554e7b.
> This patchset should be backported to all distributions, which packaged v2.0.31, otherwise
> kexec doesn't work at all at my laptop with pretty common setup with v2.0.31.
> v2.0.31 is broken without this patchset.
> 
> See details of this bug on my laptop in first commit message.
> 
> Okay, why the bug happens? I suspect this is because of "%lux"
> string specifiers, which are totally wrong. The author meant "print (and scan) in hexademical"
> here, but this specifier prints (and scans) number in decimal, followed by literal "x". Oops.
> And this seems to break kexec.
> 
> The bug reproduces on kexec-tools aecc554e7ba , but
> doesn't reproduce on kexec-tools 6aecc32c6db .
> 
> I. e. it is regression, introduced by aecc554e7ba .
> 
> Okay, how to fix this? Well, this is not easy. In 07821da7cf and d2f4297166 Andy Shevchenko
> observed compilation warnings, when %lx is used with uint64_t, so he replaced %lx with %llx.
> 
> Then in aecc554e7b Jeremy Linton observed warnings with %llx and replaced it with %lux.
> (Yes, C is nightmare.)
> 
> So, uint64_t is sometimes defined as long unsigned, and thus needs %lx, and sometimes as
> long long unsigned and thus needs %llx.
> 
> How to fix this once and for all?
> 
> I see three ways.
> 
> 1. uint64_t a; printf ("%llx", (unsigned long long)a);
> 2. uint64_t a; printf ("%" PRIx64, a);
> 3. uint64_t a; printf ("%w64x", a);
> 
> I think that %w64x is beautiful, but it causes compilation warnings on clang. (Facepalm.)
> Also it was introduced in C23, which is too young.
> 
> "(unsigned long long)a" is the best in my opinion. This is what I used in v1.
> But Andy said to me that POD conversions are evil.
> 
> PRIx64 is ugly, but this is the only option left. So this is what I used in this version.
> 
> Also this patchset fixes other misc things. See commit messages for details.
> 
> I tested on my laptop that this patchset actually fixes the bug.
> 
> v1: https://lore.kernel.org/kexec/20250805124722.11193-1-safinaskar@zohomail.com/
> v2: https://lore.kernel.org/kexec/20250807032510.4211-1-safinaskar@zohomail.com/
> v3: https://lore.kernel.org/kexec/20250819090505.647690-1-safinaskar@zohomail.com/
> 
> Changes since v1:
> * Addressed Andy's comments
> * I reproduced the bug (and tested the fix) on slightly different versions of Linux
>    and linux-firmware (see first commit message)
> 
> Changes since v2:
> * Commit messages only
> 
> Changes since v3:
> * Commit messages only
> 
> Askar Safin (6):
>    Fix printf string specifiers, otherwise kexec doesn't work on my
>      laptop
>    kexec/kexec-elf-exec.c: Replace %lux with %lx
>    kexec/arch/i386/x86-linux-setup.c: replace %d with %u
>    util_lib/elf_info.c: fix typo: prink -> printk
>    kexec/arch/i386/kexec-x86-common.c: remove duplicate <stdio.h>
>    kexec/arch/arm64/crashdump-arm64.c: remove extra whitespace
> 
>   kexec/arch/arm64/crashdump-arm64.c | 2 +-
>   kexec/arch/i386/crashdump-x86.c    | 3 ++-
>   kexec/arch/i386/kexec-x86-common.c | 4 ++--
>   kexec/arch/i386/x86-linux-setup.c  | 3 ++-
>   kexec/kexec-elf-exec.c             | 2 +-
>   util_lib/elf_info.c                | 9 +++++----
>   6 files changed, 13 insertions(+), 10 deletions(-)
>