From: Jes Sorensen <Jes.Sorensen@redhat•com>
To: Arnd Bergmann <arnd@arndb•de>
Cc: linux-wireless@vger•kernel.org, Kalle Valo <kvalo@codeaurora•org>,
Larry Finger <Larry.Finger@lwfinger•net>,
netdev@vger•kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation•org>,
Mateusz Kulikowski <mateusz.kulikowski@gmail•com>,
devel@driverdev•osuosl.org, linux-kernel@vger•kernel.org
Subject: Re: [PATCH 2/3] staging/rtl8192e: use s8 instead of char
Date: Tue, 19 Jul 2016 11:46:04 -0400 [thread overview]
Message-ID: <wrfjy44xvdfn.fsf@redhat.com> (raw)
In-Reply-To: <20160719153403.2967812-2-arnd@arndb.de> (Arnd Bergmann's message of "Tue, 19 Jul 2016 17:33:01 +0200")
Arnd Bergmann <arnd@arndb•de> writes:
> Compiling the rtlwifi drivers for ARM with gcc -Wextra warns about lots of
> incorrect code that results from 'char' being unsigned here, e.g.
>
> staging/rtl8192e/rtl8192e/r8192E_phy.c:1072:36: error: comparison is always false due to limited range of data type [-Werror=type-limits]
> staging/rtl8192e/rtl8192e/r8192E_phy.c:1104:36: error: comparison is always false due to limited range of data type [-Werror=type-limits]
> staging/rtl8192e/rtl8192e/rtl_core.c:1987:16: error: comparison is always false due to limited range of data type [-Werror=type-limits]
> staging/rtl8192e/rtl8192e/rtl_dm.c:782:37: error: comparison is always false due to limited range of data type [-Werror=type-limits]
> staging/rtl8192e/rtl819x_TSProc.c:326:14: error: comparison is always true due to limited range of data type [-Werror=type-limits]
> staging/rtl8192e/rtllib_softmac_wx.c:465:16: error: comparison is always false due to limited range of data type [-Werror=type-limits]
>
> This patch changes all uses of 'char' in this driver that refer to
> 8-bit integers to use 's8' instead, which is signed on all architectures.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb•de>
> ---
> drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c | 8 ++++----
> drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c | 2 +-
> drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 6 +++---
> drivers/staging/rtl8192e/rtl8192e/rtl_core.h | 8 ++++----
> drivers/staging/rtl8192e/rtl819x_TSProc.c | 2 +-
> 5 files changed, 13 insertions(+), 13 deletions(-)
>
Most of this looks fine to me. One issue stands out which I don't think
is right:
> diff --git a/drivers/staging/rtl8192e/rtl819x_TSProc.c b/drivers/staging/rtl8192e/rtl819x_TSProc.c
> index 2c8a526773ed..e0a2fe5e6148 100644
> --- a/drivers/staging/rtl8192e/rtl819x_TSProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_TSProc.c
> @@ -323,7 +323,7 @@ bool GetTs(struct rtllib_device *ieee, struct ts_common_info **ppTS,
> if (ieee->current_network.qos_data.supported == 0) {
> UP = 0;
> } else {
> - if (!IsACValid(TID)) {
> + if (!IsACValid((s8)TID)) {
> netdev_warn(ieee->dev, "%s(): TID(%d) is not valid\n",
> __func__, TID);
> return false;
TID is a 4-bit field, it should never go negative. The cast to s8 seems
wrong to me, if anything it should be using u8. I do realize the macro
IsACValid checks against negative too, but that just looks silly to me.
Cheers,
Jes
next prev parent reply other threads:[~2016-07-19 15:46 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-19 15:33 [PATCH 1/3] rtlwifi: don't add include path for rtl8188ee Arnd Bergmann
2016-07-19 15:33 ` [PATCH 2/3] staging/rtl8192e: use s8 instead of char Arnd Bergmann
2016-07-19 15:46 ` Jes Sorensen [this message]
2016-07-19 15:53 ` Arnd Bergmann
2016-07-19 16:05 ` Jes Sorensen
[not found] ` <wrfjlh0xvck3.fsf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-07-20 8:25 ` Arnd Bergmann
2016-07-20 11:25 ` Jes Sorensen
[not found] ` <wrfjlh0wsg9s.fsf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-07-20 14:25 ` Larry Finger
2016-07-20 15:12 ` Arnd Bergmann
2016-07-20 15:33 ` Jes Sorensen
2016-07-20 16:00 ` Arnd Bergmann
2016-07-22 2:39 ` Stefan Lippers-Hollmann
2016-07-22 11:55 ` Jes Sorensen
2016-07-22 19:47 ` Arnd Bergmann
2016-07-22 20:51 ` Stefan Lippers-Hollmann
2016-07-19 15:33 ` [PATCH 3/3] staging/rtl8192u: " Arnd Bergmann
2016-07-19 15:47 ` Jes Sorensen
2016-07-19 18:17 ` [1/3] rtlwifi: don't add include path for rtl8188ee Kalle Valo
-- strict thread matches above, loose matches on Subject: below --
2016-07-20 15:06 [PATCH 2/3] staging/rtl8192e: use s8 instead of char Xose Vazquez Perez
[not found] ` <74d0fd1d-8197-1e4a-1ab1-a879d6dcea4e-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-07-20 15:48 ` Christian Lamparter
2016-07-20 15:13 [PATCH 1/3] staging/rtl8192u: " Arnd Bergmann
2016-07-20 15:26 ` [PATCH 2/3] staging/rtl8192e: " Arnd Bergmann
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=wrfjy44xvdfn.fsf@redhat.com \
--to=jes.sorensen@redhat$(echo .)com \
--cc=Larry.Finger@lwfinger$(echo .)net \
--cc=arnd@arndb$(echo .)de \
--cc=devel@driverdev$(echo .)osuosl.org \
--cc=gregkh@linuxfoundation$(echo .)org \
--cc=kvalo@codeaurora$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-wireless@vger$(echo .)kernel.org \
--cc=mateusz.kulikowski@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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