From: Ezequiel Garcia <ezequiel@collabora.com>
To: Nathan Chancellor <natechancellor@gmail.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Hans Verkuil <hverkuil@xs4all.nl>
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
clang-built-linux@googlegroups.com,
Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH v2] media: v4l2-device.h: Explicitly compare grpmask to zero in v4l2_device_mask_call_all
Date: Sat, 07 Dec 2019 07:41:27 -0300 [thread overview]
Message-ID: <79cb93108abc1f0a11f41ad3ab02cc5535b3e784.camel@collabora.com> (raw)
In-Reply-To: <20191105045907.26123-1-natechancellor@gmail.com>
(Adding Hans)
Hi Nathan, Nick,
Thanks for the patch (and the patience).
On Mon, 2019-11-04 at 21:59 -0700, Nathan Chancellor wrote:
> When building with Clang + -Wtautological-constant-compare, several of
> the ivtv drivers warn along the lines of:
>
Nitpick: s/ivtv/ivtv and cx18
> drivers/media/pci/cx18/cx18-driver.c:1005:21: warning: converting the
> result of '<<' to a boolean always evaluates to true
> [-Wtautological-constant-compare]
> cx18_call_hw(cx, CX18_HW_GPIO_RESET_CTRL,
> ^
> drivers/media/pci/cx18/cx18-cards.h:18:37: note: expanded from macro
> 'CX18_HW_GPIO_RESET_CTRL'
> #define CX18_HW_GPIO_RESET_CTRL (1 << 6)
> ^
> 1 warning generated.
>
> This is because the shift operation is implicitly converted to a boolean
> in v4l2_device_mask_call_all before being negated. This can be solved by
> just comparing the mask result to 0 explicitly so that there is no
> boolean conversion.
>
Perhaps it's interesting to mention the reason for this change,
which you mentioned as a comment before:
"""
The ultimate goal is to get
-Wtautological-compare enabled because there are several subwarnings
that would be helpful to have and right now they are all disabled.
"""
> Link: https://github.com/ClangBuiltLinux/linux/issues/752
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> v1 -> v2: https://lore.kernel.org/lkml/20191024201240.49063-1-natechancellor@gmail.com/
>
> * Fix typo in commit message
> * Add Nick's Reviewed-by.
>
> include/media/v4l2-device.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
> index e0b8f2602670..8564b3227887 100644
> --- a/include/media/v4l2-device.h
> +++ b/include/media/v4l2-device.h
> @@ -431,8 +431,8 @@ static inline bool v4l2_device_supports_requests(struct v4l2_device *v4l2_dev)
> struct v4l2_subdev *__sd; \
> \
> __v4l2_device_call_subdevs_p(v4l2_dev, __sd, \
> - !(grpmsk) || (__sd->grp_id & (grpmsk)), o, f , \
> - ##args); \
> + (grpmsk) == 0 || (__sd->grp_id & (grpmsk)), o, \
> + f , ##args); \
> } while (0)
>
> /**
The change is small enough and if it helps enabling helpful
warnings, then it seems a good idea.
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
For consistency, it would be good to also patch the other case(s).
Thanks,
Ezequiel
next prev parent reply other threads:[~2019-12-07 10:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-24 20:12 [PATCH] media: v4l2-device.h: Explicitly compare grpmask to zero in v4l2_device_mask_call_all Nathan Chancellor
2019-11-04 19:07 ` Nick Desaulniers
2019-11-05 4:59 ` [PATCH v2] " Nathan Chancellor
2019-12-06 21:59 ` Nick Desaulniers
2019-12-07 10:41 ` Ezequiel Garcia [this message]
2019-12-08 21:11 ` [PATCH v3] media: v4l2-device.h: Explicitly compare grp{id,mask} to zero in v4l2_device macros Nathan Chancellor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=79cb93108abc1f0a11f41ad3ab02cc5535b3e784.camel@collabora.com \
--to=ezequiel@collabora.com \
--cc=clang-built-linux@googlegroups.com \
--cc=hverkuil@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=natechancellor@gmail.com \
--cc=ndesaulniers@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).