linux-kernel - Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1392030546.4128.11.camel@jlt4.sipsolutions.net>
Date:	Mon, 10 Feb 2014 12:09:06 +0100
From:	Johannes Berg <johannes@...solutions.net>
To:	Joe Perches <joe@...ches.com>
Cc:	Calvin Owens <jcalvinowens@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	"John W. Linville" <linville@...driver.com>,
	linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason
 codes

On Wed, 2014-02-05 at 20:44 -0800, Joe Perches wrote:

> Perhaps use a more common kernel style
> 
> struct ieee80211_reason_descriptions {
> 	u16		code;
> 	const char *	desc;
> }
> 
> and enumerate the reason codes with #defines and use a
> macro to populate the descriptions
> 
> #define IEEE80211_REASON_RESERVED		0
> #define IEEE80211_REASON_UNSPECIFIED		1
> 
> etc.
> 
> #define POPULATE_IEEE_REASON(code)			\
> 	{.code = IEEE80211_REASON_##code, .desc = #code}
> 
> static const struct ieee80211_reason_descriptions reasons[] = {
> 	POPULATE_IEEE_REASON(RESERVED),
> 	POPULATE_IEEE_REASON(UNSPECIFIED),
> 	[etc...]
> };
> 
> So this function becomes something like:
> 
> const char *ieee80211_get_reason_code_string(u16 reason_code)
> {
> 	int i;
> 
> 	for (i = 0; i < ARRAY_SIZE(reasons); i++) {
> 		if (reasons[i].code == reason_code)
> 			return reasons[i].desc;
> 	}

Isn't it more efficient to just let the compiler generate it with a big
switch() statement? AFAICT gcc will typically generate code like
Calvin's original hand-rolled code and/or big lookup tables anyway, if
faced with something like

switch (reason) {
case 17: return "asdf";
... // all the others
default: return "<unknown>";
};

In any case, I agree with you and Jouni about leaving the number -
that's certainly needed - but I'm willing to merge a clean patch like
this too.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/