[PATCH] livepatch: match return value to function signature

[PATCH] livepatch: match return value to function signature

[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 <hofrat@osadl.org>
---

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

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 0e7c23c..c0ae3d8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -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 {
-- 
1.7.10.4

Re: [PATCH] livepatch: match return value to function signature

[Jiri Slaby]

On 05/11/2015, 07:52 AM, Nicholas Mc Guire wrote:
> klp_initialized() should return bool but is actually returning 
> struct kobject * - convert it to a boolean explicitly.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>

Reviewed-by: Jiri Slaby <jslaby@suse.cz>

Thanks.

> ---
> 
> 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(-)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 0e7c23c..c0ae3d8 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -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 {
> 


-- 
js
suse labs

Re: [PATCH] livepatch: match return value to function signature

[Nicholas Mc Guire]

On Mon, 11 May 2015, Jiri Slaby wrote:

> On 05/11/2015, 07:52 AM, Nicholas Mc Guire wrote:
> > klp_initialized() should return bool but is actually returning 
> > struct kobject * - convert it to a boolean explicitly.
> > 
> > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> 
> Reviewed-by: Jiri Slaby <jslaby@suse.cz>
>
sorry for the noise - discussions about a similar "cleanup" in RCU
resulted Joe Perches <joe@perches.com> 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

Re: [PATCH] livepatch: match return value to function signature

[Jiri Slaby]

On 05/27/2015, 08:56 AM, Nicholas Mc Guire wrote:
> On Mon, 11 May 2015, Jiri Slaby wrote:
> 
>> On 05/11/2015, 07:52 AM, Nicholas Mc Guire wrote:
>>> klp_initialized() should return bool but is actually returning 
>>> struct kobject * - convert it to a boolean explicitly.
>>>
>>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
>>
>> Reviewed-by: Jiri Slaby <jslaby@suse.cz>
>>
> sorry for the noise - discussions about a similar "cleanup" in RCU
> resulted Joe Perches <joe@perches.com> 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

> hofrat

You? Nuremberg, 2010, Stanse?

thanks,
-- 
js
suse labs

Re: [PATCH] livepatch: match return value to function signature

[Nicholas Mc Guire]

On Wed, 27 May 2015, Jiri Slaby wrote:

> On 05/27/2015, 08:56 AM, Nicholas Mc Guire wrote:
> > On Mon, 11 May 2015, Jiri Slaby wrote:
> > 
> >> On 05/11/2015, 07:52 AM, Nicholas Mc Guire wrote:
> >>> klp_initialized() should return bool but is actually returning 
> >>> struct kobject * - convert it to a boolean explicitly.
> >>>
> >>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> >>
> >> Reviewed-by: Jiri Slaby <jslaby@suse.cz>
> >>
> > sorry for the noise - discussions about a similar "cleanup" in RCU
> > resulted Joe Perches <joe@perches.com> 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
> 
> > 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

Re: [PATCH] livepatch: match return value to function signature

[Josh Poimboeuf]

On Mon, May 11, 2015 at 07:52:29AM +0200, Nicholas Mc Guire wrote:
> klp_initialized() should return bool but is actually returning 
> struct kobject * - convert it to a boolean explicitly.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> ---
> 
> 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(-)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 0e7c23c..c0ae3d8 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -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 {
> -- 
> 1.7.10.4
> 

Thanks!

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

Re: [PATCH] livepatch: match return value to function signature

[Jiri Kosina]

On Mon, 11 May 2015, Nicholas Mc Guire wrote:

> klp_initialized() should return bool but is actually returning 
> struct kobject * - convert it to a boolean explicitly.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> ---
> 
> 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)

Applied to for-4.2/upstream. Thanks,

-- 
Jiri Kosina
SUSE Labs

livepatch: match function return value type with prototype

[Li Bin]

The klp_is_module return type should be boolean.

Signed-off-by: Li Bin <huawei.libin@huawei.com>
---
 kernel/livepatch/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
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)

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

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

[Li Bin]

The klp_is_module return type should be boolean.

Signed-off-by: Li Bin <huawei.libin@huawei.com>
---
 kernel/livepatch/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
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)

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

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

[Minfei Huang]

On Tue, May 26, 2015 at 10:44 AM, Li Bin <huawei.libin@huawei.com> wrote:
> The klp_is_module return type should be boolean.
>
> Signed-off-by: Li Bin <huawei.libin@huawei.com>
> ---
>  kernel/livepatch/core.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> 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)
>
>  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

>
>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jiri Slaby]

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

On 05/26/2015, 06:46 AM, Minfei Huang wrote:
> On Tue, May 26, 2015 at 10:44 AM, Li Bin <huawei.libin@huawei.com>
> wrote:
>> The klp_is_module return type should be boolean.
>> 
>> Signed-off-by: Li Bin <huawei.libin@huawei.com> --- 
>> kernel/livepatch/core.c |    2 +- 1 files changed, 1
>> insertions(+), 1 deletions(-)
>> 
>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c 
>> 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)
>> 
>> 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-----

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

[Li Bin]

On 2015/5/26 15:32, Jiri Slaby wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> On 05/26/2015, 06:46 AM, Minfei Huang wrote:
>> On Tue, May 26, 2015 at 10:44 AM, Li Bin <huawei.libin@huawei.com>
>> wrote:
>>> The klp_is_module return type should be boolean.
>>>
>>> Signed-off-by: Li Bin <huawei.libin@huawei.com> --- 
>>> kernel/livepatch/core.c |    2 +- 1 files changed, 1
>>> insertions(+), 1 deletions(-)
>>>
>>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c 
>>> 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)
>>>
>>> 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.
> 

OK, I see, thank you!
	Li Bin

> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
>