Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] brcmsmac: Remove always false 'channel < 0' statement
@ 2019-11-27  5:43 Austin Kim
  2019-11-27 10:48 ` Sergei Shtylyov
  0 siblings, 1 reply; 5+ messages in thread
From: Austin Kim @ 2019-11-27  5:43 UTC (permalink / raw)
  To: arend.vanspriel, franky.lin, hante.meuleman, chi-hsien.lin,
	wright.feng, kvalo, davem
  Cc: linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	netdev, linux-kernel, austindh.kim

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(-)

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))
-- 
2.6.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] brcmsmac: Remove always false 'channel < 0' statement
  2019-11-27  5:43 [PATCH] brcmsmac: Remove always false 'channel < 0' statement Austin Kim
@ 2019-11-27 10:48 ` Sergei Shtylyov
  2019-11-27 13:02   ` Austin Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2019-11-27 10:48 UTC (permalink / raw)
  To: Austin Kim, arend.vanspriel, franky.lin, hante.meuleman,
	chi-hsien.lin, wright.feng, kvalo, davem
  Cc: linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	netdev, linux-kernel

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] brcmsmac: Remove always false 'channel < 0' statement
  2019-11-27 10:48 ` Sergei Shtylyov
@ 2019-11-27 13:02   ` Austin Kim
  2019-11-27 13:35     ` Kalle Valo
       [not found]     ` <0101016ead12c253-18d4624e-98eb-4252-ba3a-fabf74d831f2-000000@us-west-2.amazonses.com>
  0 siblings, 2 replies; 5+ messages in thread
From: Austin Kim @ 2019-11-27 13:02 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: arend.vanspriel, franky.lin, hante.meuleman, chi-hsien.lin,
	wright.feng, Kalle Valo, davem, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev, linux-kernel

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] brcmsmac: Remove always false 'channel < 0' statement
  2019-11-27 13:02   ` Austin Kim
@ 2019-11-27 13:35     ` Kalle Valo
       [not found]     ` <0101016ead12c253-18d4624e-98eb-4252-ba3a-fabf74d831f2-000000@us-west-2.amazonses.com>
  1 sibling, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2019-11-27 13:35 UTC (permalink / raw)
  To: Austin Kim
  Cc: Sergei Shtylyov, arend.vanspriel, franky.lin, hante.meuleman,
	chi-hsien.lin, wright.feng, davem, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev, linux-kernel

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.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] brcmsmac: Remove always false 'channel < 0' statement
       [not found]     ` <0101016ead12c253-18d4624e-98eb-4252-ba3a-fabf74d831f2-000000@us-west-2.amazonses.com>
@ 2019-11-27 22:21       ` Austin Kim
  0 siblings, 0 replies; 5+ messages in thread
From: Austin Kim @ 2019-11-27 22:21 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Sergei Shtylyov, arend.vanspriel, franky.lin, hante.meuleman,
	chi-hsien.lin, wright.feng, davem, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27  5:43 [PATCH] brcmsmac: Remove always false 'channel < 0' statement Austin Kim
2019-11-27 10:48 ` Sergei Shtylyov
2019-11-27 13:02   ` Austin Kim
2019-11-27 13:35     ` Kalle Valo
     [not found]     ` <0101016ead12c253-18d4624e-98eb-4252-ba3a-fabf74d831f2-000000@us-west-2.amazonses.com>
2019-11-27 22:21       ` Austin Kim

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git