On Thu, Aug 7, 2025 at 10:35 PM Jeremy Linton <jeremy.lin...@arm.com> wrote:
> On 8/6/25 10:25 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 aecc554e7ba8bd449dceaf3eeecacc09b9b77fc4
> > , but
> > doesn't reproduce on kexec-tools 6aecc32c6db895b1c0b1f522d82c8810ece65542 .
> >
> > (This is cover letter, so I think using scary SHA1 sums is okay here.)
> >
> > I. e. it is regression, introduced by
> > aecc554e7ba8bd449dceaf3eeecacc09b9b77fc4 .
> >
> > 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 (v2) 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-safinas...@zohomail.com/
>
> Thanks for fixing this, that patch was a last minute addition and I
> guess it shows.
>
> The patch content looks fine to me, but I would suggest that the commit
> messages are added/cleaned up a bit. Also you need the fixes tags for
> all the cases I broke since they are now in separate files. It might be
> easier to keep some of the related ones together and the additional
> fixes you made which aren't corrections together.
>
> Thanks again,
As Jeremy said, I have nothing to add.
Codewise and split looks good otherwise.
--
With Best Regards,
Andy Shevchenko
