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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.