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
next prev parent 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).