[Bug 35239] Filter out illegal user names as per RFC 2812

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=35239

--- Comment #14 from Debarshi Ray <debarshi.ray at gmail.com> 2011-03-25 09:27:10 PDT ---
(In reply to comment #12)

> Review of attachment 44854 [details]:
> 
> ::: src/idle-connection.c
> @@ +858,3 @@
> +    IdleConnection *conn = IDLE_CONNECTION(user_data);
> +    GHashTable *details = NULL;
> +    TpConnectionStatus status = conn->parent.status;
> 
> If status is already TP_CONNECTION_STATUS_DISCONNECTED, then this function
> should early-return.

Done.

> @@ +875,3 @@
> +        server_msg = g_new0(gchar, length + 1);
> +        strncpy(server_msg, begin, length);
> +    const gchar *end = strrchr(msg, ')');
> 
> I drew some boxes on some paper to check this code. :) I think this would be
> clearer and equivalent:
> 
>   begin++;
>   end--;
>   length = end - begin;
>   server_msg = g_strndup (server_msg + begin, length);

Right. But the length should still be end - begin + 1. Else we will loose a
character.

> @@ +876,3 @@
> +        strncpy(server_msg, begin, length);
> +        server_msg[length] = '\0';
> +    const gchar *error_name = (status == TP_CONNECTION_STATUS_CONNECTING) ?
> TP_ERROR_STR_AUTHENTICATION_FAILED : TP_ERROR_STR_NETWORK_ERROR;
> 
> You should g_free (server_msg); here. ... ah, I see that you free it later. It
> would be clearer, I think, to define gchar *server_msg only within this block,
> and free it right after the call to tp_asv_new().

Done.

> @@ +879,3 @@
> +    }
> +
> +    gchar *server_msg = NULL;
> 
> As you noticed on IRC, the hash table needs to be unreffed if it's not NULL.

Done.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.