Service: Propagate error messages, check for invalid (NULL) service names, fix possible memory leak

3 files changed, 58 insertions, 14 deletions

diff --git a/src/service-dbus/LMI_ServiceProvider.c b/src/service-dbus/LMI_ServiceProvider.c
index e4fa3aa..372ad9a 100644
--- a/src/service-dbus/LMI_ServiceProvider.c
+++ b/src/service-dbus/LMI_ServiceProvider.c
@@ -47,10 +47,11 @@ static CMPIStatus LMI_ServiceEnumInstanceNames(
 {
     const char *ns = KNameSpace(cop);
     SList *slist = NULL;
+    char output[1024];
 
-    slist = service_find_all();
+    slist = service_find_all(output, sizeof(output));
     if (slist == NULL) {
-        KReturn2(_cb, ERR_FAILED, "Unable to enumerate instance names of LMI_Service");
+        KReturn2(_cb, ERR_FAILED, output);
     }
 
     for (int i = 0; i < slist->cnt; i++) {
@@ -103,11 +104,13 @@ static CMPIStatus LMI_ServiceGetInstance(
     const CMPIObjectPath* cop,
     const char** properties)
 {
+    char output[1024];
+    int res;
     LMI_Service w;
     LMI_Service_InitFromObjectPath(&w, _cb, cop);
 
     Service servicebuf = {0, };
-    if (service_get_properties(&servicebuf, w.Name.chars)) {
+    if ((res = service_get_properties(&servicebuf, w.Name.chars, output, sizeof(output))) == 0) {
         LMI_Service_Set_Status(&w, servicebuf.svStatus);
         free(servicebuf.svStatus);
         LMI_Service_Set_Started(&w, servicebuf.svStarted);
@@ -132,8 +135,10 @@ static CMPIStatus LMI_ServiceGetInstance(
 
         KReturnInstance(cr, w);
         KReturn(OK);
-    } else {
+    } else if (res == -2) { /* service of that name doesn't exist */
         KReturn(ERR_NOT_FOUND);
+    } else { /* some error occured when getting properties */
+        KReturn2(_cb, ERR_FAILED, output);
     }
 }
 
diff --git a/src/service-dbus/util/serviceutil.c b/src/service-dbus/util/serviceutil.c
index af1e7ff..70127fe 100644
--- a/src/service-dbus/util/serviceutil.c
+++ b/src/service-dbus/util/serviceutil.c
@@ -54,7 +54,9 @@ void service_free_slist(SList *slist)
     return;
 }
 
-SList *service_find_all(void)
+SList *service_find_all(
+    char *output,
+    int output_len)
 {
     GDBusProxy *manager_proxy = NULL;
     GVariantIter *arr = NULL;
@@ -71,6 +73,7 @@ SList *service_find_all(void)
     manager_proxy = g_dbus_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE,
         NULL, MANAGER_NAME, MANAGER_OP, MANAGER_INTERFACE, NULL, &error);
     if (!manager_proxy) {
+        strncpy(output, error->message, output_len);
         g_error_free(error);
         return NULL;
     }
@@ -79,6 +82,7 @@ SList *service_find_all(void)
     result = g_dbus_proxy_call_sync(manager_proxy, "ListUnitFiles", NULL,
         G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
     if (error) {
+        strncpy(output, error->message, output_len);
         g_error_free(error);
         g_object_unref(manager_proxy);
         return NULL;
@@ -89,6 +93,7 @@ SList *service_find_all(void)
     slist->name = malloc(MAX_SLIST_CNT * sizeof(char *));
     if (!slist->name) {
         free(slist);
+        g_object_unref(manager_proxy);
         return NULL;
     }
     slist->cnt = 0;
@@ -118,7 +123,9 @@ SList *service_find_all(void)
 
 int service_get_properties(
     Service *svc,
-    const char *service)
+    const char *service,
+    char *output,
+    int output_len)
 {
     GDBusProxy *manager_proxy = NULL;
     GDBusProxy *proxy = NULL;
@@ -128,14 +135,23 @@ int service_get_properties(
     GError *error = NULL;
     gchar *fragment_path = NULL, *unit_file_state = NULL;
     gchar *unit, *value_str;
+    char found = 0;
+
+    if (!service) {
+        strncpy(output, "Invalid service name", output_len);
+        return -1;
+    }
+
+    svc->svName = strdup(service);
+    if (!svc->svName) {
+        strncpy(output, "Insufficient memory", output_len);
+        return -1;
+    }
 
 #if !defined(GLIB_VERSION_2_36)
     g_type_init();
 #endif
 
-    svc->svName = strdup(service);
-    if (!svc->svName) return 0;
-
     manager_proxy = g_dbus_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE,
         NULL, MANAGER_NAME, MANAGER_OP, MANAGER_INTERFACE, NULL, &error);
     if (!manager_proxy) goto error;
@@ -149,6 +165,7 @@ int service_get_properties(
     g_variant_get(result, "(a(ss))", &arr);
     while (g_variant_iter_loop(arr, "(ss)", &fragment_path, &unit_file_state)) {
         if (strstr(fragment_path, service) && strcmp(strstr(fragment_path, service), service) == 0) {
+            found = 1;
             if (strncmp(unit_file_state, "enabled", 7) == 0)
                 svc->svEnabledDefault = ENABLED;
             if (strncmp(unit_file_state, "disabled", 8) == 0)
@@ -158,6 +175,12 @@ int service_get_properties(
     g_variant_iter_free(arr);
     arr = NULL;
 
+    if (!found) {
+        free(svc->svName);
+        g_object_unref(manager_proxy);
+        return -2;
+    }
+
     error = NULL;
     result = g_dbus_proxy_call_sync(manager_proxy, "LoadUnit", g_variant_new("(s)", service),
         G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
@@ -224,16 +247,19 @@ int service_get_properties(
     if (!svc->svStatus) goto error;
 
     g_object_unref(proxy);
-    return 1;
+    return 0;
 
 error:
     if (svc->svName) free(svc->svName);
-    if (error) g_error_free(error);
+    if (error) {
+        strncpy(output, error->message, output_len);
+        g_error_free(error);
+    }
     if (manager_proxy) g_object_unref(manager_proxy);
     if (proxy) g_object_unref(proxy);
     if (svc->svCaption) free(svc->svCaption);
 
-    return 0;
+    return -1;
 }
 
 unsigned int service_operation(
@@ -247,6 +273,16 @@ unsigned int service_operation(
     GError *error = NULL;
     GVariantBuilder *builder;
 
+    if (!service) {
+        strncpy(output, "Invalid service name", output_len);
+        return -1;
+    }
+
+    if (!method) {
+        strncpy(output, "Invalid method name", output_len);
+        return -1;
+    }
+
 #if !defined(GLIB_VERSION_2_36)
     g_type_init();
 #endif
@@ -254,6 +290,7 @@ unsigned int service_operation(
     manager_proxy = g_dbus_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE,
         NULL, MANAGER_NAME, MANAGER_OP, MANAGER_INTERFACE, NULL, &error);
     if (!manager_proxy) {
+        strncpy(output, error->message, output_len);
         g_error_free(error);
         return -1;
     }
@@ -270,6 +307,7 @@ unsigned int service_operation(
                 G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
         }
         if (error) {
+            strncpy(output, error->message, output_len);
             g_error_free(error);
             g_object_unref(manager_proxy);
             return -1;
@@ -278,6 +316,7 @@ unsigned int service_operation(
         result = g_dbus_proxy_call_sync(manager_proxy, method, g_variant_new("(ss)", service, "replace"),
             G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
         if (error) {
+            strncpy(output, error->message, output_len);
             g_error_free(error);
             g_object_unref(manager_proxy);
             return -1;
diff --git a/src/service-dbus/util/serviceutil.h b/src/service-dbus/util/serviceutil.h
index d1ed1be..4b54aaf 100644
--- a/src/service-dbus/util/serviceutil.h
+++ b/src/service-dbus/util/serviceutil.h
@@ -51,9 +51,9 @@ typedef struct _Service Service;
 typedef struct _SList SList;
 
 void service_free_slist(SList *slist);
-SList *service_find_all(void);
+SList *service_find_all(char *output, int output_len);
 
-int service_get_properties(Service *svc, const char *service);
+int service_get_properties(Service *svc, const char *service, char *output, int output_len);
 
 unsigned int service_operation(const char *service, const char *method, char *output, int output_len);