2018-01-30 10:31:16

by Gustavo A. R. Silva

[permalink] [raw]
Subject: [PATCH v2] drm/edid: use true and false for boolean values

Assign true or false to boolean variables instead of an integer value.

This issue was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
Changes in v2:
- Use true for boolean value in add_detailed_mode as suggested by Daniel
Vetter.
- Update subject.

drivers/gpu/drm/drm_edid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ddd5379..b1cb262 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2767,7 +2767,7 @@ do_detailed_mode(struct detailed_timing *timing, void *c)

drm_mode_probed_add(closure->connector, newmode);
closure->modes++;
- closure->preferred = 0;
+ closure->preferred = false;
}
}

@@ -2784,7 +2784,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
struct detailed_mode_closure closure = {
.connector = connector,
.edid = edid,
- .preferred = 1,
+ .preferred = true,
.quirks = quirks,
};

--
2.7.4



2018-01-30 15:21:53

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [PATCH v2] drm/edid: use true and false for boolean values

On Tue, Jan 30, 2018 at 10:09:27AM -0500, Sean Paul wrote:
> On Tue, Jan 30, 2018 at 04:05:28AM -0600, Gustavo A. R. Silva wrote:
> > Assign true or false to boolean variables instead of an integer value.
> >
> > This issue was detected with the help of Coccinelle.
>
> I suppose you could also fix up the other preferred assignment by adding !!
> to the bitwise & operation.

Assigning >1 to a bool is well defined. No need to clutter the code with
!! imo.

>
> It's also helpful to post the spatch in the commit message so others can
> replicate your result (this case is pretty trivial, so less important).
>
> Sean
>
> >
> > Signed-off-by: Gustavo A. R. Silva <[email protected]>
> > ---
> > Changes in v2:
> > - Use true for boolean value in add_detailed_mode as suggested by Daniel
> > Vetter.
> > - Update subject.
> >
> > drivers/gpu/drm/drm_edid.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index ddd5379..b1cb262 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -2767,7 +2767,7 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
> >
> > drm_mode_probed_add(closure->connector, newmode);
> > closure->modes++;
> > - closure->preferred = 0;
> > + closure->preferred = false;
> > }
> > }
> >
> > @@ -2784,7 +2784,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
> > struct detailed_mode_closure closure = {
> > .connector = connector,
> > .edid = edid,
> > - .preferred = 1,
> > + .preferred = true,
> > .quirks = quirks,
> > };
> >
> > --
> > 2.7.4
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Ville Syrj?l?
Intel OTC

2018-01-30 16:50:02

by Sean Paul

[permalink] [raw]
Subject: Re: [PATCH v2] drm/edid: use true and false for boolean values

On Tue, Jan 30, 2018 at 04:05:28AM -0600, Gustavo A. R. Silva wrote:
> Assign true or false to boolean variables instead of an integer value.
>
> This issue was detected with the help of Coccinelle.

I suppose you could also fix up the other preferred assignment by adding !!
to the bitwise & operation.

It's also helpful to post the spatch in the commit message so others can
replicate your result (this case is pretty trivial, so less important).

Sean

>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
> ---
> Changes in v2:
> - Use true for boolean value in add_detailed_mode as suggested by Daniel
> Vetter.
> - Update subject.
>
> drivers/gpu/drm/drm_edid.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index ddd5379..b1cb262 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2767,7 +2767,7 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
>
> drm_mode_probed_add(closure->connector, newmode);
> closure->modes++;
> - closure->preferred = 0;
> + closure->preferred = false;
> }
> }
>
> @@ -2784,7 +2784,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
> struct detailed_mode_closure closure = {
> .connector = connector,
> .edid = edid,
> - .preferred = 1,
> + .preferred = true,
> .quirks = quirks,
> };
>
> --
> 2.7.4
>

--
Sean Paul, Software Engineer, Google / Chromium OS

2018-01-30 16:58:08

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH v2] drm/edid: use true and false for boolean values

On Tue, 30 Jan 2018, Ville Syrjälä <[email protected]> wrote:
> On Tue, Jan 30, 2018 at 10:09:27AM -0500, Sean Paul wrote:
>> On Tue, Jan 30, 2018 at 04:05:28AM -0600, Gustavo A. R. Silva wrote:
>> > Assign true or false to boolean variables instead of an integer value.
>> >
>> > This issue was detected with the help of Coccinelle.
>>
>> I suppose you could also fix up the other preferred assignment by adding !!
>> to the bitwise & operation.
>
> Assigning >1 to a bool is well defined. No need to clutter the code with
> !! imo.

Agreed.

BR,
Jani.

>
>>
>> It's also helpful to post the spatch in the commit message so others can
>> replicate your result (this case is pretty trivial, so less important).
>>
>> Sean
>>
>> >
>> > Signed-off-by: Gustavo A. R. Silva <[email protected]>
>> > ---
>> > Changes in v2:
>> > - Use true for boolean value in add_detailed_mode as suggested by Daniel
>> > Vetter.
>> > - Update subject.
>> >
>> > drivers/gpu/drm/drm_edid.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> > index ddd5379..b1cb262 100644
>> > --- a/drivers/gpu/drm/drm_edid.c
>> > +++ b/drivers/gpu/drm/drm_edid.c
>> > @@ -2767,7 +2767,7 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
>> >
>> > drm_mode_probed_add(closure->connector, newmode);
>> > closure->modes++;
>> > - closure->preferred = 0;
>> > + closure->preferred = false;
>> > }
>> > }
>> >
>> > @@ -2784,7 +2784,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>> > struct detailed_mode_closure closure = {
>> > .connector = connector,
>> > .edid = edid,
>> > - .preferred = 1,
>> > + .preferred = true,
>> > .quirks = quirks,
>> > };
>> >
>> > --
>> > 2.7.4
>> >
>>
>> --
>> Sean Paul, Software Engineer, Google / Chromium OS
>> _______________________________________________
>> dri-devel mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Jani Nikula, Intel Open Source Technology Center

2018-01-30 17:25:30

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v2] drm/edid: use true and false for boolean values

On Tue, Jan 30, 2018 at 11:02:44AM -0500, Sean Paul wrote:
> On Tue, Jan 30, 2018 at 05:19:46PM +0200, Ville Syrj?l? wrote:
> > On Tue, Jan 30, 2018 at 10:09:27AM -0500, Sean Paul wrote:
> > > On Tue, Jan 30, 2018 at 04:05:28AM -0600, Gustavo A. R. Silva wrote:
> > > > Assign true or false to boolean variables instead of an integer value.
> > > >
> > > > This issue was detected with the help of Coccinelle.
> > >
> > > I suppose you could also fix up the other preferred assignment by adding !!
> > > to the bitwise & operation.
> >
> > Assigning >1 to a bool is well defined. No need to clutter the code with
> > !! imo.
>
> There are examples of both in the file already. I don't have strong feelings
> either way.

Yeah, I applied this one for now, we can do more color choice discussions
with follow ups :-)

Thanks for the patch.
-Daniel

>
> Sea
>
> >
> > >
> > > It's also helpful to post the spatch in the commit message so others can
> > > replicate your result (this case is pretty trivial, so less important).
> > >
> > > Sean
> > >
> > > >
> > > > Signed-off-by: Gustavo A. R. Silva <[email protected]>
> > > > ---
> > > > Changes in v2:
> > > > - Use true for boolean value in add_detailed_mode as suggested by Daniel
> > > > Vetter.
> > > > - Update subject.
> > > >
> > > > drivers/gpu/drm/drm_edid.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > index ddd5379..b1cb262 100644
> > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > @@ -2767,7 +2767,7 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
> > > >
> > > > drm_mode_probed_add(closure->connector, newmode);
> > > > closure->modes++;
> > > > - closure->preferred = 0;
> > > > + closure->preferred = false;
> > > > }
> > > > }
> > > >
> > > > @@ -2784,7 +2784,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
> > > > struct detailed_mode_closure closure = {
> > > > .connector = connector,
> > > > .edid = edid,
> > > > - .preferred = 1,
> > > > + .preferred = true,
> > > > .quirks = quirks,
> > > > };
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > >
> > > --
> > > Sean Paul, Software Engineer, Google / Chromium OS
> > > _______________________________________________
> > > dri-devel mailing list
> > > [email protected]
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Ville Syrj?l?
> > Intel OTC
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2018-01-30 17:49:20

by Sean Paul

[permalink] [raw]
Subject: Re: [PATCH v2] drm/edid: use true and false for boolean values

On Tue, Jan 30, 2018 at 05:19:46PM +0200, Ville Syrj?l? wrote:
> On Tue, Jan 30, 2018 at 10:09:27AM -0500, Sean Paul wrote:
> > On Tue, Jan 30, 2018 at 04:05:28AM -0600, Gustavo A. R. Silva wrote:
> > > Assign true or false to boolean variables instead of an integer value.
> > >
> > > This issue was detected with the help of Coccinelle.
> >
> > I suppose you could also fix up the other preferred assignment by adding !!
> > to the bitwise & operation.
>
> Assigning >1 to a bool is well defined. No need to clutter the code with
> !! imo.

There are examples of both in the file already. I don't have strong feelings
either way.

Sea

>
> >
> > It's also helpful to post the spatch in the commit message so others can
> > replicate your result (this case is pretty trivial, so less important).
> >
> > Sean
> >
> > >
> > > Signed-off-by: Gustavo A. R. Silva <[email protected]>
> > > ---
> > > Changes in v2:
> > > - Use true for boolean value in add_detailed_mode as suggested by Daniel
> > > Vetter.
> > > - Update subject.
> > >
> > > drivers/gpu/drm/drm_edid.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index ddd5379..b1cb262 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -2767,7 +2767,7 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
> > >
> > > drm_mode_probed_add(closure->connector, newmode);
> > > closure->modes++;
> > > - closure->preferred = 0;
> > > + closure->preferred = false;
> > > }
> > > }
> > >
> > > @@ -2784,7 +2784,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
> > > struct detailed_mode_closure closure = {
> > > .connector = connector,
> > > .edid = edid,
> > > - .preferred = 1,
> > > + .preferred = true,
> > > .quirks = quirks,
> > > };
> > >
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> > _______________________________________________
> > dri-devel mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrj?l?
> Intel OTC

--
Sean Paul, Software Engineer, Google / Chromium OS