Re: [Xen-devel] printf formatter for pci_sbdf_t

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

On Thu, Jul 18, 2019 at 11:51:57AM +0000, Jan Beulich wrote:
> On 17.07.2019 19:06, Andrew Cooper wrote:
> > On 17/07/2019 15:08, Roger Pau Monné wrote:
> >> As part of some PCI related cleanup I'm doing, which includes
> >> expanding the usage of pci_sbdf_t, I'm also planning to add a custom
> >> printf formatter for pci_sbdf_t [0], so that a SBDF can be printed
> >> without having to specify four formatters (and pass four parameters)
> >> each time (%04x:%02x:%02x.%u).
> >>
> >> There's been some debate on the previous version about whether the
> >> formatter should be %pp or %op, and I would like to settle on one of
> >> them before sending a new version:
> > 
> > I am firmly opposed to overloading %o.
> 
> And I am firmly of the opinion that using %o for SBDF is the more
> natural thing to do.
> 
> > Nothing about PCI coordinates has anything to do with octal
> > representation; its mostly hex.
> 
> The domain and vCPU IDs aren#t pointer-ish either, for example.

But we do pass the vcpu and the domain struct pointer to those
modifiers? ie: we don't pass a vcpu or a domain ID directly in either
case.

> > And for the record, I'm firmly opposed to overloading %[xuid] as well.
> 
> I agree that we don#t want to overload any of these.
> 
> > %p is the only formatter which has magic overloading so far, and
> > avoiding gaining a second would be of substantial value when it comes to
> > reading the code.
> 
> I don't buy this argument. Readability of some of the printk()
> invocations in Roger's patch was (severely imo) hampered by the need
> to take addresses of things that could be easily passed by value.
> Generated code (size) should be taken into consideration here too,
> as should be the (slightly) larger stack consumption when going the
> %pp route.

I personally don't think passing the address instead of the value (ie:
adding an extra &) matters that much. If we pass by value then we
would have to explicitly pass the sbdf field of the struct I guess?
Which again seems to differ from what we do for vcpus and domains,
where a pointer to the struct is passed, regardless of whether the
actually formatter only cares about the ID.

AFAICT both you an Andrew agree that a custom printf formatter for PCI
SBDF is a desirable thing to have, the only disagreement is on the
actual implementation detail (whether %pp or %op should be used). In
which case I think I would like to call for THE REST to also voice in
their opinion on the matter in order to try to resolve the situation
so that I can make progress on the series, sorry if this is awkward.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel