Merge lp:~sforshee/powerd/remove-dbus-pid-arg into lp:powerd

[Seth Forshee]

Remove pid arguments from dbus API

Remove pid arguments from dbus API

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:68
http://jenkins.qa.ubuntu.com/job/powerd-ci/81/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/powerd-saucy-armhf-ci/35
        deb: http://jenkins.qa.ubuntu.com/job/powerd-saucy-armhf-ci/35/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/powerd-ci/81/rebuild

review: Approve (continuous-integration)

[Matt Fischer (mfisch)]

looks good

review: Approve

=== modified file 'cli/powerd-cli.c'
--- cli/powerd-cli.c	2013-07-01 21:58:56 +0000
+++ cli/powerd-cli.c	2013-07-02 15:12:55 +0000
@@ -43,11 +43,13 @@
 
 GDBusProxy *powerd_proxy = NULL;
 char test_dbusname[128] = "";
-pid_t mypid;
+
+/* For tests */
+GDBusConnection *powerd_cli_bus;
+const char *powerd_cli_bus_name;
 
 struct PublicSysRequest {
     const gchar *owner;
-    pid_t pid;
     int state;
 };
 
@@ -55,14 +57,11 @@
     const gchar *owner;
     enum powerd_display_state state;
     enum powerd_display_brightness brightness;
-    pid_t pid;
     guint32 flags;
 };
 
 static GMainLoop *main_loop = NULL;
 static powerd_cookie_t main_cookie;
-static gboolean checkForPid(pid_t pid, int count, GArray *requests,
-        gboolean isSys);
 static gboolean checkForDbusName(const char *dbusname, int count,
         GArray *requests, gboolean isSys);
 static void sig_handler(int signum);
@@ -112,7 +111,7 @@
 
     ret = g_dbus_proxy_call_sync(powerd_proxy,
             "requestSysState",
-            g_variant_new("(ii)", state, mypid),
+            g_variant_new("(i)", state),
             G_DBUS_CALL_FLAGS_NONE,
             -1,
             NULL,
@@ -152,8 +151,7 @@
 
     ret = g_dbus_proxy_call_sync(powerd_proxy,
             "requestDisplayState",
-            g_variant_new("(iiiu)", pdr.state, pdr.brightness, pdr.pid,
-                pdr.flags),
+            g_variant_new("(iiu)", pdr.state, pdr.brightness, pdr.flags),
             G_DBUS_CALL_FLAGS_NONE,
             -1,
             NULL,
@@ -229,14 +227,12 @@
         g_error_free(error);
     }
     else {
-        g_variant_get(ret, "(a(sii))", &iter);
+        g_variant_get(ret, "(a(si))", &iter);
         g_message("Current Sys Requests:");
         while ((item = g_variant_iter_next_value (iter))) {
             g_variant_get_child (item, 0, "s", &psr.owner);
-            g_variant_get_child (item, 1, "i", &psr.pid);
-            g_variant_get_child (item, 2, "i", &psr.state);
-            g_message("    Owner: %s, Pid: %d, State: %d", psr.owner,
-                psr.pid, psr.state);
+            g_variant_get_child (item, 1, "i", &psr.state);
+            g_message("    Owner: %s, State: %d", psr.owner, psr.state);
             g_array_append_val(retarray, psr);
             g_variant_unref(item);
         }
@@ -269,17 +265,15 @@
         g_error_free(error);
     }
     else {
-        g_variant_get(ret, "(a(siiiu))", &iter);
+        g_variant_get(ret, "(a(siiu))", &iter);
         g_message("Current Display Requests:");
         while ((item = g_variant_iter_next_value (iter))) {
             g_variant_get_child(item, 0, "s", &pdr.owner);
-            g_variant_get_child(item, 1, "i", &pdr.pid);
-            g_variant_get_child(item, 2, "i", &pdr.state);
-            g_variant_get_child(item, 3, "i", &pdr.brightness);
-            g_variant_get_child(item, 4, "u", &pdr.flags);
-            g_message("    Owner: %s, Pid: %d, State: %d, Brightness: %d, "\
-                "Flags: %u", pdr.owner, pdr.pid, pdr.state, pdr.brightness,
-                pdr.flags);
+            g_variant_get_child(item, 1, "i", &pdr.state);
+            g_variant_get_child(item, 2, "i", &pdr.brightness);
+            g_variant_get_child(item, 3, "u", &pdr.flags);
+            g_message("    Owner: %s, State: %d, Brightness: %d, "\
+                "Flags: %u", pdr.owner, pdr.state, pdr.brightness, pdr.flags);
             g_array_append_val(retarray, pdr);
             g_variant_unref(item);
         }
@@ -373,7 +367,7 @@
     // Make sure clean-up didn't remove our request, note we don't pass
     // the child PID here.
     requests = listSysRequests();
-    do_test(checkForPid(mypid, 1, requests, TRUE));
+    do_test(checkForDbusName(powerd_cli_bus_name, 1, requests, TRUE));
     g_array_free(requests, TRUE);
 
     /* Close pid */
@@ -384,7 +378,7 @@
     //cleanup
     do_test(clearSysState(main_cookie) == TRUE);
     requests = listSysRequests();
-    do_test(checkForPid(mypid, 0, requests, TRUE));
+    do_test(checkForDbusName(powerd_cli_bus_name, 0, requests, TRUE));
     g_array_free(requests, TRUE);
 }
 
@@ -461,7 +455,7 @@
     // Make sure we have at least NUM_REQUESTS + 1
     requests = listSysRequests();
     do_test(requests->len >= TEST_NUM_SYS_REQUESTS+1);
-    do_test(checkForPid(mypid, TEST_NUM_SYS_REQUESTS+1, requests, TRUE));
+    do_test(checkForDbusName(powerd_cli_bus_name, TEST_NUM_SYS_REQUESTS+1, requests, TRUE));
     g_array_free(requests, TRUE);
 
     for (i = 0; i < TEST_NUM_SYS_REQUESTS; i++)
@@ -470,7 +464,7 @@
     // We should still have at least 1 request here
     requests = listSysRequests();
     do_test(requests->len >= 1);
-    do_test(checkForPid(mypid, 1, requests, TRUE));
+    do_test(checkForDbusName(powerd_cli_bus_name, 1, requests, TRUE));
     g_array_free(requests, TRUE);
 
     // Now try and freeing the same cookies again. They should all be
@@ -484,7 +478,7 @@
     // We should still have at least 1 request here
     requests = listSysRequests();
     do_test(requests->len >= 1);
-    do_test(checkForPid(mypid, 1, requests, TRUE));
+    do_test(checkForDbusName(powerd_cli_bus_name, 1, requests, TRUE));
     g_array_free(requests, TRUE);
 
     //cannot request the suspend state, this will fail
@@ -496,7 +490,7 @@
 
     //cleanup
     do_test(clearSysState(main_cookie) == TRUE);
-    do_test(checkForPid(mypid, 0, requests, TRUE));
+    do_test(checkForDbusName(powerd_cli_bus_name, 0, requests, TRUE));
 }
 
 static void
@@ -511,8 +505,6 @@
     // Hold active state request as long we're running tests
     requestSysState(POWERD_SYS_STATE_ACTIVE, &main_cookie);
 
-    pdr.pid = mypid;
-
     state = 0;
     brightness = 0;
     for (i = 0; i < TEST_NUM_DISP_REQUESTS; i++) {
@@ -531,7 +523,7 @@
     requests = listDisplayRequests();
     do_test(requests->len >= TEST_NUM_DISP_REQUESTS);
     // We should see our PID NUM_REQUESTS times
-    do_test(checkForPid(mypid, TEST_NUM_DISP_REQUESTS, requests, FALSE));
+    do_test(checkForDbusName(powerd_cli_bus_name, TEST_NUM_DISP_REQUESTS, requests, FALSE));
     g_array_free(requests, TRUE);
 
     for (i = 0; i < TEST_NUM_DISP_REQUESTS; i++)
@@ -539,7 +531,7 @@
 
     requests = listDisplayRequests();
     // We aren't holding anymore with this PID
-    do_test(checkForPid(mypid, 0, requests, FALSE));
+    do_test(checkForDbusName(powerd_cli_bus_name, 0, requests, FALSE));
     g_array_free(requests, TRUE);
 
     // Now try and freeing the same cookies again. They should all be
@@ -595,50 +587,10 @@
 
     //cleanup
     do_test(clearSysState(main_cookie) == TRUE);
-    do_test(checkForPid(mypid, 0, requests, TRUE));
-}
-
-// Check that pid appears count times in a list of sys/disp requests
-static gboolean
-checkForPid(pid_t pid,
-        int count,
-        GArray *requests,
-        gboolean isSys)
-{
-    int i;
-    int found = 0;
-    struct PublicSysRequest *psr;
-    struct PublicDispRequest *pdr;
-
-    if (requests == NULL) {
-        g_warning("requests is invalid");
-        return FALSE;
-    }
-
-    for (i=0; i<requests->len; i++)
-    {
-        if (isSys) {
-            psr = &g_array_index(requests, struct PublicSysRequest, i);
-            if (psr->pid == pid) {
-                found++;
-            }
-        }
-        else {
-            pdr = &g_array_index(requests, struct PublicDispRequest, i);
-            if (pdr->pid == pid) {
-                found++;
-            }
-        }
-    }
-
-    if (found != count) {
-        g_warning("Expected %d requests from PID, found %d", count, found);
-        return FALSE;
-    }
-    return TRUE;
-}
-
-// Check that pid appears count times in a list of sys/disp requests
+    do_test(checkForDbusName(powerd_cli_bus_name, 0, requests, TRUE));
+}
+
+// Check that dbus owner appears count times in a list of sys/disp requests
 static gboolean
 checkForDbusName(const char *dbusname,
         int count,
@@ -689,7 +641,6 @@
 static gboolean
 buildDisplayRequest(char **argv,
         int argc,
-        pid_t pid,
         struct PublicDispRequest *pdr)
 {
     int i;
@@ -834,9 +785,6 @@
         return -1;
     }
 
-    mypid = getpid();
-    g_debug("Running as pid %d\n", mypid);
-
     if (!strcmp(argv[1],"list")) {
         requests = listSysRequests();
         g_array_free(requests, TRUE);
@@ -862,7 +810,7 @@
             usage(argv[0]);
             return -1;
         }
-        if (buildDisplayRequest(argv,argc,mypid,&pdr) == FALSE) {
+        if (buildDisplayRequest(argv,argc,&pdr) == FALSE) {
             usage(argv[0]);
             return -1;
         }
@@ -902,9 +850,24 @@
         }
     }
     else if (!strcmp(argv[1],"test")) {
+        powerd_cli_bus = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL);
+        if (!powerd_cli_bus) {
+            fprintf(stderr, "Could not connect to message bus\n");
+            exit(-1);
+        }
+        powerd_cli_bus_name = g_dbus_connection_get_unique_name(powerd_cli_bus);
+        if (!powerd_cli_bus_name) {
+            fprintf(stderr, "Could not get unique bus name\n");
+            exit(-1);
+        }
+
         runDbusNameVanishTests(argv[0]);
         runSysTests();
         runDisplayTests();
+
+        g_object_unref(powerd_cli_bus);
+        powerd_cli_bus = NULL;
+        powerd_cli_bus_name = NULL;
     }
     else if (!strcmp(argv[1],"listen")) {
         listenForSignals(powerd_proxy);
 
=== modified file 'data/com.canonical.powerd.xml'
--- data/com.canonical.powerd.xml	2013-06-13 20:18:18 +0000
+++ data/com.canonical.powerd.xml	2013-07-02 15:12:55 +0000
@@ -6,7 +6,6 @@
     <!-- Functions -->
     <method name="requestSysState">
       <arg type="i" name="state" direction="in" />
-      <arg type="i" name="pid" direction="in" />
       <arg type="s" name="cookie" direction="out" />
     </method>
 
@@ -17,7 +16,6 @@
     <method name="requestDisplayState">
       <arg type="i" name="state" direction="in" />
       <arg type="i" name="brightness" direction="in" />
-      <arg type="i" name="pid" direction="in" />
       <arg type="u" name="flags" direction="in" />
       <arg type="s" name="cookie" direction="out" />
     </method>
@@ -35,11 +33,11 @@
 
     <!-- for debug/testing -->
     <method name="listSysRequests">
-      <arg type="a(sii)" name="requestList" direction="out" />
+      <arg type="a(si)" name="requestList" direction="out" />
     </method>
 
     <method name="listDisplayRequests">
-      <arg type="a(siiiu)" name="requestList" direction="out" />
+      <arg type="a(siiu)" name="requestList" direction="out" />
     </method>
 
     <!-- Signals -->
 
=== modified file 'src/display-request.c'
--- src/display-request.c	2013-07-01 21:23:32 +0000
+++ src/display-request.c	2013-07-02 15:12:55 +0000
@@ -145,7 +145,6 @@
     __remove_request(&ireq->req);
     __add_request(new_req);
 
-    /* Update all fields except the pid */
     ireq->req.state = new_req->state;
     ireq->req.brightness = new_req->brightness;
     ireq->req.flags = new_req->flags;
@@ -278,8 +277,7 @@
 
 gboolean handle_add_display_request(PowerdSource *obj,
                                     GDBusMethodInvocation *invocation,
-                                    int state, int brightness, int pid,
-                                    guint32 flags)
+                                    int state, int brightness, guint32 flags)
 {
     struct powerd_display_request req = {0,};
     const char *owner;
@@ -287,12 +285,11 @@
     int ret;
 
     owner = g_dbus_method_invocation_get_sender(invocation);
-    powerd_debug("%s from %s (%d): state %d brightness %d flags %#08x",
-                 __func__, owner, pid, state, brightness, flags);
+    powerd_debug("%s from %s: state %d brightness %d flags %#08x",
+                 __func__, owner, state, brightness, flags);
 
     req.state = state;
     req.brightness = brightness;
-    req.pid = pid;
     req.flags = flags;
 
     ret = __powerd_add_display_request(&req, owner);
@@ -387,12 +384,12 @@
     char cookie_str[UUID_STR_LEN];
 
     uuid_unparse(req->cookie, cookie_str);
-    powerd_debug("  owner: %s, pid: %d, cookie: %s, state: %d, brightness: %d, flags: %#08x",
-                 ireq->owner, req->pid, cookie_str, req->state,
-                 req->brightness, req->flags);
+    powerd_debug("  owner: %s, cookie: %s, state: %d, brightness: %d, flags: %#08x",
+                 ireq->owner, cookie_str, req->state, req->brightness,
+                 req->flags);
 
-    g_variant_builder_add(builder, "(siiiu)", ireq->owner, req->pid,
-                          req->state, req->brightness, req->flags);
+    g_variant_builder_add(builder, "(siiu)", ireq->owner, req->state,
+                          req->brightness, req->flags);
 }
 
 static int build_display_request_list(GVariantBuilder *builder)
@@ -409,13 +406,13 @@
     GVariantBuilder *builder;
     int count;
 
-    builder = g_variant_builder_new(G_VARIANT_TYPE("a(siiiu)"));
+    builder = g_variant_builder_new(G_VARIANT_TYPE("a(siiu)"));
     count = build_display_request_list(builder);
     if (count > 0) {
         list = g_variant_builder_end(builder);
     } else {
         g_variant_builder_clear(builder);
-        list = g_variant_new_array(G_VARIANT_TYPE("(siiiu)"), NULL, 0);
+        list = g_variant_new_array(G_VARIANT_TYPE("(siiu)"), NULL, 0);
     }
 
     tuple = g_variant_new_tuple(&list, 1);
 
=== modified file 'src/display.c'
--- src/display.c	2013-06-28 16:08:55 +0000
+++ src/display.c	2013-07-02 15:12:55 +0000
@@ -182,7 +182,7 @@
     case POWERD_DISPLAY_STATE_ON:
         if (actual_screen_state != POWERD_DISPLAY_STATE_ON) {
             powerd_debug("Requesting active state internally");
-            ret = request_sys_state_internal(POWERD_SYS_STATE_ACTIVE, getpid(),
+            ret = request_sys_state_internal(POWERD_SYS_STATE_ACTIVE,
                                              &internal_request_cookie, NULL);
             if (!ret)
                 powerd_warn("Request for active state failed");
 
=== modified file 'src/power-request.c'
--- src/power-request.c	2013-07-01 21:23:32 +0000
+++ src/power-request.c	2013-07-02 15:12:55 +0000
@@ -41,7 +41,6 @@
 struct SysStateRequest {
     gchar *owner;
     uuid_t cookie;
-    int pid;
     enum SysPowerStates state;
 };
 
@@ -163,12 +162,11 @@
 
     /* XXX: For debug, remove later */
     uuid_unparse(sr->cookie, cookie_str);
-    powerd_debug("  Owner: %s, Pid: %d, Cookie: %s, State: %s (%d)",
-                 sr->owner, sr->pid, cookie_str,
-                 state_to_string(sr->state), sr->state);
+    powerd_debug("  Owner: %s, Cookie: %s, State: %s (%d)",
+                 sr->owner, cookie_str, state_to_string(sr->state), sr->state);
 
     if (builder)
-        g_variant_builder_add(builder, "(sii)", sr->owner, sr->pid, sr->state);
+        g_variant_builder_add(builder, "(si)", sr->owner, sr->state);
 }
 
 /*
@@ -198,14 +196,14 @@
     GVariantBuilder *builder;
     guint count = 0;
 
-    builder = g_variant_builder_new(G_VARIANT_TYPE("a(sii)"));
+    builder = g_variant_builder_new(G_VARIANT_TYPE("a(si)"));
     count = list_sys_requests_internal(builder);
     if (count>0) {
         list = g_variant_builder_end(builder);
     }
     else {
         g_variant_builder_clear(builder);
-        list = g_variant_new_array(G_VARIANT_TYPE("(sii)"), NULL, 0);
+        list = g_variant_new_array(G_VARIANT_TYPE("(si)"), NULL, 0);
     }
     tuple = g_variant_new_tuple(&list,1);
     g_dbus_method_invocation_return_value(invocation, tuple);
@@ -225,7 +223,7 @@
 
 /* Note: owner is NULL for internal usage */
 gboolean
-request_sys_state_internal(int state, int pid, uuid_t *cookie, const gchar *owner)
+request_sys_state_internal(int state, uuid_t *cookie, const gchar *owner)
 {
     struct SysStateRequest *sr = NULL;
 
@@ -242,7 +240,6 @@
 
     sr = g_new(struct SysStateRequest, 1);
     sr->state = (enum SysPowerStates)state;
-    sr->pid = pid;
     if (owner) {
         sr->owner = g_strdup(owner);
     }
@@ -258,7 +255,7 @@
 }
 
 gboolean
-handle_request_sys_state (PowerdSource *obj, GDBusMethodInvocation *invocation, int state, int pid)
+handle_request_sys_state (PowerdSource *obj, GDBusMethodInvocation *invocation, int state)
 {
     gboolean retval;
     const char *owner;
@@ -266,10 +263,10 @@
     char ext_cookie[UUID_STR_LEN];
 
     owner = g_dbus_method_invocation_get_sender(invocation);
-    powerd_debug("handle_requestSysState from %s: %d - %s (%d)", owner, pid,
+    powerd_debug("handle_requestSysState from %s - %s (%d)", owner,
                  state_to_string(state), state);
 
-    retval = request_sys_state_internal(state, pid, &cookie, owner);
+    retval = request_sys_state_internal(state, &cookie, owner);
     if (retval == TRUE) {
         powerd_debug("handle_requestSysState - SUCCESS");
         uuid_unparse(cookie, ext_cookie);
 
=== modified file 'src/powerd-internal.h'
--- src/powerd-internal.h	2013-07-01 17:02:32 +0000
+++ src/powerd-internal.h	2013-07-02 15:12:55 +0000
@@ -37,7 +37,6 @@
     uuid_t cookie;
     enum powerd_display_state state;
     enum powerd_display_brightness brightness;
-    pid_t pid;
     guint32 flags;
 };
 
@@ -87,8 +86,7 @@
 void display_request_deinit(void);
 gboolean handle_add_display_request(PowerdSource *obj,
                                     GDBusMethodInvocation *invocation,
-                                    int state, int brightness, int pid,
-                                    guint32 flags);
+                                    int state, int brightness, guint32 flags);
 gboolean handle_update_display_request(PowerdSource *obj,
                                        GDBusMethodInvocation *invocation,
                                        const gchar *ext_cookie, int state,
@@ -104,13 +102,12 @@
 gboolean handle_list_sys_requests(PowerdSource *obj,
                                   GDBusMethodInvocation *invocation);
 gboolean handle_request_sys_state(PowerdSource *obj,
-                                  GDBusMethodInvocation *invocation,
-                                  int state, int pid);
+                                  GDBusMethodInvocation *invocation, int state);
 gboolean handle_clear_sys_state(PowerdSource *obj,
                                 GDBusMethodInvocation *invocation,
                                 gchar *cookie);
-gboolean request_sys_state_internal(int state, int pid,
-                                    uuid_t *cookie, const gchar *owner);
+gboolean request_sys_state_internal(int state, uuid_t *cookie,
+                                    const gchar *owner);
 gboolean clear_sys_state_internal(uuid_t cookie);
 void update_system_state(void);
 void power_request_init(void);
 
=== modified file 'src/powerd.cpp'
--- src/powerd.cpp	2013-07-01 17:02:32 +0000
+++ src/powerd.cpp	2013-07-02 15:12:55 +0000
@@ -107,7 +107,6 @@
     {0,},                                   /* cookie */
     POWERD_DISPLAY_STATE_ON,                /* state */
     POWERD_DISPLAY_BRIGHTNESS_BRIGHT,       /* brightness */
-    0,                                      /* pid */
     0                                       /* flags */
 };
 
@@ -116,7 +115,6 @@
     {0,},                                   /* cookie */
     POWERD_DISPLAY_STATE_DONT_CARE,         /* state */
     POWERD_DISPLAY_BRIGHTNESS_DONT_CARE,    /* brightness */
-    0,                                      /* pid */
     POWERD_DISPLAY_FLAG_USE_PROXIMITY       /* flags */
 };