Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reasoncodes

[Johannes Berg]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/