Code review comment for lp:~ted/libdbusmenu/lp940651

Revision history for this message
Charles Kerr (charlesk) wrote :

Approved because the code looks fine, but I have a two minor suggestions for cleanup:

1. g_clear_error(&localerror) instead of "if (localerror != NULL) { g_error_free(localerror); }

2. The "callback all the folks we can find" section can be simpler:

    properties_listener_t * listener = find_listener(listeners, 0, id);
    if (listener == NULL) {
        g_warning("Unable to find listener for ID %d", id);
    } else if (listener->replied) {
        g_warning("Odd, we've already replied to the listener on ID %d", id);
    } else {
        GVariant * properties = g_variant_get_child_value(child, 1);
        listener->callback(properties, NULL, listener->user_data);
        listener->replied = TRUE;
        g_variant_unref(properties);
    }

    g_variant_unref(child);

review: Approve

« Back to merge proposal