netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jean-Mickael Guerin <jean-mickael.guerin@6wind.com>
To: Ivo Van Doorn <ivdoorn@gmail.com>
Cc: linville@tuxdriver.com, netdev@vger.kernel.org
Subject: Re: drivers/net/wireless/d80211: Check configuration type in hw->config_interface.
Date: Thu, 20 Jul 2006 00:05:37 +0200	[thread overview]
Message-ID: <44BEACB1.70900@6wind.com> (raw)
In-Reply-To: <a32f33a40607191413k5cab2766u4fc4ee8b4f6c4088@mail.gmail.com>


>
> When submitting a patch, please state what drivers you are really 
> changing,
> your mail subject suggested a change to the dscape stack itself.
> Since I look at those patches at an irregular basis, while I am always
> checking for rt2x00 related patches I could have missed this one.
> Especially when you don't CC the driver maintainers about the patch
> for their drivers.

> Also I think most people would prefer if you split up patches when it
> affects multiple drivers,
> in this case rt2x00 and adm8211.
what I meant with this title is this patch is for all d80211-based 
current drivers of Linville's wireless-dev,
but I see your point...

>
>> diff --git a/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
>> b/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
>> index 946cf86..1d45851 100644
>> --- a/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
>> +++ b/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
>> @@ -1877,7 +1877,9 @@ rt2400pci_config_interface(struct net_de
>>      if (rt2x00pci->type == IEEE80211_IF_TYPE_MNTR)
>>          return 0;
>>
>> -    rt2400pci_config_bssid(rt2x00pci, conf->bssid);
>> +    if (conf->type == IEEE80211_IF_TYPE_STA ||
>> +        conf->type == IEEE80211_IF_TYPE_IBSS)
>> +        rt2400pci_config_bssid(rt2x00pci, conf->bssid);
>
> Should rt2400pci_config_bssid not simply run a check to see if the
> bssid argument is valid?
> This would prevent the risk of having a similar problem when the
> function is called from somewhere else as well.
>
I was thinking a function named xxx_config_bssid() assumes a valid bssid 
pointer,
- I would even add BUG_ON(conf->bssid==NULL) in xxx_config_bssid().
And hw->interface already already some tests with conf->type.
And net/d80211 uses same kind of test before setting conf->bssid.

anyway I don't mind you make the patch the other way.
> Same comment applies for rt2500pci and rt61pci.
>
> Any particular reason why you applied this change to PCI drivers in
> rt2x00 only and not to the USB drivers?
>
likely because I have only a pci card :-)
USB drivers needs same change too.

Thanks,

Jean-Mickael

  reply	other threads:[~2006-07-19 22:05 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-19 20:26 [PATCH] drivers/net/wireless/d80211: Check configuration type in hw->config_interface Jean-Mickael Guerin
2006-07-19 21:13 ` Ivo Van Doorn
2006-07-19 22:05   ` Jean-Mickael Guerin [this message]
2006-07-20  1:07 ` [PATCH] " Michael Wu
2006-07-20  8:54   ` Jiri Benc
2006-07-20  8:46 ` Jiri Benc
2006-07-20 16:19   ` Ivo Van Doorn
2006-07-20 16:47     ` Ivo Van Doorn

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=44BEACB1.70900@6wind.com \
    --to=jean-mickael.guerin@6wind.com \
    --cc=ivdoorn@gmail.com \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).