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

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

On Sat, Sep 13, 2025 at 10:12:46AM +0200, Michal Pecio wrote:
> On Fri, 12 Sep 2025 21:02:23 +0300, Andy Shevchenko wrote:
> > 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.
> 
> If I equated correctness with ugliness I would equally confidently

Ugliness?! No, the full information printed is way too better than
whatever pieces of it are (even if it's correct in some cases).
We are doing 64-bit hardware, we have to deal with 64-bit pointers.
Period. Shortcutting and premature optimizing is always a slippery slope
when we enter to the debugging field.

> state that unnecessary zero-padding is incorrect. But I don't.

Right, because it won't be incorrect, there is no such thing as unnecessary
zero-padding for the pointers.

> This is exactly the absurd argument I previously made for padding all
> %x and %d formats to full width. Pad tables, not lone log messages.

> > 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.
> 
> At least if you do it, the compiler will also do the right thing:
> - if the cast doesn't match the format, warn (xhci needs a patch here)

Of course this doesn't work properly on the types that are less than int. So,
this is fragile argument to support explicit castings.

> - if it matches, widen the provided value as necessary
> 
> And it works consistently regardless of whether the variable is a
> dma_addr_t or u64, on all architectures, with or without PAE.

Yes, with "unnecessary" zero-paddings (that case when they are not needed).

> Reminder: this drivers handles DMAs as u64 too, so it will *never*
> print all DMAs as %pad. And if it tries, it will be a silent bug.

Yes, and the problem here is not in the printf() specifiers, the problem is
in the (used) data types. For the printf() it's crystal clear, if the type of
the variable is dma_addr_t, we have %pad. No need to reinvent the wheel.

> > > 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.
> 
> Indeed, not holding my breath for the %pad situation improving.
> This includes impossibility of passing it by value - it's forced
> by compilers being unaware of %pad and following usual %p rules.
> 
> > 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.
> 
> Surely people would laugh me off if I actually suggested that.
> If those formats work for their cases, use them.

Sure, there are thousands of the cases in the kernel when we print our custom
data types in a better format and this works and will continue working
independently on the legacy of the compilers or C standard modifications.

Removing them is like suggesting Americans to drop automatic gear in favour of
the manual to the promoting eco-transportation (horses + carts). I don't believe
this is valuable proposal.

P.S.
I'm sorry, but I lost the objective of this discussion. Can you summarize,
please, what's wrong with the patch?

-- 
With Best Regards,
Andy Shevchenko