Linux PCI subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Bjorn Helgaas" <helgaas@kernel.org>
Cc: linux-pci@vger.kernel.org
Subject: Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size
Date: Sat, 08 Aug 2026 17:38:32 +0000	[thread overview]
Message-ID: <20260808173833.5A1D01F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260808172308.282591-2-bhelgaas@google.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a KUnit test regression by unconditionally appending a human-readable resource size for memory ranges, affecting the raw '%pr' format as well as the decoded '%pR' format. The commit message states it adds the size to '%pR', but the code incorrectly modifies both formats, breaking the printf test suite.
--

--- Patch [1]: [PATCH v1 1/2] vsprintf: Add %pR human-readable size ---
commit d77ec66543a5589a88168c27ffcdb3693182c105
Author: Bjorn Helgaas <helgaas@kernel.org>

vsprintf: Add %pR human-readable size

Include human-readable size when printing struct resource memory ranges
with the %pR format to make it easier to read.

> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 2bc6ef483576c..c49044b6dbeeb 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
[ ... ]
> @@ -1134,6 +1136,14 @@ char *resource_string(char *buf, char *end, struct resource *res,
>  	} else {
>  		p = hex_range(p, pend, res->start, res->end, *specp);
>  	}
> +	if (res->flags & IORESOURCE_MEM) {
> +		*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);
> +		*p++ = ')';
> +	}

[Severity: Medium]
Does this code inadvertently modify the raw '%pr' format as well?

The commit message states it adds the size to '%pR', but this check doesn't
verify the 'decode' boolean before appending the human-readable size.

This unconditionally appends the human-readable resource size for memory ranges,
which affects the raw '%pr' format and breaks KUnit tests expecting the
unmodified output:

lib/tests/printf_kunit.c:struct_resource() {
    ...
    test("[mem 0xc0ffee00 flags 0x200]",
         "%pr", &test_resource);
    ...
}

>  	if (decode) {
>  		if (res->flags & IORESOURCE_MEM_64)
>  			p = string_nocheck(p, pend, " 64bit", str_spec);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808172308.282591-1-bhelgaas@google.com?part=1

  reply	other threads:[~2026-08-08 17:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-08 17:23 [PATCH v1 0/2] PCI: Include human-readable BAR and window sizes Bjorn Helgaas
2026-08-08 17:23 ` [PATCH v1 1/2] vsprintf: Add %pR human-readable size Bjorn Helgaas
2026-08-08 17:38   ` sashiko-bot [this message]
2026-08-08 17:43   ` Andy Shevchenko
2026-08-08 23:45     ` Bjorn Helgaas
2026-08-09  8:25       ` Andy Shevchenko
2026-08-08 17:23 ` [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages Bjorn Helgaas
2026-08-08 17:27   ` sashiko-bot
2026-08-08 17:48   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260808173833.5A1D01F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=helgaas@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox