Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size

On Sat, Aug 08, 2026 at 06:45:30PM -0500, Bjorn Helgaas wrote:
> On Sat, Aug 08, 2026 at 08:43:30PM +0300, Andy Shevchenko wrote:
> > On Sat, Aug 08, 2026 at 12:23:07PM -0500, Bjorn Helgaas wrote:

...

> > > +	if (res->flags & IORESOURCE_MEM) {
> > 
> > Besides we have resource_type() macro, why only this? I would expect
> > IO as well as 64-bit MEM version.
> 
> We could use resource_type() here, but would have to check other bits
> directly (IORESOURCE_PREFETCH, IORESOURCE_MEM_64, etc) since they're
> not included.

Sure.

> I considered including IO and certainly could do that.  IO resources
> are smaller and it's not as interesting, but maybe we should.
> 
> 64-bit MEM resources are already included because they have both
> IORESOURCE_MEM and IORESOURCE_MEM_64 set.

Is it guaranteed to have MEM when MEM_64 is set?

...

> > > +		*p++ = ' ';
> > > +		*p++ = '(';
> > > +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> > > +				size_buf, sizeof(size_buf));
> > 
> > > +		p = string_nocheck(p, pend, size_buf, str_spec);
> > 
> > Why do you need this? Use returned value from string_get_size() directly.
> 
> string_get_size() doesn't return a char * like string_nocheck() et al
> do.  We could do something like "p += string_get_size(..., p, pend - p)",
> but it's a little messy

See the use of string_escape_mem() in the same file.

> because the return value is the length that
> *would* have been written if it was truncated, so I think we'd have to
> check for "p > pend".  I'll ponder this one.

Yes, and that's exactly what we want from the vsnprintf() as a returned value.
What I mean is to use the given buffer directly without any intermediate one.
The *p++ = 'xxx' will become an idiomatic

	if (end > ...)
		*buf = 'xxx';
	++buf;

which vsprintf.c is full of.

> > > +		*p++ = ')';
> > > +	}

-- 
With Best Regards,
Andy Shevchenko