Bug 26750

+      /* messages will be NULL, we can just continue */

This is probably fine in reality, but I think it'd be best to at least add the documentation to tpl_log_manager_get_messages_for_date_async_finish() that guarantees its return value is NULL if there was any error.

Preferably, I'd also set messages to NULL just below that comment just to be explict and safer (and update the comment according).


 }
 
+static void
+_lookup_next_date (RecentMessagesContext *ctx)
+{

Telepathy Style has two blank lines between function declarations.


+    }
+
+}

(in _lookup_next_date()): extraneous blank line


+  ctx->ptr = g_list_last (ctx->dates);

Since this relies upon the chronological ordering of the return value of tpl_log_manager_get_dates(), could you update the documentation for that function to make that guarantee?


+  if (ctx->dates == NULL)
+    {
+      error = g_error_new_literal (TPL_DBUS_SERVICE_ERROR,
+          TPL_DBUS_SERVICE_ERROR_FAILED, "Error during date list retrieving, "
+          "probably the account path or the identifier does not exist");
+      dbus_g_method_return_error (ctx->context, error);
+      goto out;
+    }
+
+  _lookup_next_date (ctx);
+
+out:
+
+  g_clear_error (&error);
+}

Looks like you're leaking the ctx and its allocated members in case ctx->dates is NULL.


If you agree with the changes, feel free to just make them and merge.

(In reply to comment #1)
> +      /* messages will be NULL, we can just continue */
> 
> Preferably, I'd also set messages to NULL just below that comment just to be
> explict and safer (and update the comment according).

Yeah. Good call.

> +  ctx->ptr = g_list_last (ctx->dates);
> 
> Since this relies upon the chronological ordering of the return value of
> tpl_log_manager_get_dates(), could you update the documentation for that
> function to make that guarantee?

The function builds sorted lists, but we actually intend to update the function at some point to allow the list to be presorted in either direction (based on an explicit enum). Will leave it for now.

> Looks like you're leaking the ctx and its allocated members in case ctx->dates
> is NULL.

Good catch.

> If you agree with the changes, feel free to just make them and merge.

Thanks. Done.