* [PATCH] staging: rtl8192e: use explicitly signed char @ 2022-10-24 16:30 Jason A. Donenfeld 2022-10-25 6:19 ` Dan Carpenter 2022-10-25 17:22 ` [PATCH] staging: rtl8192e: use explicitly signed char Philipp Hortmann 0 siblings, 2 replies; 9+ messages in thread From: Jason A. Donenfeld @ 2022-10-24 16:30 UTC (permalink / raw) To: linux-kernel; +Cc: Jason A. Donenfeld, Greg Kroah-Hartman, linux-staging With char becoming unsigned by default, and with `char` alone being ambiguous and based on architecture, signed chars need to be marked explicitly as such. In this case, passing `char *extra` is part of the iw API, and that extra is mostly intended to be somewhat opaque. So just cast to `s8 *` for the sign test. This fixes warnings like: drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-staging@lists.linux.dev Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- drivers/staging/rtl8192e/rtllib_softmac_wx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c index f9589c5b62ba..4563e3b5bd47 100644 --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c @@ -456,7 +456,7 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, } for (i = 0; i < len; i++) { - if (extra[i] < 0) { + if (((s8 *)extra)[i] < 0) { ret = -1; goto out; } -- 2.38.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] staging: rtl8192e: use explicitly signed char 2022-10-24 16:30 [PATCH] staging: rtl8192e: use explicitly signed char Jason A. Donenfeld @ 2022-10-25 6:19 ` Dan Carpenter 2022-10-25 10:45 ` Greg Kroah-Hartman 2022-10-25 17:22 ` [PATCH] staging: rtl8192e: use explicitly signed char Philipp Hortmann 1 sibling, 1 reply; 9+ messages in thread From: Dan Carpenter @ 2022-10-25 6:19 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: linux-kernel, Greg Kroah-Hartman, linux-staging On Mon, Oct 24, 2022 at 06:30:05PM +0200, Jason A. Donenfeld wrote: > With char becoming unsigned by default, and with `char` alone being > ambiguous and based on architecture, signed chars need to be marked > explicitly as such. In this case, passing `char *extra` is part of the > iw API, and that extra is mostly intended to be somewhat opaque. So just > cast to `s8 *` for the sign test. This fixes warnings like: > > drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-staging@lists.linux.dev > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > drivers/staging/rtl8192e/rtllib_softmac_wx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > index f9589c5b62ba..4563e3b5bd47 100644 > --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c > +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > @@ -456,7 +456,7 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > } > > for (i = 0; i < len; i++) { > - if (extra[i] < 0) { > + if (((s8 *)extra)[i] < 0) { I agree with Linus that this if statement is nonsense and should just be deleted. regards, dan carpenter > ret = -1; > goto out; > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] staging: rtl8192e: use explicitly signed char 2022-10-25 6:19 ` Dan Carpenter @ 2022-10-25 10:45 ` Greg Kroah-Hartman 2022-10-25 12:21 ` [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test Jason A. Donenfeld 0 siblings, 1 reply; 9+ messages in thread From: Greg Kroah-Hartman @ 2022-10-25 10:45 UTC (permalink / raw) To: Dan Carpenter, Jason A. Donenfeld; +Cc: linux-kernel, linux-staging On Tue, Oct 25, 2022 at 09:19:58AM +0300, Dan Carpenter wrote: > On Mon, Oct 24, 2022 at 06:30:05PM +0200, Jason A. Donenfeld wrote: > > With char becoming unsigned by default, and with `char` alone being > > ambiguous and based on architecture, signed chars need to be marked > > explicitly as such. In this case, passing `char *extra` is part of the > > iw API, and that extra is mostly intended to be somewhat opaque. So just > > cast to `s8 *` for the sign test. This fixes warnings like: > > > > drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: linux-staging@lists.linux.dev > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > --- > > drivers/staging/rtl8192e/rtllib_softmac_wx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > index f9589c5b62ba..4563e3b5bd47 100644 > > --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > @@ -456,7 +456,7 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > > } > > > > for (i = 0; i < len; i++) { > > - if (extra[i] < 0) { > > + if (((s8 *)extra)[i] < 0) { > > I agree with Linus that this if statement is nonsense and should just be > deleted. Yeah, I agree as well, let's just delete this invalid check. No other wifi driver cares about ssid characters like this. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test 2022-10-25 10:45 ` Greg Kroah-Hartman @ 2022-10-25 12:21 ` Jason A. Donenfeld 2022-10-25 12:34 ` Dan Carpenter 2022-10-25 17:35 ` Philipp Hortmann 0 siblings, 2 replies; 9+ messages in thread From: Jason A. Donenfeld @ 2022-10-25 12:21 UTC (permalink / raw) To: linux-kernel, linux-staging, dan.carpenter, gregkh, kvalo, linux-wireless Cc: Jason A. Donenfeld This error triggers on some architectures with unsigned `char` types: drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' But actually, the entire test is bogus, as ssids don't have any sign validity rules like that. So just remove this check look all together. Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-staging@lists.linux.dev Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- Changes v1->v2: - Remove ssid sign test entirely rather than casting to `s8 *`. drivers/staging/rtl8192e/rtllib_softmac_wx.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c index f9589c5b62ba..1e5ad3b476ef 100644 --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c @@ -439,7 +439,7 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, union iwreq_data *wrqu, char *extra) { - int ret = 0, len, i; + int ret = 0, len; short proto_started; unsigned long flags; @@ -455,13 +455,6 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, goto out; } - for (i = 0; i < len; i++) { - if (extra[i] < 0) { - ret = -1; - goto out; - } - } - if (proto_started) rtllib_stop_protocol(ieee, true); -- 2.38.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test 2022-10-25 12:21 ` [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test Jason A. Donenfeld @ 2022-10-25 12:34 ` Dan Carpenter 2022-10-25 17:35 ` Philipp Hortmann 1 sibling, 0 replies; 9+ messages in thread From: Dan Carpenter @ 2022-10-25 12:34 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-kernel, linux-staging, gregkh, kvalo, linux-wireless On Tue, Oct 25, 2022 at 02:21:50PM +0200, Jason A. Donenfeld wrote: > This error triggers on some architectures with unsigned `char` types: > > drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' > > But actually, the entire test is bogus, as ssids don't have any sign > validity rules like that. So just remove this check look all together. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-staging@lists.linux.dev > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > Changes v1->v2: > - Remove ssid sign test entirely rather than casting to `s8 *`. Thanks! Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> regards, dan carpenter ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test 2022-10-25 12:21 ` [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test Jason A. Donenfeld 2022-10-25 12:34 ` Dan Carpenter @ 2022-10-25 17:35 ` Philipp Hortmann 2022-10-25 17:41 ` Greg KH 2022-10-25 17:43 ` Jason A. Donenfeld 1 sibling, 2 replies; 9+ messages in thread From: Philipp Hortmann @ 2022-10-25 17:35 UTC (permalink / raw) To: Jason A. Donenfeld, linux-kernel, linux-staging, dan.carpenter, gregkh, kvalo, linux-wireless On 10/25/22 14:21, Jason A. Donenfeld wrote: > This error triggers on some architectures with unsigned `char` types: > > drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' > > But actually, the entire test is bogus, as ssids don't have any sign > validity rules like that. So just remove this check look all together. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-staging@lists.linux.dev > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > Changes v1->v2: > - Remove ssid sign test entirely rather than casting to `s8 *`. > > drivers/staging/rtl8192e/rtllib_softmac_wx.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > index f9589c5b62ba..1e5ad3b476ef 100644 > --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c > +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > @@ -439,7 +439,7 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > union iwreq_data *wrqu, char *extra) > { > > - int ret = 0, len, i; > + int ret = 0, len; > short proto_started; > unsigned long flags; > > @@ -455,13 +455,6 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > goto out; > } > > - for (i = 0; i < len; i++) { > - if (extra[i] < 0) { > - ret = -1; > - goto out; > - } > - } > - > if (proto_started) > rtllib_stop_protocol(ieee, true); This patch cannot be applied on: [PATCH] staging: rtl8192e: use explicitly signed char On 10/24/22 18:30, Jason A. Donenfeld As line 456 was changed. Bye Philipp > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test 2022-10-25 17:35 ` Philipp Hortmann @ 2022-10-25 17:41 ` Greg KH 2022-10-25 17:43 ` Jason A. Donenfeld 1 sibling, 0 replies; 9+ messages in thread From: Greg KH @ 2022-10-25 17:41 UTC (permalink / raw) To: Philipp Hortmann Cc: Jason A. Donenfeld, linux-kernel, linux-staging, dan.carpenter, kvalo, linux-wireless On Tue, Oct 25, 2022 at 07:35:08PM +0200, Philipp Hortmann wrote: > On 10/25/22 14:21, Jason A. Donenfeld wrote: > > This error triggers on some architectures with unsigned `char` types: > > > > drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' > > > > But actually, the entire test is bogus, as ssids don't have any sign > > validity rules like that. So just remove this check look all together. > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: linux-staging@lists.linux.dev > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > --- > > Changes v1->v2: > > - Remove ssid sign test entirely rather than casting to `s8 *`. > > > > drivers/staging/rtl8192e/rtllib_softmac_wx.c | 9 +-------- > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > index f9589c5b62ba..1e5ad3b476ef 100644 > > --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > @@ -439,7 +439,7 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > > union iwreq_data *wrqu, char *extra) > > { > > - int ret = 0, len, i; > > + int ret = 0, len; > > short proto_started; > > unsigned long flags; > > @@ -455,13 +455,6 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > > goto out; > > } > > - for (i = 0; i < len; i++) { > > - if (extra[i] < 0) { > > - ret = -1; > > - goto out; > > - } > > - } > > - > > if (proto_started) > > rtllib_stop_protocol(ieee, true); > > This patch cannot be applied on: > [PATCH] staging: rtl8192e: use explicitly signed char > On 10/24/22 18:30, Jason A. Donenfeld > As line 456 was changed. This now in my staging-linus branch, so perhaps you applied it to the wrong one. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test 2022-10-25 17:35 ` Philipp Hortmann 2022-10-25 17:41 ` Greg KH @ 2022-10-25 17:43 ` Jason A. Donenfeld 1 sibling, 0 replies; 9+ messages in thread From: Jason A. Donenfeld @ 2022-10-25 17:43 UTC (permalink / raw) To: Philipp Hortmann Cc: linux-kernel, linux-staging, dan.carpenter, gregkh, kvalo, linux-wireless On Tue, Oct 25, 2022 at 7:35 PM Philipp Hortmann <philipp.g.hortmann@gmail.com> wrote: > > On 10/25/22 14:21, Jason A. Donenfeld wrote: > > This error triggers on some architectures with unsigned `char` types: > > > > drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' > > > > But actually, the entire test is bogus, as ssids don't have any sign > > validity rules like that. So just remove this check look all together. > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: linux-staging@lists.linux.dev > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > --- > > Changes v1->v2: > > - Remove ssid sign test entirely rather than casting to `s8 *`. > > > > drivers/staging/rtl8192e/rtllib_softmac_wx.c | 9 +-------- > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > index f9589c5b62ba..1e5ad3b476ef 100644 > > --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > > @@ -439,7 +439,7 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > > union iwreq_data *wrqu, char *extra) > > { > > > > - int ret = 0, len, i; > > + int ret = 0, len; > > short proto_started; > > unsigned long flags; > > > > @@ -455,13 +455,6 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > > goto out; > > } > > > > - for (i = 0; i < len; i++) { > > - if (extra[i] < 0) { > > - ret = -1; > > - goto out; > > - } > > - } > > - > > if (proto_started) > > rtllib_stop_protocol(ieee, true); > > This patch cannot be applied on: > [PATCH] staging: rtl8192e: use explicitly signed char They're mutually exclusive, which is why this one here was marked as a v2 and sent in reply to that one. Greg picked up the v2 and all is well. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] staging: rtl8192e: use explicitly signed char 2022-10-24 16:30 [PATCH] staging: rtl8192e: use explicitly signed char Jason A. Donenfeld 2022-10-25 6:19 ` Dan Carpenter @ 2022-10-25 17:22 ` Philipp Hortmann 1 sibling, 0 replies; 9+ messages in thread From: Philipp Hortmann @ 2022-10-25 17:22 UTC (permalink / raw) To: Jason A. Donenfeld, linux-kernel; +Cc: Greg Kroah-Hartman, linux-staging On 10/24/22 18:30, Jason A. Donenfeld wrote: > With char becoming unsigned by default, and with `char` alone being > ambiguous and based on architecture, signed chars need to be marked > explicitly as such. In this case, passing `char *extra` is part of the > iw API, and that extra is mostly intended to be somewhat opaque. So just > cast to `s8 *` for the sign test. This fixes warnings like: > > drivers/staging/rtl8192e/rtllib_softmac_wx.c:459 rtllib_wx_set_essid() warn: impossible condition '(extra[i] < 0) => (0-255 < 0)' > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: linux-staging@lists.linux.dev > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > drivers/staging/rtl8192e/rtllib_softmac_wx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > index f9589c5b62ba..4563e3b5bd47 100644 > --- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c > +++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c > @@ -456,7 +456,7 @@ int rtllib_wx_set_essid(struct rtllib_device *ieee, > } > > for (i = 0; i < len; i++) { > - if (extra[i] < 0) { > + if (((s8 *)extra)[i] < 0) { > ret = -1; > goto out; > } Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-10-25 17:43 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-24 16:30 [PATCH] staging: rtl8192e: use explicitly signed char Jason A. Donenfeld 2022-10-25 6:19 ` Dan Carpenter 2022-10-25 10:45 ` Greg Kroah-Hartman 2022-10-25 12:21 ` [PATCH v2] staging: rtl8192e: remove bogus ssid character sign test Jason A. Donenfeld 2022-10-25 12:34 ` Dan Carpenter 2022-10-25 17:35 ` Philipp Hortmann 2022-10-25 17:41 ` Greg KH 2022-10-25 17:43 ` Jason A. Donenfeld 2022-10-25 17:22 ` [PATCH] staging: rtl8192e: use explicitly signed char Philipp Hortmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox