Re: [PATCH 2/7] usb: xhci: use '%pad' specifier for DMA address printing

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, Sep 12, 2025 at 11:46:44AM +0200, Michal Pecio wrote:
> On Thu, 11 Sep 2025 23:13:54 +0300, Andy Shevchenko wrote:
> > On Thu, Sep 11, 2025 at 11:34:51AM +0200, Michal Pecio wrote:
> > > As for the %08llx format widespread in dynamic debug, I think it was
> > > used in the past because it does approximately the right thing on both
> > > types of systems and it's the only format capable of giving consistent
> > > result on both dma_addr_t and u64, used for some DMA pointers too.  
> > 
> > The problem with it is that it can't give the proper result for the ranges that
> > span over the 4G. Which I consider a bad thing. So, the correct use is to stick
> > with HW register size and do appropriate specifier as it was a pointer.
> 
> I see no reason to bother padding pointers to full variable width and
> when I run 'dmesg' on my 64 bit machine I see that most of the kernel
> doesn't really bother either, so xhci isn't any outlier.

Can you point out to some examples? I see that pointers printed in many cases
normally as 64-bit values.

> (Plus: why should we stop at pointers? Integers too have a width.)

This is not an argument at all. Pointers are special, they are not _just_
integers. Coincidentally I read an article about pointer in C
https://stefansf.de/post/pointers-are-more-abstract-than-you-might-expect/

> It amounts to embedding static type information in logs. Maybe there
> are cases where it could be helpful for people reading the log, maybe
> there aren't, but this patch doesn't even attempt to make such case,
> it just talks vaguely about "correctness".

The correctness here is in information that is printed. Again, imagine
the loop that goes above 4G on a 64-bit machine. Out of a sudden %08llx will be
expanded to cover the 64-bit addresses and becomes one digit at a time creating
a ladder (ugly looking) output. This is incorrect.

> I only see one truly incorrect case fixed here, a missing (u64) cast
> for %llx format, which I presume will print garbage on 32 bits.

No, it's other way around, we should not put explicit casts in printf() in C
as there are plenty of the format specifiers that allows us to be sure that
the printed value is correct independently on architecture, endianess, etc.

> This brings up another problem with %pad: it is unknown to compilers
> so they don't type check it. 
> 
> but if I switch to %pad and later change my mind and extend 'addr' to
> u64 without updating this format, the compiler will eat it up and once
> again garbage will be printed on some systems.

This topic was risen a few times in the past. Somebody proposed to have a GCC
plugin with that, somebody else proposed to completely rewrite the %p
extensions to be more like Pyhon or C++ ones (when you just specify argument
and handler for it). None so far is implemented AFAIK. But this is not
particular problem of %pad, it's for all %p extensions. And the extensions
exist for a purpose. What you are proposing here behind the lines is to kill
that completely. I believe this is not the right direction to go. So, TL;DR:
one should be careful about %p extensions, but at the same time open coding
them is not a good idea either.

-- 
With Best Regards,
Andy Shevchenko