capabilities plugin: address review comments

Change-Id: I16b74ee6511ab1a1db8c7f3f0f515212286eb34e
Signed-off-by: Kamil Wiatrowski <kamilx.wiatrowski@intel.com>

diff --git a/src/capabilities.c b/src/capabilities.c
index 29bba0433968285486e3d08c831ad1cf4aad9137..5c27e6a3a1eaf0016b69d664795fd97f99397c47 100644 (file)
--- a/src/capabilities.c
+++ b/src/capabilities.c
@@ -46,7 +46,7 @@ typedef struct dmi_type_name_s {
   const char *name;
 } dmi_type_name_t;
 
-static char *g_cap_string = NULL;
+static char *g_cap_json = NULL;
 static char *httpd_host = NULL;
 static unsigned short httpd_port = 9104;
 static struct MHD_Daemon *httpd;
@@ -127,13 +127,13 @@ static int cap_get_dmi_variables(json_t *parent, const dmi_type type,
       DEBUG("    %s:%s", reader.name, reader.value);
       attributes = NULL;
       if (entries == NULL) {
-        ERROR(CAP_PLUGIN ": unexpected dmi output format");
+        ERROR(CAP_PLUGIN ": unexpected dmi output format.");
         dmi_reader_clean(&reader);
         return -1;
       }
       if (json_object_set_new(entries, reader.name,
                               json_string(reader.value))) {
-        ERROR(CAP_PLUGIN ": Failed to set json entry.");
+        ERROR(CAP_PLUGIN ": Failed to set json object for entries.");
         dmi_reader_clean(&reader);
         return -1;
       }
@@ -142,7 +142,7 @@ static int cap_get_dmi_variables(json_t *parent, const dmi_type type,
     case DMI_ENTRY_LIST_NAME:
       DEBUG("    %s:", reader.name);
       if (entries == NULL) {
-        ERROR(CAP_PLUGIN ": unexpected dmi output format");
+        ERROR(CAP_PLUGIN ": unexpected dmi output format.");
         dmi_reader_clean(&reader);
         return -1;
       }
@@ -153,7 +153,8 @@ static int cap_get_dmi_variables(json_t *parent, const dmi_type type,
         return -1;
       }
       if (json_object_set_new(entries, reader.name, attributes)) {
-        ERROR(CAP_PLUGIN ": Failed to set json entry.");
+        ERROR(CAP_PLUGIN ": Failed to set json object for entry %s.",
+              reader.name);
         dmi_reader_clean(&reader);
         return -1;
       }
@@ -202,15 +203,19 @@ static int cap_http_handler(void *cls, struct MHD_Connection *connection,
     *connection_state = &(int){44};
     return MHD_YES;
   }
-  DEBUG(CAP_PLUGIN ": formatted response: %s", g_cap_string);
+  DEBUG(CAP_PLUGIN ": formatted response: %s", g_cap_json);
 
 #if defined(MHD_VERSION) && MHD_VERSION >= 0x00090500
   struct MHD_Response *res = MHD_create_response_from_buffer(
-      strlen(g_cap_string), g_cap_string, MHD_RESPMEM_PERSISTENT);
+      strlen(g_cap_json), g_cap_json, MHD_RESPMEM_PERSISTENT);
 #else
   struct MHD_Response *res =
-      MHD_create_response_from_data(strlen(g_cap_string), g_cap_string, 0, 0);
+      MHD_create_response_from_data(strlen(g_cap_json), g_cap_json, 0, 0);
 #endif
+  if (res == NULL) {
+    ERROR(CAP_PLUGIN ": MHD create response failed.");
+    return MHD_NO;
+  }
 
   MHD_add_response_header(res, MHD_HTTP_HEADER_CONTENT_TYPE, CONTENT_TYPE_JSON);
   int status = MHD_queue_response(connection, MHD_HTTP_OK, res);
@@ -264,12 +269,14 @@ static int cap_open_socket() {
     }
 
     if (bind(fd, ai->ai_addr, ai->ai_addrlen) != 0) {
+      INFO(CAP_PLUGIN ": bind failed: %s", STRERRNO);
       close(fd);
       fd = -1;
       continue;
     }
 
     if (listen(fd, LISTEN_BACKLOG) != 0) {
+      INFO(CAP_PLUGIN ": listen failed: %s", STRERRNO);
       close(fd);
       fd = -1;
       continue;
@@ -313,10 +320,8 @@ static struct MHD_Daemon *cap_start_daemon() {
 #endif
       MHD_OPTION_EXTERNAL_LOGGER, cap_logger, NULL, MHD_OPTION_END);
 
-  if (d == NULL) {
+  if (d == NULL)
     ERROR(CAP_PLUGIN ": MHD_start_daemon() failed.");
-    return NULL;
-  }
 
   return d;
 }
@@ -364,7 +369,7 @@ static int cap_shutdown() {
   }
 
   sfree(httpd_host);
-  sfree(g_cap_string);
+  sfree(g_cap_json);
   return 0;
 }
 
@@ -383,10 +388,10 @@ static int cap_init(void) {
       return -1;
     }
 
-  g_cap_string = json_dumps(root, JSON_COMPACT);
+  g_cap_json = json_dumps(root, JSON_COMPACT);
   json_decref(root);
 
-  if (g_cap_string == NULL) {
+  if (g_cap_json == NULL) {
     ERROR(CAP_PLUGIN ": json_dumps() failed.");
     cap_shutdown();
     return -1;

diff --git a/src/capabilities_test.c b/src/capabilities_test.c
index 95ac66657467636aec3e5461e1f79267605fb959..2d9edd42a40d289114155607bc0973f51a3e1bfb 100644 (file)
--- a/src/capabilities_test.c
+++ b/src/capabilities_test.c
@@ -52,6 +52,8 @@ static entry_type entry[] = {
     DMI_ENTRY_MAP,       DMI_ENTRY_END};
 static size_t len = STATIC_ARRAY_SIZE(entry);
 
+static struct MHD_Response *mhd_res = NULL;
+
 /* mock functions */
 int dmi_reader_init(dmi_reader_t *reader, const dmi_type type) {
   reader->current_type = DMI_ENTRY_NONE;
@@ -82,13 +84,13 @@ void MHD_stop_daemon(struct MHD_Daemon *daemon) {}
 struct MHD_Response *
 MHD_create_response_from_buffer(size_t size, void *data,
                                 enum MHD_ResponseMemoryMode mode) {
-  return NULL;
+  return mhd_res;
 }
 
 struct MHD_Response *MHD_create_response_from_data(size_t size, void *data,
                                                    int must_free,
                                                    int must_copy) {
-  return NULL;
+  return mhd_res;
 }
 
 int MHD_add_response_header(struct MHD_Response *response, const char *header,
@@ -212,7 +214,7 @@ DEF_TEST(plugin_config_fail) {
 
 DEF_TEST(http_handler) {
   void *state = NULL;
-  g_cap_string = "TEST";
+  g_cap_json = "TEST";
   int ret = cap_http_handler(NULL, NULL, NULL, MHD_HTTP_METHOD_PUT, NULL, NULL,
                              NULL, &state);
   EXPECT_EQ_INT(MHD_NO, ret);
@@ -223,12 +225,20 @@ DEF_TEST(http_handler) {
   EXPECT_EQ_INT(MHD_YES, ret);
   CHECK_NOT_NULL(state);
 
+  ret = cap_http_handler(NULL, NULL, NULL, MHD_HTTP_METHOD_GET, NULL, NULL,
+                         NULL, &state);
+  EXPECT_EQ_INT(MHD_NO, ret);
+  CHECK_NOT_NULL(state);
+
+  /* mock not NULL pointer */
+  mhd_res = (struct MHD_Response *)&(int){0};
   ret = cap_http_handler(NULL, NULL, NULL, MHD_HTTP_METHOD_GET, NULL, NULL,
                          NULL, &state);
   EXPECT_EQ_INT(MHD_HTTP_OK, ret);
   CHECK_NOT_NULL(state);
 
-  g_cap_string = NULL;
+  g_cap_json = NULL;
+  mhd_res = NULL;
 
   return 0;
 }

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index 92d6e3e4a9ca88dc04f07f7ea73376a05e1d3e2a..f676abb3e802e0238d39033a3624f71c0af2b0c1 100644 (file)
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -1478,7 +1478,7 @@ returned by plugin is in json format.
 B<Synopsis:>
 
   <Plugin capabilities>
-    Host "<hostname>"
+    Host "localhost"
     Port "9104"
   </Plugin>
 

diff --git a/src/utils/dmi/dmi.c b/src/utils/dmi/dmi.c
index 12e98845afe326de335ef23aaa69bec67012f1e5..5fe3823906ad769070542d7ce877b2839d6bd4ef 100644 (file)
--- a/src/utils/dmi/dmi.c
+++ b/src/utils/dmi/dmi.c
@@ -53,7 +53,7 @@
 #include "utils/common/common.h"
 #include "utils/dmi/dmi.h"
 
-#define UTIL_NAME "DMI_READER"
+#define UTIL_NAME "dmi_reader"
 
 #define DMIDECODE_CMD_FMT_LEN DMI_MAX_LEN
 
@@ -66,7 +66,7 @@ static int dmi_read_entry(dmi_reader_t *r) {
       buff++;
     if (*buff == '\0') {
       r->current_type = DMI_ENTRY_NONE;
-      r->_read_callback = dmi_look_for_handle;
+      r->_read_next = dmi_look_for_handle;
       return DMI_OK;
     }
 
@@ -108,7 +108,7 @@ static int dmi_read_type_name(dmi_reader_t *r) {
 
     r->name = r->_buff;
     r->current_type = DMI_ENTRY_NAME;
-    r->_read_callback = dmi_read_entry;
+    r->_read_next = dmi_read_entry;
 
     return DMI_OK;
   }
@@ -123,7 +123,7 @@ static int dmi_look_for_handle(dmi_reader_t *r) {
     if (strncmp(handle, r->_buff, strlen(handle)) != 0)
       continue;
 
-    r->_read_callback = dmi_read_type_name;
+    r->_read_next = dmi_read_type_name;
     return DMI_OK;
   }
 
@@ -153,8 +153,8 @@ int dmi_reader_init(dmi_reader_t *reader, const dmi_type type) {
 
   reader->name = NULL;
   reader->value = NULL;
-  reader->_read_callback = dmi_look_for_handle;
   reader->current_type = DMI_ENTRY_NONE;
+  reader->_read_next = dmi_look_for_handle;
 
   return DMI_OK;
 }
@@ -165,23 +165,25 @@ void dmi_reader_clean(dmi_reader_t *reader) {
     return;
   }
 
-  if (reader->_fd)
+  if (reader->_fd) {
     pclose(reader->_fd);
+    reader->_fd = NULL;
+  }
 
-  reader->_fd = NULL;
-  reader->_read_callback = NULL;
+  reader->_read_next = NULL;
 }
 
 int dmi_read_next(dmi_reader_t *reader) {
-  if (reader == NULL || reader->_read_callback == NULL || reader->_fd == NULL) {
+  if (reader == NULL || reader->_read_next == NULL || reader->_fd == NULL) {
     ERROR(UTIL_NAME ".%s: NULL pointer.", __func__);
     return DMI_ERROR;
   }
-  int ret = reader->_read_callback(reader);
+  int ret = reader->_read_next(reader);
 
   if (reader->current_type == DMI_ENTRY_END || ret == DMI_ERROR) {
     pclose(reader->_fd);
-    reader->_read_callback = NULL;
+    reader->_fd = NULL;
+    reader->_read_next = NULL;
     DEBUG(UTIL_NAME ": dmidecode reader finished, status=%d.", ret);
   }
 

diff --git a/src/utils/dmi/dmi.h b/src/utils/dmi/dmi.h
index 181477f90f78c1dbc8194318f1111f29588c95f7..c81ff77b65cebd37dca0a8052e56d9a3c7a61190 100644 (file)
--- a/src/utils/dmi/dmi.h
+++ b/src/utils/dmi/dmi.h
@@ -95,7 +95,7 @@ typedef enum entry_type_e {
 typedef struct dmi_reader_s {
   FILE *_fd;
   char _buff[DMI_MAX_LEN];
-  int (*_read_callback)(struct dmi_reader_s *reader);
+  int (*_read_next)(struct dmi_reader_s *reader);
   /* Type of current entry */
   entry_type current_type;
   /* Entry name, the pointer changes after every read. */