Re: [PATCH 2/3] PCI: iov: Update format string type to match variable type

From: Bjorn Helgaas <helgaas@kernel.org>
To: "Krzysztof Wilczyński" <kw@linux.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Boqun Feng <boqun.feng@gmail.com>, Long Li <longli@microsoft.com>,
	"K. Y. Srinivasan" <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Stephen Hemminger <sthemmin@microsoft.com>,
	Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
	linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH 2/3] PCI: iov: Update format string type to match variable type
Date: Mon, 11 Oct 2021 16:13:56 -0500	[thread overview]
Message-ID: <20211011211356.GA1692323@bhelgaas> (raw)
In-Reply-To: <20211008222732.2868493-2-kw@linux.com>

On Fri, Oct 08, 2021 at 10:27:31PM +0000, Krzysztof Wilczyński wrote:
> Functions pci_iov_sysfs_link() and pci_iov_remove_virtfn() take
> a Virtual Function (VF) ID as an integer value and then use it to
> assemble the desired name for the corresponding sysfs attribute (a
> symbolic link in this case).

It's not really clear to me that "int" is the correct type for the VF
ID.  pci_iov_add_virtfn() is declared to take an int, but
sriov_add_vfs() passes as unsigned int, which I think probably makes
more sense unless there's some interface that may return either a VF
ID or an error.

NumVFs in the SR-IOV Capability is only 16 bits wide, so I guess
either mostly works...

> Internally, both functions use sprintf() to create the desired attribute
> name, and leverage the "%u" modifier as part of the format string used
> to do so.  However, the VF ID is passed to both functions as a signed
> integer type variable, which makes the variable type and format string
> modifier somewhat incompatible.
> 
> Thus, change the modifier used in the format string to "%d" to better
> match the variable type.
> 
> No change to functionality intended.
> 
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> ---
>  drivers/pci/iov.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index dafdc652fcd0..056bba3b4236 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -140,7 +140,7 @@ int pci_iov_sysfs_link(struct pci_dev *dev,
>  	char buf[VIRTFN_ID_LEN];
>  	int rc;
>  
> -	sprintf(buf, "virtfn%u", id);
> +	sprintf(buf, "virtfn%d", id);
>  	rc = sysfs_create_link(&dev->dev.kobj, &virtfn->dev.kobj, buf);
>  	if (rc)
>  		goto failed;
> @@ -322,7 +322,7 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id)
>  	if (!virtfn)
>  		return;
>  
> -	sprintf(buf, "virtfn%u", id);
> +	sprintf(buf, "virtfn%d", id);
>  	sysfs_remove_link(&dev->dev.kobj, buf);
>  	/*
>  	 * pci_stop_dev() could have been called for this virtfn already,
> -- 
> 2.33.0
> 