| Message ID | 20191127054358.GA59549@LGEARND20B15 (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 37bc6c72f5b76f3c6ae00e3001d3187100fbeb1b |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmsmac: Remove always false 'channel < 0' statement | expand |
On 27.11.2019 8:43, Austin Kim wrote: > As 'channel' is declared as u16, the following statement is always false. > channel < 0 > > So we can remove unnecessary 'always false' statement. It's an expression, not a statement. > Signed-off-by: Austin Kim <austindh.kim@gmail.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c > index 3f09d89..7f2c15c 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c > @@ -5408,7 +5408,7 @@ int brcms_c_set_channel(struct brcms_c_info *wlc, u16 channel) > { > u16 chspec = ch20mhz_chspec(channel); > > - if (channel < 0 || channel > MAXCHANNEL) > + if (channel > MAXCHANNEL) > return -EINVAL; > > if (!brcms_c_valid_chanspec_db(wlc->cmi, chspec)) MBR, Sergei
2019년 11월 27일 (수) 오후 7:48, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>님이 작성: > > On 27.11.2019 8:43, Austin Kim wrote: > > > As 'channel' is declared as u16, the following statement is always false. > > channel < 0 > > > > So we can remove unnecessary 'always false' statement. > > It's an expression, not a statement. > According to below link, it is okay to use 'statement' in above case. https://en.wikipedia.org/wiki/Statement_(computer_science) Why don't you show your opition about patch rather than commit message? Thanks, Austin Kim > > Signed-off-by: Austin Kim <austindh.kim@gmail.com> > > --- > > drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c > > index 3f09d89..7f2c15c 100644 > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c > > @@ -5408,7 +5408,7 @@ int brcms_c_set_channel(struct brcms_c_info *wlc, u16 channel) > > { > > u16 chspec = ch20mhz_chspec(channel); > > > > - if (channel < 0 || channel > MAXCHANNEL) > > + if (channel > MAXCHANNEL) > > return -EINVAL; > > > > if (!brcms_c_valid_chanspec_db(wlc->cmi, chspec)) > > MBR, Sergei >
Austin Kim <austindh.kim@gmail.com> writes: > 2019년 11월 27일 (수) 오후 7:48, Sergei Shtylyov > <sergei.shtylyov@cogentembedded.com>님이 작성: >> >> On 27.11.2019 8:43, Austin Kim wrote: >> >> > As 'channel' is declared as u16, the following statement is always false. >> > channel < 0 >> > >> > So we can remove unnecessary 'always false' statement. >> >> It's an expression, not a statement. >> > > According to below link, it is okay to use 'statement' in above case. > https://en.wikipedia.org/wiki/Statement_(computer_science) I don't have time to start arguing about this, and I'm no C language lawyer either, but all I say is that I agree with Sergei here. > Why don't you show your opition about patch rather than commit message? But this comment is not ok. Patch review (including commit logs) is the core principle of upstream development so you need to have an open mind for all comments, even the ones you don't like.
2019년 11월 27일 (수) 오후 10:35, Kalle Valo <kvalo@codeaurora.org>님이 작성: > > Austin Kim <austindh.kim@gmail.com> writes: > > > 2019년 11월 27일 (수) 오후 7:48, Sergei Shtylyov > > <sergei.shtylyov@cogentembedded.com>님이 작성: > >> > >> On 27.11.2019 8:43, Austin Kim wrote: > >> > >> > As 'channel' is declared as u16, the following statement is always false. > >> > channel < 0 > >> > > >> > So we can remove unnecessary 'always false' statement. > >> > >> It's an expression, not a statement. > >> > > > > According to below link, it is okay to use 'statement' in above case. > > https://en.wikipedia.org/wiki/Statement_(computer_science) > > I don't have time to start arguing about this, and I'm no C language > lawyer either, but all I say is that I agree with Sergei here. Thanks for your opinion. I will use 'expression' rather than 'statement' when I upstream similar patch later. > > > Why don't you show your opition about patch rather than commit message? > > But this comment is not ok. Patch review (including commit logs) is the > core principle of upstream development so you need to have an open mind > for all comments, even the ones you don't like. Oh! I Agreed. If I were you, I would leave similar comment. Thanks, Austin Kim > > -- > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Austin Kim <austindh.kim@gmail.com> wrote: > As 'channel' is declared as u16, the following expression is always false. > channel < 0 > > So we can remove unnecessary 'always false' statement. > > Signed-off-by: Austin Kim <austindh.kim@gmail.com> Patch applied to wireless-drivers-next.git, thanks. 37bc6c72f5b7 brcmsmac: Remove always false 'channel < 0' statement
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c index 3f09d89..7f2c15c 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c @@ -5408,7 +5408,7 @@ int brcms_c_set_channel(struct brcms_c_info *wlc, u16 channel) { u16 chspec = ch20mhz_chspec(channel); - if (channel < 0 || channel > MAXCHANNEL) + if (channel > MAXCHANNEL) return -EINVAL; if (!brcms_c_valid_chanspec_db(wlc->cmi, chspec))
As 'channel' is declared as u16, the following statement is always false. channel < 0 So we can remove unnecessary 'always false' statement. Signed-off-by: Austin Kim <austindh.kim@gmail.com> --- drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)