diff mbox series

macintosh: macio_asic: fix resource_size.cocci warnings

Message ID 20220414140304.82751-1-hanyihao@vivo.com (mailing list archive)
State Changes Requested
Headers show
Series macintosh: macio_asic: fix resource_size.cocci warnings | expand

Commit Message

Yihao Han April 14, 2022, 2:02 p.m. UTC
drivers/macintosh/macio_asic.c:219:26-29: WARNING:
Suspicious code. resource_size is maybe missing with res
drivers/macintosh/macio_asic.c:221:26-29: WARNING:
Suspicious code. resource_size is maybe missing with res

Use resource_size function on resource object instead of
explicit computation.

Generated by: scripts/coccinelle/api/resource_size.cocci

Signed-off-by: Yihao Han <hanyihao@vivo.com>
---
 drivers/macintosh/macio_asic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Uwe Kleine-König April 19, 2022, 7:54 a.m. UTC | #1
On Thu, Apr 14, 2022 at 07:02:42AM -0700, Yihao Han wrote:
> drivers/macintosh/macio_asic.c:219:26-29: WARNING:
> Suspicious code. resource_size is maybe missing with res
> drivers/macintosh/macio_asic.c:221:26-29: WARNING:
> Suspicious code. resource_size is maybe missing with res

For log messages it's ok to overstep the line length limitation for
commit logs. IMHO adding newlines is worse, not sure that there are no
other strong opinions though.

> Use resource_size function on resource object instead of
> explicit computation.
> 
> Generated by: scripts/coccinelle/api/resource_size.cocci
> 
> Signed-off-by: Yihao Han <hanyihao@vivo.com>
> ---
>  drivers/macintosh/macio_asic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
> index 1943a007e2d5..260fccb3863e 100644
> --- a/drivers/macintosh/macio_asic.c
> +++ b/drivers/macintosh/macio_asic.c
> @@ -216,9 +216,9 @@ static int macio_resource_quirks(struct device_node *np, struct resource *res,
>  	/* Some older IDE resources have bogus sizes */
>  	if (of_node_name_eq(np, "IDE") || of_node_name_eq(np, "ATA") ||
>  	    of_node_is_type(np, "ide") || of_node_is_type(np, "ata")) {
> -		if (index == 0 && (res->end - res->start) > 0xfff)
> +		if (index == 0 && (resource_size(res)) > 0xfff)

You can drop the parenthesis around resource_size(res) here.

Other than that looks fine,
Uwe
Michael Ellerman April 22, 2022, 6:44 a.m. UTC | #2
Yihao Han <hanyihao@vivo.com> writes:
> drivers/macintosh/macio_asic.c:219:26-29: WARNING:
> Suspicious code. resource_size is maybe missing with res
> drivers/macintosh/macio_asic.c:221:26-29: WARNING:
> Suspicious code. resource_size is maybe missing with res
>
> Use resource_size function on resource object instead of
> explicit computation.
>
> Generated by: scripts/coccinelle/api/resource_size.cocci
>
> Signed-off-by: Yihao Han <hanyihao@vivo.com>
> ---
>  drivers/macintosh/macio_asic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
> index 1943a007e2d5..260fccb3863e 100644
> --- a/drivers/macintosh/macio_asic.c
> +++ b/drivers/macintosh/macio_asic.c
> @@ -216,9 +216,9 @@ static int macio_resource_quirks(struct device_node *np, struct resource *res,
>  	/* Some older IDE resources have bogus sizes */
>  	if (of_node_name_eq(np, "IDE") || of_node_name_eq(np, "ATA") ||
>  	    of_node_is_type(np, "ide") || of_node_is_type(np, "ata")) {
> -		if (index == 0 && (res->end - res->start) > 0xfff)
> +		if (index == 0 && (resource_size(res)) > 0xfff)
>  			res->end = res->start + 0xfff;
> -		if (index == 1 && (res->end - res->start) > 0xff)
> +		if (index == 1 && (resource_size(res)) > 0xff)

Are you sure the conversion is correct? It's not exactly equivalent:

static inline resource_size_t resource_size(const struct resource *res)
{
	return res->end - res->start + 1;
}

cheers
Uwe Kleine-König April 25, 2022, 5:23 p.m. UTC | #3
Hello Michael,

On Fri, Apr 22, 2022 at 04:44:24PM +1000, Michael Ellerman wrote:
> Yihao Han <hanyihao@vivo.com> writes:
> > -		if (index == 0 && (res->end - res->start) > 0xfff)
> > +		if (index == 0 && (resource_size(res)) > 0xfff)
> >  			res->end = res->start + 0xfff;
> > -		if (index == 1 && (res->end - res->start) > 0xff)
> > +		if (index == 1 && (resource_size(res)) > 0xff)
> 
> Are you sure the conversion is correct? It's not exactly equivalent:
> 
> static inline resource_size_t resource_size(const struct resource *res)
> {
> 	return res->end - res->start + 1;
> }

Indeed. I pointed out this issue in the v2 that already hit my inbox.

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
index 1943a007e2d5..260fccb3863e 100644
--- a/drivers/macintosh/macio_asic.c
+++ b/drivers/macintosh/macio_asic.c
@@ -216,9 +216,9 @@  static int macio_resource_quirks(struct device_node *np, struct resource *res,
 	/* Some older IDE resources have bogus sizes */
 	if (of_node_name_eq(np, "IDE") || of_node_name_eq(np, "ATA") ||
 	    of_node_is_type(np, "ide") || of_node_is_type(np, "ata")) {
-		if (index == 0 && (res->end - res->start) > 0xfff)
+		if (index == 0 && (resource_size(res)) > 0xfff)
 			res->end = res->start + 0xfff;
-		if (index == 1 && (res->end - res->start) > 0xff)
+		if (index == 1 && (resource_size(res)) > 0xff)
 			res->end = res->start + 0xff;
 	}
 	return 0;