[v1] iommu/amd: use str_plural() to simplify the code

[PATCH] iommu/amd: use str_plural() to simplify the code

Posted by Xichao Zhao 1 month, 2 weeks ago

Use the string choice helper function str_plural() to simplify the code.

Signed-off-by: Xichao Zhao <zhao.xichao@vivo.com>
---
 drivers/iommu/amd/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index eb348c63a8d0..b5c829f89544 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -265,7 +265,7 @@ static inline int get_acpihid_device_id(struct device *dev,
 		return -EINVAL;
 	if (fw_bug)
 		dev_err_once(dev, FW_BUG "No ACPI device matched UID, but %d device%s matched HID.\n",
-			     hid_count, hid_count > 1 ? "s" : "");
+			     hid_count,  str_plural(hid_count));
 	if (hid_count > 1)
 		return -EINVAL;
 	if (entry)
-- 
2.34.1

Re: [PATCH] iommu/amd: use str_plural() to simplify the code

Posted by David Laight 1 month, 2 weeks ago

On Mon, 18 Aug 2025 11:53:31 +0800
Xichao Zhao <zhao.xichao@vivo.com> wrote:

> Use the string choice helper function str_plural() to simplify the code.

This also changes (maybe fixes) the code my adding "s" for zero.

Although I'd define it as ("s" + !!count) - almost certainly generates
better code.

	David

> 
> Signed-off-by: Xichao Zhao <zhao.xichao@vivo.com>
> ---
>  drivers/iommu/amd/iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index eb348c63a8d0..b5c829f89544 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -265,7 +265,7 @@ static inline int get_acpihid_device_id(struct device *dev,
>  		return -EINVAL;
>  	if (fw_bug)
>  		dev_err_once(dev, FW_BUG "No ACPI device matched UID, but %d device%s matched HID.\n",
> -			     hid_count, hid_count > 1 ? "s" : "");
> +			     hid_count,  str_plural(hid_count));
>  	if (hid_count > 1)
>  		return -EINVAL;
>  	if (entry)

Re: [PATCH] iommu/amd: use str_plural() to simplify the code

Posted by Ankit Soni 1 month, 2 weeks ago

On Wed, Aug 20, 2025 at 07:03:59PM +0100, David Laight wrote:
> On Mon, 18 Aug 2025 11:53:31 +0800
> Xichao Zhao <zhao.xichao@vivo.com> wrote:
> 
> > Use the string choice helper function str_plural() to simplify the code.
> 
> This also changes (maybe fixes) the code my adding "s" for zero.
> 
> Although I'd define it as ("s" + !!count) - almost certainly generates
> better code.
> 
> 	David
> 

Hi,
In case of hid_count is 0, below condition (before fw_bug check) will hit.
		if (!p1)
	                return -EINVAL;

Apart from this, i dont see hid_count will remain 0.

-Ankit

> > 
> > Signed-off-by: Xichao Zhao <zhao.xichao@vivo.com>
> > ---
> >  drivers/iommu/amd/iommu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> > index eb348c63a8d0..b5c829f89544 100644
> > --- a/drivers/iommu/amd/iommu.c
> > +++ b/drivers/iommu/amd/iommu.c
> > @@ -265,7 +265,7 @@ static inline int get_acpihid_device_id(struct device *dev,
> >  		return -EINVAL;
> >  	if (fw_bug)
> >  		dev_err_once(dev, FW_BUG "No ACPI device matched UID, but %d device%s matched HID.\n",
> > -			     hid_count, hid_count > 1 ? "s" : "");
> > +			     hid_count,  str_plural(hid_count));
> >  	if (hid_count > 1)
> >  		return -EINVAL;
> >  	if (entry)
>

Re: [PATCH] iommu/amd: use str_plural() to simplify the code

Posted by Christophe JAILLET 1 month, 2 weeks ago

Le 18/08/2025 à 05:53, Xichao Zhao a écrit :
> Use the string choice helper function str_plural() to simplify the code.
> 
> Signed-off-by: Xichao Zhao <zhao.xichao@vivo.com>

Hi,

> ---
>   drivers/iommu/amd/iommu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index eb348c63a8d0..b5c829f89544 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -265,7 +265,7 @@ static inline int get_acpihid_device_id(struct device *dev,
>   		return -EINVAL;
>   	if (fw_bug)
>   		dev_err_once(dev, FW_BUG "No ACPI device matched UID, but %d device%s matched HID.\n",
> -			     hid_count, hid_count > 1 ? "s" : "");
> +			     hid_count,  str_plural(hid_count));

Nitpick: There is a double space after the comma.

>   	if (hid_count > 1)
>   		return -EINVAL;
>   	if (entry)

You should also include <linux/string_choices.h> I think.

CJ