[PATCH 1/3] PCI: hv: Remove unnecessary integer promotion from dev_err()

[PATCH 1/3] PCI: hv: Remove unnecessary integer promotion from dev_err()

[Krzysztof Wilczyński]

Internally, printk() et al already correctly handles the standard
integer promotions, so there is no need to explicitly "%h" modifier as
part of a format string such as "%hx".

Thus, drop the "%h" modifier as it's completely unnecessary (N.B. this
wouldn't be true for the sscanf() function family), and match preferred
coding style.

Related:
  commit cbacb5ab0aa0 ("docs: printk-formats: Stop encouraging use of unnecessary
  commit 70eb2275ff8e ("checkpatch: add warning for unnecessary use of %h[xudi] and %hh[xudi]")
  https://lore.kernel.org/lkml/CAHk-=wgoxnmsj8GEVFJSvTwdnWm8wVJthefNk2n6+4TC=20e0Q@mail.gmail.com/

No change to functionality intended.

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/controller/pci-hyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 67c46e52c0dc..6733cb14e775 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -3126,14 +3126,14 @@ static int hv_pci_probe(struct hv_device *hdev,
 
 	if (dom == HVPCI_DOM_INVALID) {
 		dev_err(&hdev->device,
-			"Unable to use dom# 0x%hx or other numbers", dom_req);
+			"Unable to use dom# 0x%x or other numbers", dom_req);
 		ret = -EINVAL;
 		goto free_bus;
 	}
 
 	if (dom != dom_req)
 		dev_info(&hdev->device,
-			 "PCI dom# 0x%hx has collision, using 0x%hx",
+			 "PCI dom# 0x%x has collision, using 0x%x",
 			 dom_req, dom);
 
 	hbus->bridge->domain_nr = dom;
-- 
2.33.0

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

[Krzysztof Wilczyński]

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).

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

[PATCH 3/3] PCI: Update variable type to match sscanf() format string

[Krzysztof Wilczyński]

To test whether a string contains a valid PCI device path formatted as
the "/<device>.<function>" address (see output from "lspci -P" as an
example; such format can also be provided as part of kernel command-line
parameters), the function called pci_dev_str_match_path() can be used.

Internally, pci_dev_str_match_path() function uses sscanf() and the "%x"
format string as part of its path matching implementation where it would
parse a given value as a unsigned hexadecimal number.  This particular
format string type requires the argument to be of an unsigned int type.

Thus, to match given format string requirements and also safeguard
against a potential undefined behaviour, change type of the variables
passed to sscanf() to unsigned int accordingly.

No change to functionality intended.

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ce2ab62b64cf..7998b65e9ae5 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -269,7 +269,7 @@ static int pci_dev_str_match_path(struct pci_dev *dev, const char *path,
 				  const char **endptr)
 {
 	int ret;
-	int seg, bus, slot, func;
+	unsigned int seg, bus, slot, func;
 	char *wpath, *p;
 	char end;
 
-- 
2.33.0

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

[Bjorn Helgaas]

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
> 

Re: [PATCH 1/3] PCI: hv: Remove unnecessary integer promotion from dev_err()

[Bjorn Helgaas]

On Fri, Oct 08, 2021 at 10:27:30PM +0000, Krzysztof Wilczyński wrote:
> Internally, printk() et al already correctly handles the standard
> integer promotions, so there is no need to explicitly "%h" modifier as
> part of a format string such as "%hx".
> 
> Thus, drop the "%h" modifier as it's completely unnecessary (N.B. this
> wouldn't be true for the sscanf() function family), and match preferred
> coding style.
> 
> Related:
>   commit cbacb5ab0aa0 ("docs: printk-formats: Stop encouraging use of unnecessary
>   commit 70eb2275ff8e ("checkpatch: add warning for unnecessary use of %h[xudi] and %hh[xudi]")
>   https://lore.kernel.org/lkml/CAHk-=wgoxnmsj8GEVFJSvTwdnWm8wVJthefNk2n6+4TC=20e0Q@mail.gmail.com/
> 
> No change to functionality intended.

Applied 1/3 and 3/3 to pci/misc for v5.16, thanks!

For 2/3, I think we might want to convert the VF ID to be unsigned
consistently.

> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 67c46e52c0dc..6733cb14e775 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -3126,14 +3126,14 @@ static int hv_pci_probe(struct hv_device *hdev,
>  
>  	if (dom == HVPCI_DOM_INVALID) {
>  		dev_err(&hdev->device,
> -			"Unable to use dom# 0x%hx or other numbers", dom_req);
> +			"Unable to use dom# 0x%x or other numbers", dom_req);
>  		ret = -EINVAL;
>  		goto free_bus;
>  	}
>  
>  	if (dom != dom_req)
>  		dev_info(&hdev->device,
> -			 "PCI dom# 0x%hx has collision, using 0x%hx",
> +			 "PCI dom# 0x%x has collision, using 0x%x",
>  			 dom_req, dom);
>  
>  	hbus->bridge->domain_nr = dom;
> -- 
> 2.33.0
> 

Re: [PATCH 1/3] PCI: hv: Remove unnecessary integer promotion from dev_err()

[Krzysztof Wilczyński]

Hi Bjorn,

> > Internally, printk() et al already correctly handles the standard
> > integer promotions, so there is no need to explicitly "%h" modifier as
> > part of a format string such as "%hx".
> > 
> > Thus, drop the "%h" modifier as it's completely unnecessary (N.B. this
> > wouldn't be true for the sscanf() function family), and match preferred
> > coding style.
> > 
> > Related:
> >   commit cbacb5ab0aa0 ("docs: printk-formats: Stop encouraging use of unnecessary
> >   commit 70eb2275ff8e ("checkpatch: add warning for unnecessary use of %h[xudi] and %hh[xudi]")
> >   https://lore.kernel.org/lkml/CAHk-=wgoxnmsj8GEVFJSvTwdnWm8wVJthefNk2n6+4TC=20e0Q@mail.gmail.com/
> > 
> > No change to functionality intended.
> 
> Applied 1/3 and 3/3 to pci/misc for v5.16, thanks!
> 
> For 2/3, I think we might want to convert the VF ID to be unsigned
> consistently.

Will do!  I am going to send a separate patch for it.

	Krzysztof