Re: [PATCH -next 0/8] drm/amd/amdgpu: fix comparison pointer to bool warning

From: Christian König
Date: Wed Sep 16 2020 - 16:53:16 EST


Am 16.09.20 um 15:36 schrieb Alex Deucher:
On Wed, Sep 16, 2020 at 3:51 AM Daniel Vetter <daniel@xxxxxxxx> wrote:
On Wed, Sep 16, 2020 at 09:38:34AM +0200, Christian König wrote:
Am 15.09.20 um 21:35 schrieb Ville Syrjälä:
On Tue, Sep 15, 2020 at 03:16:32PM -0400, Alex Deucher wrote:
I question the value of these warnings. Why even have a boolean type
if you are going to get warnings when you use them...
That said, applied to avoid getting these patches again and again
every time someone sees this.
if (this_is_sparta)
if (this_is_sparta == true)
if (this_is_sparta != false)

I think the first one reads the best, and avoids having to
decide between truth and falsehood :)
+1
+1, especially because we also have the inversion when using negative
errno codes for failures and 0 as success, which results in

if (errno == 0) /* success case */

but
if (bool == 0) /* failure case */

now creative people do sometimes

if (!errno) /* success case */

which I think is horribly confusing. So imo for more easier telling apart
of these too I think consistently using the short form for booleans, and
consistently using the more explicit long form for errno checks is a Very
Good Pattern :-)
I don't disagree with your logic, but we regularly get patches to
convert errno checks to drop the direct comparison because that is the
"preferred kernel style". Arguably, we should be explicit in all
cases as that avoids all confusion. With that in mind, my original
point stands. Why have a type when comparisons against valid settings
for that type produce errors?

Well it isn't an error, but raising a nice warning is most likely a good idea.

Christian.


Alex

Cheers, Daniel

Christian.

Alex

On Wed, Sep 9, 2020 at 9:21 AM Christian König <christian.koenig@xxxxxxx> wrote:
Acked-by: Christian König <christian.koenig@xxxxxxx> for the series.

Am 09.09.20 um 15:07 schrieb Zheng Bin:
Zheng Bin (8):
drm/amd/amdgpu: fix comparison pointer to bool warning in gfx_v9_0.c
drm/amd/amdgpu: fix comparison pointer to bool warning in gfx_v10_0.c
drm/amd/amdgpu: fix comparison pointer to bool warning in sdma_v5_0.c
drm/amd/amdgpu: fix comparison pointer to bool warning in sdma_v5_2.c
drm/amd/amdgpu: fix comparison pointer to bool warning in si.c
drm/amd/amdgpu: fix comparison pointer to bool warning in uvd_v6_0.c
drm/amd/amdgpu: fix comparison pointer to bool warning in
amdgpu_atpx_handler.c
drm/amd/amdgpu: fix comparison pointer to bool warning in sdma_v4_0.c

drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 2 +-
drivers/gpu/drm/amd/amdgpu/si.c | 2 +-
drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 4 ++--
8 files changed, 11 insertions(+), 11 deletions(-)

--
2.26.0.106.g9fadedd

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Cdc7a8d7517d341e3a80c08d85a458ba8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637358602051676053&amp;sdata=MS0vcBcU7unXjEFlbd8kLbJkJ4sKcvIdLjc8yhX4UUI%3D&amp;reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Cdc7a8d7517d341e3a80c08d85a458ba8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637358602051686051&amp;sdata=MgfR%2BwCVY9gWfhQ9i5kWcKiiYkV1C8O2dEKlZYSqscE%3D&amp;reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Cdc7a8d7517d341e3a80c08d85a458ba8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637358602051686051&amp;sdata=MgfR%2BwCVY9gWfhQ9i5kWcKiiYkV1C8O2dEKlZYSqscE%3D&amp;reserved=0
--
Daniel Vetter
Software Engineer, Intel Corporation
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Cdc7a8d7517d341e3a80c08d85a458ba8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637358602051686051&amp;sdata=GvLs5OXw2Ny%2BieJxm8hjawNb0rGA966539iAwlWwPMY%3D&amp;reserved=0