diff mbox series

[1/2] staging: vc04_services: mmal-vchiq: Do not assign bool to u32

Message ID 20221117125953.88441-2-umang.jain@ideasonboard.com
State New
Headers
Series vc04_services: vchiq-mmal: Drop bool usage |

Commit Message

Umang Jain Nov. 17, 2022, 12:59 p.m. UTC 
  From: Dave Stevenson <dave.stevenson@raspberrypi.com>

struct vchiq_mmal_component.enabled is a u32 type. Do not assign
it a bool.

Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Greg KH Nov. 17, 2022, 1:13 p.m. UTC | #1 
On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> it a bool.
> 
> Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> index cb921c94996a..17f8ceda87ca 100644
> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
>  
>  	ret = enable_component(instance, component);
>  	if (ret == 0)
> -		component->enabled = true;
> +		component->enabled = 1;

Why not make enabled a bool instead?

thanks,

greg k-h
Umang Jain Nov. 17, 2022, 1:27 p.m. UTC | #2 
Hi Greg,

Thanks for the comment,

On 11/17/22 6:43 PM, Greg Kroah-Hartman wrote:
> On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
>> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>
>> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
>> it a bool.
>>
>> Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> index cb921c94996a..17f8ceda87ca 100644
>> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
>>   
>>   	ret = enable_component(instance, component);
>>   	if (ret == 0)
>> -		component->enabled = true;
>> +		component->enabled = 1;
> Why not make enabled a bool instead?

Makes sense. It would probably require reverting the 640e77466e69 
("staging: mmal-vchiq: Avoid use of bool in structures")

I'll also find other occurances in vchiq-mmal (if any). Also for other 
reviewers, I found the context at:

7967656ffbfa ("coding-style: Clarify the expectations around bool")

Thanks,

uajain

>
> thanks,
>
> greg k-h
Dan Carpenter Nov. 17, 2022, 1:43 p.m. UTC | #3 
On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> it a bool.

It's not a u32 type so this is wrong.

	u32 enabled:1;

But also "true" is better than "1" in terms of a human reading the code.

Perhaps this is from a static checker?  I am also the author of a checker
tool so I know how stupid they can be.  When the checker says something
dumb, then the correct response is to be be briefly amused and not to
slavishly obey it.

regards,
dan
Dan Carpenter Nov. 17, 2022, 1:50 p.m. UTC | #4 
On Thu, Nov 17, 2022 at 06:57:07PM +0530, Umang Jain wrote:
> Hi Greg,
> 
> Thanks for the comment,
> 
> On 11/17/22 6:43 PM, Greg Kroah-Hartman wrote:
> > On Thu, Nov 17, 2022 at 06:29:52PM +0530, Umang Jain wrote:
> > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > 
> > > struct vchiq_mmal_component.enabled is a u32 type. Do not assign
> > > it a bool.
> > > 
> > > Fixes: 640e77466e69 ("staging: mmal-vchiq: Avoid use of bool in structures")
> > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > > ---
> > >   drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > index cb921c94996a..17f8ceda87ca 100644
> > > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > > @@ -1773,7 +1773,7 @@ int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
> > >   	ret = enable_component(instance, component);
> > >   	if (ret == 0)
> > > -		component->enabled = true;
> > > +		component->enabled = 1;
> > Why not make enabled a bool instead?
> 
> Makes sense. It would probably require reverting the 640e77466e69 ("staging:
> mmal-vchiq: Avoid use of bool in structures")
> 

Reverting that patch seems like a good idea.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index cb921c94996a..17f8ceda87ca 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -1773,7 +1773,7 @@  int vchiq_mmal_component_enable(struct vchiq_mmal_instance *instance,
 
 	ret = enable_component(instance, component);
 	if (ret == 0)
-		component->enabled = true;
+		component->enabled = 1;
 
 	mutex_unlock(&instance->vchiq_mutex);