Return-path: Received: from mx1.redhat.com ([209.132.183.28]:45256 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753176AbcGSQFC (ORCPT ); Tue, 19 Jul 2016 12:05:02 -0400 From: Jes Sorensen To: Arnd Bergmann Cc: linux-wireless@vger.kernel.org, Kalle Valo , Larry Finger , netdev@vger.kernel.org, Greg Kroah-Hartman , Mateusz Kulikowski , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] staging/rtl8192e: use s8 instead of char References: <20160719153403.2967812-1-arnd@arndb.de> <20160719153403.2967812-2-arnd@arndb.de> <3858062.NGQDOLnTj0@wuerfel> Date: Tue, 19 Jul 2016 12:05:00 -0400 In-Reply-To: <3858062.NGQDOLnTj0@wuerfel> (Arnd Bergmann's message of "Tue, 19 Jul 2016 17:53:38 +0200") Message-ID: (sfid-20160719_180532_351149_57FCD673) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-wireless-owner@vger.kernel.org List-ID: Arnd Bergmann writes: > On Tuesday, July 19, 2016 11:46:04 AM CEST Jes Sorensen wrote: >> > 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. > > Ok, I'll remove the extra comparison then to avoid the warning: > > staging/rtl8192e/rtl819x_TSProc.c:326:14: error: comparison is always > true due to limited range of data type [-Werror=type-limits] > > I guess it should be a separate patch. I had just stumbled over the > same thing before resending the patch but decided not to change it > to keep the patch simple. I think that would be better, albeit not a big issue. I'd like to get rid of all the drivers/staging/rtl* drivers eventually :) Cheers, Jes