On Tue, Mar 20, 2012 at 6:46 AM, John W. Linville
<linville@xxxxxxxxxxxxx> wrote:
> On Tue, Mar 20, 2012 at 12:13:34AM -0700, Luis R. Rodriguez wrote:
>> On Tue, Mar 20, 2012 at 12:08 AM, Johannes Berg
>> <johannes@xxxxxxxxxxxxxxxx> wrote:
>> > On Tue, 2012-03-20 at 00:00 -0700, Luis R. Rodriguez wrote:
>> >
>> >> >> net/wireless/reg.c | 12 ++++++++++++
>> >> >> 1 file changed, 12 insertions(+)
>> >> >>
>> >> >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> >> >> index e9a0ac8..85f51b3 100644
>> >> >> --- a/net/wireless/reg.c
>> >> >> +++ b/net/wireless/reg.c
>> >> >> @@ -388,7 +388,18 @@ static void reg_regdb_query(const char *alpha2)
>> >> >>
>> >> >> schedule_work(®_regdb_work);
>> >> >> }
>> >> >> +
>> >> >> +/* Feel free to add any other sanity checks here */
>> >> >> +static void reg_regdb_size_check(void)
>> >> >> +{
>> >> >> +#ifdef CONFIG_CFG80211_REG_DEBUG
>> >> >> + BUILD_BUG_ON(!reg_regdb_size);
>> >> >> +#else
>> >> >> + WARN_ONCE(!reg_regdb_size, "db.txt is empty, you should update it...");
>> >> >> +#endif
>> >> >
>> >> > That ifdef seems a bit pointless? If anything I would have expected it
>> >> > the other way around since the BUILD_BUG_ON compiles to nothing?
>> >>
>> >> As I tested it, the BUILD_BUG_ON() forces a compile failure.
>> >
>> > Right. Why would you not want that always?
>>
>> Ah well that is a question for you, John and Stephen. I didn't use
>> that *always* given that it would break random build testing whenever
>> CFG80211_INTERNAL_REGDB was enabled, given that it requires a manual
>> cp of db.txt from wireless-regdb. With this it would only break build
>> testing with debugging cfg80211 regulatory, both
>> CFG80211_INTERNAL_REGDB and CONFIG_CFG80211_REG_DEBUG enabled. If you
>> guys are happy with it then so am I -- I prefer it, just didn't want
>> any surprises or anyone reporting an unexpected build breakage later.
>
> My first inclination is like Johannes, just break the build in this
> case. But the random build test breakage could be an annoyance for
> the folks doing that. I don't suppose there is any Kconfig magic
> that would prevent selecting CFG80211_INTERNAL_REGDB unless there is
> a non-zero db.txt file?
That'd be really cool.
> Also, I guess that an empty db.txt just forces the user back to
> the build it "world" domain. While that is less than ideal, it is
> sufficient for some minimal functionality. So breaking the build in
> that case seems like bad form too.
>
> Maybe a runtime warning is sufficient?
That's what I have when CONFIG_CFG80211_REG_DEBUG is disabled in the
patch. My only concern with this is that so far based on experience a
lot of debugging may have undergone by then. But I'm also OK with this
as well to avoid build breakage suffering. Will send a v2 that just
does this.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
