Thread (26 messages) 26 messages, 6 authors, 2008-10-02

Re: [PATCH] wireless: consolidate on a single escape_essid implementation

From: Dan Williams <hidden>
Date: 2008-09-25 16:49:13

On Thu, 2008-09-25 at 06:46 +0200, Johannes Berg wrote:
On Wed, 2008-09-24 at 18:15 -0400, John W. Linville wrote:
quoted
This is also an excuse to create the long rumored lib80211 module...
Let's also take the chance to clean up the mess Intel has, once again,
created here.
quoted
 		switch (info_element->id) {
 		case MFIE_TYPE_SSID:
-			if (ieee80211_is_empty_essid(info_element->data,
-						     info_element->len)) {
+			if (is_empty_essid(info_element->data,
+					   info_element->len)) {
 				network->flags |= NETWORK_EMPTY_ESSID;
 				break;
 			}
You can remove the whole NETWORK_EMPTY_ESSID flag and this whole test;
the code slightly below needs to take care to actually escape the SSID
for the debug output.
quoted
@@ -1411,7 +1412,7 @@ static int ieee80211_handle_assoc_resp(struct ieee80211_device *ieee, struct iee
 			network->mode |= IEEE_B;
 	}
 
-	if (ieee80211_is_empty_essid(network->ssid, network->ssid_len))
+	if (is_empty_essid(network->ssid, network->ssid_len))
 		network->flags |= NETWORK_EMPTY_ESSID;
drop the whole if
 
quoted
 
-	if (ieee80211_is_empty_essid(network->ssid, network->ssid_len))
+	if (is_empty_essid(network->ssid, network->ssid_len))
 		network->flags |= NETWORK_EMPTY_ESSID;
ditto.

that leaves once place using the NETWORK_EMPTY_ESSID flag, in
ieee80211_wx.c:

        /* Add the ESSID */
        iwe.cmd = SIOCGIWESSID;
        iwe.u.data.flags = 1;
        if (network->flags & NETWORK_EMPTY_ESSID) {
                iwe.u.data.length = sizeof("<hidden>");
                start = iwe_stream_add_point(info, start, stop,
                                             &iwe, "<hidden>");
        } else {
                iwe.u.data.length = min(network->ssid_len, (u8) 32);
                start = iwe_stream_add_point(info, start, stop,
                                             &iwe, network->ssid);
        }

which is of course *completely* *wrong*. There's no excuse for messing
up the values passed to userspace like that.
quoted
+const char *escape_essid(const char *essid, u8 essid_len)
+{
+	static char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
+	const char *s = essid;
+	char *d = escaped;
+
+	if (is_empty_essid(essid, essid_len)) {
+		memcpy(escaped, "<hidden>", sizeof("<hidden>"));
+		return escaped;
+	}
Once you've done the above, is_empty_essid is only used here. I'll leave
it to you whether you want to print <hidden> or not, I'd prefer if the
function was to just escape the ASCII-NULs as normal since then you can
actually distinguish the various forms of hidden SSIDs which might help.
Maybe that needs quotes around the escaped SSID then.
quoted
+	essid_len = min(essid_len, (u8) IW_ESSID_MAX_SIZE);
+	while (essid_len--) {
+		if (*s == '\0') {
+			*d++ = '\\';
+			*d++ = '0';
+			s++;
+		} else {
+			*d++ = *s++;
+		}
+	}
+	*d = '\0';
+	return escaped;
+}
+EXPORT_SYMBOL(escape_essid);
Also, we could take the chance to stop making Jean's mistake in calling
this thing the _E_SSID, there's no such thing, it's Jean's invention,
based on his misguided thought that this might somehow be less confusing
to the user.
+1

Dan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help