| Message ID | 20221117125953.88441-2-umang.jain@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vc04_services: vchiq-mmal: Drop bool usage | expand |
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
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
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
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 --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);