[Bug 37145] Replace low level network code with GIO

[bugzilla-daemon at freedesktop.org]

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

--- Comment #18 from Will Thompson <will.thompson at collabora.co.uk> 2011-09-09 07:38:29 PDT ---
+typedef struct _CloseAsyncData CloseAsyncData;
+typedef struct _WriteAsyncData WriteAsyncData;

Vestigal typedefs!

+    if (g_input_stream_read_finish(input_stream, res, &error) == -1) {
+        IDLE_DEBUG("g_input_stream_read failed: %s", error->message);
+        g_error_free(error);
+        goto disconnect;
     }

+    if (socket_connection == NULL) {
+        IDLE_DEBUG("g_socket_client_connect_to_host failed: %s",
error->message);
+        g_error_free(error);
+        change_state(conn, SERVER_CONNECTION_STATE_NOT_CONNECTED,
SERVER_CONNECTION_STATE_REASON_ERROR);

It's a shame that we lose the errors in these cases. Ah, I see that in the
second case, you later hooked up the message.

+    memset(priv->input_buffer, '\0', sizeof(priv->input_buffer));
+    g_input_stream_read_async (input_stream, &priv->input_buffer,
sizeof(priv->input_buffer) - 1, G_PRIORITY_DEFAULT, priv->cancellable,
_input_stream_read_ready, conn);

This is duplicated, it'd be nice to have a helper function for this.

+    g_cancellable_reset(priv->cancellable);
+    g_object_ref(conn);
+    g_socket_client_connect_to_host_async(priv->socket_client, priv->host,
priv->port, priv->cancellable, _connect_to_host_ready, conn);

I don't see why you're resetting the cancellable here. If it was in the
cancelled state, surely we shouldn't be trying to connect at all.

  IdleConnection: Initialize priv->conn

+    priv->conn = NULL;

This shouldn't be needed: GObject zeroes out your structures when it allocates
them.

 static void _connect_to_host_ready(GObject *source_object, GAsyncResult *res,
gpointer user_data) {
     GSocketClient *socket_client = G_SOCKET_CLIENT(source_object);
-    IdleServerConnection *conn = IDLE_SERVER_CONNECTION(user_data);
+    GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT(user_data);
+    IdleServerConnection *conn =
IDLE_SERVER_CONNECTION(g_async_result_get_source_object(G_ASYNC_RESULT(result)));
…
         change_state(conn, SERVER_CONNECTION_STATE_NOT_CONNECTED,
SERVER_CONNECTION_STATE_REASON_ERROR);
-        g_object_unref(conn);
-        return;
+        goto cleanup;
…
+    g_cancellable_reset(priv->cancellable);
+    g_object_ref(conn);
     g_input_stream_read_async (input_stream, &priv->input_buffer,
sizeof(priv->input_buffer) - 1, G_PRIORITY_DEFAULT, priv->cancellable,
_input_stream_read_ready, conn);

g_async_result_get_source_object() returns a reference, so if I'm not mistaken,
the removal of the call to g_object_unref and the addition of the call to
g_object_ref mean you will leak a reference.

IdleConnection: No need to disconnect if connection can not be made

I don't understand this change: if _iface_shut_down is called while we're in
state CONNECTING, surely we do want to tell the IdleServerConnection to give
up?

These are in existing code that you just moved around, feel free to ignore
them:

+    if (!priv->realname || !priv->realname[0]) {

tp_str_empty()

+        if (g_realname && g_realname[0] && strcmp(g_realname, "Unknown"))

!tp_strdiff (g_realname, "Unknown")

-- 
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.