Return-path: Received: from mail-gy0-f174.google.com ([209.85.160.174]:49651 "EHLO mail-gy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965069Ab2CTN61 convert rfc822-to-8bit (ORCPT ); Tue, 20 Mar 2012 09:58:27 -0400 Received: by ghrr11 with SMTP id r11so65319ghr.19 for ; Tue, 20 Mar 2012 06:58:26 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20120320134629.GD19742@tuxdriver.com> References: <1332192901-2500-1-git-send-email-mcgrof@frijolero.org> <1332223323.3329.0.camel@jlt3.sipsolutions.net> <1332227307.3329.2.camel@jlt3.sipsolutions.net> <20120320134629.GD19742@tuxdriver.com> From: "Luis R. Rodriguez" Date: Tue, 20 Mar 2012 06:58:06 -0700 Message-ID: (sfid-20120320_145844_503756_2649989A) Subject: Re: [PATCH] cfg80211: warn if db.txt is empty with CONFIG_CFG80211_INTERNAL_REGDB To: "John W. Linville" Cc: Johannes Berg , sfr@canb.auug.org.au, linux-wireless@vger.kernel.org, Youngsin Lee , Raja Mani , Senthil Kumar Balasubramanian , Vipin Mehta , yahuan@qca.qualcomm.com, jjan@qca.qualcomm.com, vthiagar@qca.qualcomm.com, henrykim@qualcomm.com, jouni@qca.qualcomm.com, athiruve@qca.qualcomm.com, cjkim@qualcomm.com, philipk@qca.qualcomm.com, sunnykim@qualcomm.com, sskwak@qualcomm.com, kkim@qualcomm.com, mattbyun@qualcomm.com, ryanlee@qualcomm.com, simbap@qualcomm.com, krislee@qualcomm.com, conner@qualcomm.com, hojinkim@qualcomm.com, honglee@qualcomm.com, johnwkim@qualcomm.com Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Tue, Mar 20, 2012 at 6:46 AM, John W. Linville 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 >> 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