Skip to contentSkip to conversation

livepatch: match return value to function signature

PATCH Applied Nicholas Mc Guire 7 participants 12 messages May 11, 2015 – May 28, 2015 Live patching

lore
Share & tools
lore

Nicholas Mc Guire

author
#0 lore raw

PATCH 1/1 livepatch: match return value to function signature

klp_initialized() should return bool but is actually returning struct kobject * - convert it to a boolean explicitly.

Signed-off-by Nicholas Mc Guire <[email protected]>

---

static code checking was not happy with
./kernel/livepatch/core.c:131 WARNING: return of wrong type

                               bool != struct kobject *

This adds !! to explicitly convert to boolean type.

Patch was compile tested with x86_64_defconfig

patch is against 4.1-rc2 (localversion-next is -next-20150508)

 kernel/livepatch/core.c     |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/livepatch/core.c +1 -1
index 0e7c23c..c0ae3d8 100644
@@ -128,7 +128,7 @@ static bool klp_is_patch_registered(struct klp_patch *patch)
 
 static bool klp_initialized(void)
 {
-	return klp_root_kobj;
+	return !!klp_root_kobj;
 }
 
 struct klp_find_arg {
Signature · 1 line
1.7.10.4
Reviewed-by on v1 1/1
#1 lore raw
Quoted 4 lines · Nicholas Mc Guire

klp_initialized() should return bool but is actually returning struct kobject * - convert it to a boolean explicitly.

Signed-off-by Nicholas Mc Guire <[email protected]>
Reviewed-by Jiri Slaby <[email protected]>

Thanks.

Quoted 29 lines

---

static code checking was not happy with
./kernel/livepatch/core.c:131 WARNING: return of wrong type

                               bool != struct kobject *

This adds !! to explicitly convert to boolean type.

Patch was compile tested with x86_64_defconfig

patch is against 4.1-rc2 (localversion-next is -next-20150508)

 kernel/livepatch/core.c     |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/livepatch/core.c +1 -1
index 0e7c23c..c0ae3d8 100644
@@ -128,7 +128,7 @@ static bool klp_is_patch_registered(struct klp_patch *patch)
 
 static bool klp_initialized(void)
 {
-	return klp_root_kobj;
+	return !!klp_root_kobj;
 }
 
 struct klp_find_arg {
Signature · 2 lines
js
suse labs
#2 lore raw
Quoted 8 lines · Jiri Slaby
Quoted 4 lines · Nicholas Mc Guire

klp_initialized() should return bool but is actually returning struct kobject * - convert it to a boolean explicitly.

Signed-off-by Nicholas Mc Guire <[email protected]>
Reviewed-by Jiri Slaby <[email protected]>

sorry for the noise - discussions about a similar "cleanup" in RCU resulted Joe Perches <[email protected]> clarifying the matter - this explicit conversion makes static code checkers happy but is not needed for gcc (or C for that mater) - see
http://lkml.org/lkml/2015/5/24/47
and ISO/IEC 9899:1999 (c99) 6.8.6.4 subclause 3)

thx!
hofrat

#3 lore raw
Quoted 8 lines · Nicholas Mc Guire
Quoted 6 lines · Jiri Slaby
Quoted 4 lines · Nicholas Mc Guire

klp_initialized() should return bool but is actually returning struct kobject * - convert it to a boolean explicitly.

Signed-off-by Nicholas Mc Guire <[email protected]>
Quoted 8 lines
Quoted 2 lines
Reviewed-by Jiri Slaby <[email protected]>

sorry for the noise - discussions about a similar "cleanup" in RCU resulted Joe Perches <[email protected]> clarifying the matter - this explicit conversion makes static code checkers happy but is not needed for gcc (or C for that mater) - see
http://lkml.org/lkml/2015/5/24/47
and ISO/IEC 9899:1999 (c99) 6.8.6.4 subclause 3)

Oh yes and I seem to be inconsistent with replies:
https://lkml.org/lkml/2015/5/26/90

Quoted 1 line

hofrat

You? Nuremberg, 2010, Stanse?

thanks,

Signature · 2 lines
js
suse labs
#4 lore raw
Quoted 18 lines · Jiri Slaby
Quoted 8 lines · Nicholas Mc Guire
Quoted 6 lines · Jiri Slaby
Quoted 4 lines · Nicholas Mc Guire

klp_initialized() should return bool but is actually returning struct kobject * - convert it to a boolean explicitly.

Signed-off-by Nicholas Mc Guire <[email protected]>
Quoted 8 lines
Quoted 2 lines
Reviewed-by Jiri Slaby <[email protected]>

sorry for the noise - discussions about a similar "cleanup" in RCU resulted Joe Perches <[email protected]> clarifying the matter - this explicit conversion makes static code checkers happy but is not needed for gcc (or C for that mater) - see
http://lkml.org/lkml/2015/5/24/47
and ISO/IEC 9899:1999 (c99) 6.8.6.4 subclause 3)

Quoted 7 lines

Oh yes and I seem to be inconsistent with replies:
https://lkml.org/lkml/2015/5/26/90

Quoted 1 line

hofrat

You? Nuremberg, 2010, Stanse?

Nop - coccinelle scripts - but I guess other static code checkers would have that issue as well if they look at types of variables and not take possible implicit conversions into account.

stanse is on my TODO list for SIL2LinuxMP (formal qualification of Linux according to IEC 61508 Ed 2 to Systematic Capability level SC 2)

thx!
hofrat

Acked-by on v1 1/1
#5 lore raw
In reply to Nicholas Mc Guire
Quoted 35 lines · Nicholas Mc Guire

klp_initialized() should return bool but is actually returning struct kobject * - convert it to a boolean explicitly.

Signed-off-by Nicholas Mc Guire <[email protected]>

---

static code checking was not happy with
./kernel/livepatch/core.c:131 WARNING: return of wrong type

                               bool != struct kobject *

This adds !! to explicitly convert to boolean type.

Patch was compile tested with x86_64_defconfig

patch is against 4.1-rc2 (localversion-next is -next-20150508)

 kernel/livepatch/core.c     |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/livepatch/core.c +1 -1
index 0e7c23c..c0ae3d8 100644
@@ -128,7 +128,7 @@ static bool klp_is_patch_registered(struct klp_patch *patch)
 
 static bool klp_initialized(void)
 {
-	return klp_root_kobj;
+	return !!klp_root_kobj;
 }
 
 struct klp_find_arg {
Signature · 1 line
1.7.10.4

Thanks!

Acked-by Josh Poimboeuf <[email protected]>
Applied
#6 lore raw
In reply to Nicholas Mc Guire
Quoted 8 lines · Nicholas Mc Guire

klp_initialized() should return bool but is actually returning struct kobject * - convert it to a boolean explicitly.

Signed-off-by Nicholas Mc Guire <[email protected]>

---

static code checking was not happy with
./kernel/livepatch/core.c:131 WARNING: return of wrong type

Quoted 7 lines
                               bool != struct kobject *

This adds !! to explicitly convert to boolean type.

Patch was compile tested with x86_64_defconfig

patch is against 4.1-rc2 (localversion-next is -next-20150508)

Applied to for-4.2/upstream. Thanks,

Signature · 2 lines
Jiri Kosina
SUSE Labs
#7 lore raw

livepatch: match function return value type with prototype

The klp_is_module return type should be boolean.

Signed-off-by Li Bin <[email protected]>
---
 kernel/livepatch/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/livepatch/core.c +1 -1
index 284e269..30e9339 100644
@@ -78,7 +78,7 @@ static struct klp_ops *klp_find_ops(unsigned long old_addr)
 
 static bool klp_is_module(struct klp_object *obj)
 {
-	return obj->name;
+	return !!obj->name;
 }
 
 static bool klp_is_object_loaded(struct klp_object *obj)
Signature · 1 line
1.7.1
#8 lore raw

PATCH 1/1 livepatch: match function return value type with prototype

In reply to Jiri Kosina

The klp_is_module return type should be boolean.

Signed-off-by Li Bin <[email protected]>
---
 kernel/livepatch/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/livepatch/core.c +1 -1
index 284e269..30e9339 100644
@@ -78,7 +78,7 @@ static struct klp_ops *klp_find_ops(unsigned long old_addr)
 
 static bool klp_is_module(struct klp_object *obj)
 {
-	return obj->name;
+	return !!obj->name;
 }
 
 static bool klp_is_object_loaded(struct klp_object *obj)
Signature · 1 line
1.7.1
#9 lore raw

Re: [PATCH] livepatch: match function return value type with prototype

Quoted 12 lines · Li Bin

The klp_is_module return type should be boolean.

Signed-off-by Li Bin <[email protected]>
---
 kernel/livepatch/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/livepatch/core.c +0 -0
index 284e269..30e9339 100644

kernel/livepatch/core.c static struct klp_ops *klp_find_ops(unsigned long old_addr) · line 82

Quoted 6 lines
kernel/livepatch/core.c +1 -1
@@ -78,7 +78,7 @@ static struct klp_ops *klp_find_ops(unsigned long old_addr)
 
 static bool klp_is_module(struct klp_object *obj)
 {
-       return obj->name;
+       return !!obj->name;
 }

Hi, Bin.

Does compile blame the prototype before you convert it? If not, I think we do not need to fix it, since there are a lot of places in kernel where we return point value as boolean.

Thanks
Minfei

Quoted 10 lines
kernel/livepatch/core.c +0 -1
 static bool klp_is_object_loaded(struct klp_object *obj)
--

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#10 lore raw

Re: [PATCH] livepatch: match function return value type with prototype

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Quoted 13 lines · Minfei Huang
Quoted 11 lines · Li Bin

The klp_is_module return type should be boolean.

Signed-off-by Li Bin <[email protected]> ---

kernel/livepatch/core.c | 2 +- 1 files changed, 1
insertions(+), 1 deletions(-)

kernel/livepatch/core.c +0 -0
index 284e269..30e9339 100644 --- a/kernel/livepatch/core.c +++

b/kernel/livepatch/core.c @@ -78,7 +78,7 @@ static struct klp_ops *klp_find_ops(unsigned long old_addr)

Quoted 8 lines
Quoted 2 lines

static bool klp_is_module(struct klp_object *obj) { -
return obj->name; + return !!obj->name; }

Hi, Bin.

Does compile blame the prototype before you convert it? If not, I think we do not need to fix it, since there are a lot of places in kernel where we return point value as boolean.

Yes, but the real reason is not that we use it on a lot of places, but that it is standardized in ¶ 6.3.1.2:
When any scalar value is converted to _Bool, the result is 0 if the value compares equal to 0; otherwise, the result is 1.

Along with the definition of scalar types:
Arithmetic types and pointer types are collectively called scalar types.

thanks,
- --
js
suse labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVZCGXAAoJEL0lsQQGtHBJMCEP/AufUwrMFZ7KTD/pWKpjpDlu fP8eflJ7iBxGtHXe9PGOsthY4kd1mB+kCo+54N1gks9bbnWF6P302hck5zrBHBdH /wbo7YXhbHicUNAXZUzxfDh3nkKmR96CIeqZhMp2H1UBzmGRKzd6kVSdBwlbJ0/W ZNcWmiIaALdr7aMEw+qEExV35kUdbJaqUcHKC9in3qQtlzCVQbZ5mCqURe+61ZTL 9u/Sbf3vB+nJPzyC/8uSVAxF616PiPdgGvxRrrRrRH82JmRaVJFjsRJ1WeMNxOFt s4gbNByNePmTG3SisqFFKQ6VJYyeEsxkeRKbvL2mW4IlzJjiWCbp6XHcv2/IzR9g GdIu3Kgy5R0OBXTxMbb5VLVtDRZUYnD/HiOFVLdUiqI6HQ/MtdZGujnVhWAVGIse BD8T+hTOQyt6Yk7evlEF+REOlAU8jvBvnq3PUfRlUjWso8w0giPx1Re6rQk4k39P TmfIVrve79n2nR/OejeCAB3xBEDtFbKBg9I+5ONm1gogxz/+3o6mwfMGS8TSdNs9 l4Kl/dhQhSW3aOmrclUoOZpcAzfTJSNCepTFq6+hKIbKibtnKs63wkLnGyL+XZSE dEOWBC/B5YNC9VVAm20U8jAtUS5gw3n0NpXFTxOi0nb62IGY+mT+D5pPylVn31Rq H7FDzgpZ7+lyvx+FDqpv
=UzJg
-----END PGP SIGNATURE-----

#11 lore raw

Re: [PATCH] livepatch: match function return value type with prototype

Quoted 22 lines · Jiri Slaby

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Quoted 18 lines · Minfei Huang
Quoted 11 lines · Li Bin

The klp_is_module return type should be boolean.

Signed-off-by Li Bin <[email protected]> ---

kernel/livepatch/core.c | 2 +- 1 files changed, 1
insertions(+), 1 deletions(-)

kernel/livepatch/core.c +0 -0
index 284e269..30e9339 100644 --- a/kernel/livepatch/core.c +++

b/kernel/livepatch/core.c @@ -78,7 +78,7 @@ static struct klp_ops *klp_find_ops(unsigned long old_addr)

Quoted 2 lines

static bool klp_is_module(struct klp_object *obj) { -
return obj->name; + return !!obj->name; }

Hi, Bin.

Quoted 9 lines
Quoted 3 lines

Does compile blame the prototype before you convert it? If not, I think we do not need to fix it, since there are a lot of places in kernel where we return point value as boolean.

Yes, but the real reason is not that we use it on a lot of places, but that it is standardized in ¶ 6.3.1.2:
When any scalar value is converted to _Bool, the result is 0 if the value compares equal to 0; otherwise, the result is 1.

OK, I see, thank you!
Li Bin

Quoted 31 lines

Along with the definition of scalar types:
Arithmetic types and pointer types are collectively called scalar types.

thanks,
- --
js
suse labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVZCGXAAoJEL0lsQQGtHBJMCEP/AufUwrMFZ7KTD/pWKpjpDlu fP8eflJ7iBxGtHXe9PGOsthY4kd1mB+kCo+54N1gks9bbnWF6P302hck5zrBHBdH /wbo7YXhbHicUNAXZUzxfDh3nkKmR96CIeqZhMp2H1UBzmGRKzd6kVSdBwlbJ0/W ZNcWmiIaALdr7aMEw+qEExV35kUdbJaqUcHKC9in3qQtlzCVQbZ5mCqURe+61ZTL 9u/Sbf3vB+nJPzyC/8uSVAxF616PiPdgGvxRrrRrRH82JmRaVJFjsRJ1WeMNxOFt s4gbNByNePmTG3SisqFFKQ6VJYyeEsxkeRKbvL2mW4IlzJjiWCbp6XHcv2/IzR9g GdIu3Kgy5R0OBXTxMbb5VLVtDRZUYnD/HiOFVLdUiqI6HQ/MtdZGujnVhWAVGIse BD8T+hTOQyt6Yk7evlEF+REOlAU8jvBvnq3PUfRlUjWso8w0giPx1Re6rQk4k39P TmfIVrve79n2nR/OejeCAB3xBEDtFbKBg9I+5ONm1gogxz/+3o6mwfMGS8TSdNs9 l4Kl/dhQhSW3aOmrclUoOZpcAzfTJSNCepTFq6+hKIbKibtnKs63wkLnGyL+XZSE dEOWBC/B5YNC9VVAm20U8jAtUS5gw3n0NpXFTxOi0nb62IGY+mT+D5pPylVn31Rq H7FDzgpZ7+lyvx+FDqpv
=UzJg
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

.