Optimize jabber_id_new()

Paul Aurich paul at darkrain42.org
Tue Jun 30 14:04:01 EDT 2009


And Mark Doliner spoke on 06/30/2009 01:31 AM, saying:
<snip>
> In other words: How does everyone feel about the attached patch?

Like Daniel and Ethan, I think this is fine, with a few comments (see below).

> 
> -Mark
> 

+	for (c = str; *c != '\0'; c++) {
+		switch (*c) {
+			case '@':
+				if (!at)
+					at = c;
+				break;
+			case '/':
+				if (!at) {
+					/*
+					 * If at is NULL then c is still pointing to the node
+					 * name, and node names can not contain forward slashes
+					 */
+					return NULL;
+				}
+				if (!slash)
+					slash = c;
+				break;
+			default:

This introduces a few corner cases where invalid JIDs would get through (I
think something like "foo@@bar.com"). That's probably not too much of an
issue. It's also technically valid to have a JID of the form
"domain/resource", which this would fail to validate (again, not actually
too big of an issue).

I'd envision something like this (assuming enum { NODE, DOMAIN, RESOURCE }
state), which I don't think should add much overhead, but addresses both of
those issues:

case '@':
	if (state == NODE && !at) {
		at = c;
		state = DOMAIN;
	} else if (state == DOMAIN) {
		/* An '@' isn't valid in the domain portion of a JID */
		return NULL;
	}
	break;

case '/':
	if (!slash) {
		slash = c;
		state = RESOURCE;

		if (*(c+1) == '\0')
			/*
			 * A JID containing '/' cannot have a 0-length
			 * resource.
			 */
			return NULL;
	}
	break;

For the needs_validation == FALSE case, I would recommend moving to
g_ascii_strdown().

~Paul




More information about the Devel mailing list