Merge remote-tracking branch 'github/pr/2277'
authorFlorian Forster <octo@collectd.org>
Wed, 17 May 2017 06:40:59 +0000 (08:40 +0200)
committerFlorian Forster <octo@collectd.org>
Wed, 17 May 2017 06:42:17 +0000 (08:42 +0200)
1  2 
src/curl_json.c
src/perl.c
src/testing.h

diff --combined src/curl_json.c
@@@ -86,6 -86,7 +86,6 @@@ struct cj_s /* {{{ *
  
    yajl_handle yajl;
    c_avl_tree_t *tree;
 -  cj_key_t *key;
    int depth;
    struct {
      union {
@@@ -106,11 -107,7 +106,11 @@@ typedef unsigned int yajl_len_t
  #endif
  
  static int cj_read(user_data_t *ud);
 -static void cj_submit(cj_t *db, cj_key_t *key, value_t *value);
 +static void cj_submit_impl(cj_t *db, cj_key_t *key, value_t *value);
 +
 +/* cj_submit is a function pointer to cj_submit_impl, allowing the unit-test to
 + * overwrite which function is called. */
 +static void (*cj_submit)(cj_t *, cj_key_t *, value_t *) = cj_submit_impl;
  
  static size_t cj_curl_callback(void *buf, /* {{{ */
                                 size_t size, size_t nmemb, void *user_data) {
    len = size * nmemb;
  
    if (len == 0)
-     return (len);
+     return len;
  
    db = user_data;
    if (db == NULL)
-     return (0);
+     return 0;
  
    status = yajl_parse(db->yajl, (unsigned char *)buf, len);
    if (status == yajl_status_ok)
-     return (len);
+     return len;
  #if !HAVE_YAJL_V2
    else if (status == yajl_status_insufficient_data)
-     return (len);
+     return len;
  #endif
  
    unsigned char *msg =
                       /* jsonText = */ (unsigned char *)buf, (unsigned int)len);
    ERROR("curl_json plugin: yajl_parse failed: %s", msg);
    yajl_free_error(db->yajl, msg);
-   return (0); /* abort write callback */
+   return 0; /* abort write callback */
  } /* }}} size_t cj_curl_callback */
  
  static int cj_get_type(cj_key_t *key) {
    return ds->ds[0].type;
  }
  
 -static int cj_cb_map_key(void *ctx, const unsigned char *val, yajl_len_t len);
 +/* cj_load_key loads the configuration for "key" from the parent context and
 + * sets either .key or .tree in the current context. */
 +static int cj_load_key(cj_t *db, char const *key) {
 +  if (db == NULL || key == NULL || db->depth <= 0)
 +    return EINVAL;
  
 -static void cj_cb_inc_array_index(void *ctx, _Bool update_key) {
 -  cj_t *db = (cj_t *)ctx;
 +  sstrncpy(db->state[db->depth].name, key, sizeof(db->state[db->depth].name));
 +
 +  c_avl_tree_t *tree = db->state[db->depth - 1].tree;
 +  if (tree == NULL) {
 +    return 0;
 +  }
 +
 +  /* the parent has a key, so the tree pointer is invalid. */
 +  if (CJ_IS_KEY(db->state[db->depth - 1].key)) {
 +    return 0;
 +  }
  
 +  void *value = NULL;
 +  if (c_avl_get(tree, key, (void *)&value) == 0) {
 +    if (CJ_IS_KEY((cj_key_t *)value)) {
 +      db->state[db->depth].key = value;
 +    } else {
 +      db->state[db->depth].tree = value;
 +    }
 +  } else if (c_avl_get(tree, CJ_ANY, (void *)&value) == 0) {
 +    if (CJ_IS_KEY((cj_key_t *)value)) {
 +      db->state[db->depth].key = value;
 +    } else {
 +      db->state[db->depth].tree = value;
 +    }
 +  } else {
 +    db->state[db->depth].key = NULL;
 +  }
 +
 +  return 0;
 +}
 +
 +static void cj_advance_array(cj_t *db) {
    if (!db->state[db->depth].in_array)
      return;
  
    db->state[db->depth].index++;
  
 -  if (update_key) {
 -    char name[DATA_MAX_NAME_LEN];
 -
 -    ssnprintf(name, sizeof(name), "%d", db->state[db->depth].index - 1);
 -
 -    cj_cb_map_key(ctx, (unsigned char *)name, (yajl_len_t)strlen(name));
 -  }
 +  char name[DATA_MAX_NAME_LEN];
 +  ssnprintf(name, sizeof(name), "%d", db->state[db->depth].index);
 +  cj_load_key(db, name);
  }
  
  /* yajl callbacks */
  #define CJ_CB_CONTINUE 1
  
  static int cj_cb_boolean(void *ctx, int boolVal) {
 -  cj_cb_inc_array_index(ctx, /* update_key = */ 0);
 +  cj_advance_array(ctx);
-   return (CJ_CB_CONTINUE);
+   return CJ_CB_CONTINUE;
  }
  
  static int cj_cb_null(void *ctx) {
 -  cj_cb_inc_array_index(ctx, /* update_key = */ 0);
 +  cj_advance_array(ctx);
-   return (CJ_CB_CONTINUE);
+   return CJ_CB_CONTINUE;
  }
  
  static int cj_cb_number(void *ctx, const char *number, yajl_len_t number_len) {
  
    cj_t *db = (cj_t *)ctx;
    cj_key_t *key = db->state[db->depth].key;
 -  value_t vt;
 -  int type;
 -  int status;
  
    /* Create a null-terminated version of the string. */
    memcpy(buffer, number, number_len);
    buffer[sizeof(buffer) - 1] = 0;
  
 -  if ((key == NULL) || !CJ_IS_KEY(key)) {
 -    if (key != NULL &&
 -        !db->state[db->depth].in_array /*can be inhomogeneous*/) {
 -      NOTICE("curl_json plugin: Found \"%s\", but the configuration expects"
 -             " a map.",
 -             buffer);
 -      return CJ_CB_CONTINUE;
 -    }
--
 -    cj_cb_inc_array_index(ctx, /* update_key = */ 1);
 -    key = db->state[db->depth].key;
 -    if ((key == NULL) || !CJ_IS_KEY(key)) {
 -      return CJ_CB_CONTINUE;
 -    }
 -  } else {
 -    cj_cb_inc_array_index(ctx, /* update_key = */ 1);
 +  if (key == NULL) {
 +    /* no config for this element. */
 +    cj_advance_array(ctx);
 +    return CJ_CB_CONTINUE;
 +  } else if (!CJ_IS_KEY(key)) {
 +    /* the config expects a map or an array. */
 +    NOTICE(
 +        "curl_json plugin: Found \"%s\", but the configuration expects a map.",
 +        buffer);
 +    cj_advance_array(ctx);
 +    return CJ_CB_CONTINUE;
    }
  
 -  type = cj_get_type(key);
 -  status = parse_value(buffer, &vt, type);
 +  int type = cj_get_type(key);
 +  value_t vt;
 +  int status = parse_value(buffer, &vt, type);
    if (status != 0) {
      NOTICE("curl_json plugin: Unable to parse number: \"%s\"", buffer);
-     return (CJ_CB_CONTINUE);
 +    cj_advance_array(ctx);
+     return CJ_CB_CONTINUE;
    }
  
    cj_submit(db, key, &vt);
-   return (CJ_CB_CONTINUE);
 +  cj_advance_array(ctx);
+   return CJ_CB_CONTINUE;
  } /* int cj_cb_number */
  
  /* Queries the key-tree of the parent context for "in_name" and, if found,
   * NULL. */
  static int cj_cb_map_key(void *ctx, unsigned char const *in_name,
                           yajl_len_t in_name_len) {
 -  cj_t *db = (cj_t *)ctx;
 -  c_avl_tree_t *tree;
 +  char name[in_name_len + 1];
  
 -  tree = db->state[db->depth - 1].tree;
 -
 -  if (tree != NULL) {
 -    cj_key_t *value = NULL;
 -    char *name;
 -    size_t name_len;
 -
 -    /* Create a null-terminated version of the name. */
 -    name = db->state[db->depth].name;
 -    name_len =
 -        COUCH_MIN((size_t)in_name_len, sizeof(db->state[db->depth].name) - 1);
 -    memcpy(name, in_name, name_len);
 -    name[name_len] = 0;
 -
 -    if (c_avl_get(tree, name, (void *)&value) == 0) {
 -      if (CJ_IS_KEY((cj_key_t *)value)) {
 -        db->state[db->depth].key = value;
 -      } else {
 -        db->state[db->depth].tree = (c_avl_tree_t *)value;
 -      }
 -    } else if (c_avl_get(tree, CJ_ANY, (void *)&value) == 0)
 -      if (CJ_IS_KEY((cj_key_t *)value)) {
 -        db->state[db->depth].key = value;
 -      } else {
 -        db->state[db->depth].tree = (c_avl_tree_t *)value;
 -      }
 -    else
 -      db->state[db->depth].key = NULL;
 -  }
 +  memmove(name, in_name, in_name_len);
 +  name[sizeof(name) - 1] = 0;
 +
 +  if (cj_load_key(ctx, name) != 0)
 +    return CJ_CB_ABORT;
  
    return CJ_CB_CONTINUE;
  }
  
  static int cj_cb_string(void *ctx, const unsigned char *val, yajl_len_t len) {
    /* Handle the string as if it was a number. */
-   return (cj_cb_number(ctx, (const char *)val, len));
+   return cj_cb_number(ctx, (const char *)val, len);
  } /* int cj_cb_string */
  
 -static int cj_cb_start(void *ctx) {
 -  cj_t *db = (cj_t *)ctx;
 -  if (++db->depth >= YAJL_MAX_DEPTH) {
 -    ERROR("curl_json plugin: %s depth exceeds max, aborting.",
 -          db->url ? db->url : db->sock);
 -    return CJ_CB_ABORT;
 -  }
 -  return CJ_CB_CONTINUE;
 -}
 -
  static int cj_cb_end(void *ctx) {
    cj_t *db = (cj_t *)ctx;
    db->state[db->depth].tree = NULL;
 -  --db->depth;
 +  db->depth--;
 +  cj_advance_array(ctx);
-   return (CJ_CB_CONTINUE);
+   return CJ_CB_CONTINUE;
  }
  
  static int cj_cb_start_map(void *ctx) {
 -  cj_cb_inc_array_index(ctx, /* update_key = */ 1);
 -  return cj_cb_start(ctx);
 +  cj_t *db = (cj_t *)ctx;
 +
 +  if ((db->depth + 1) >= YAJL_MAX_DEPTH) {
 +    ERROR("curl_json plugin: %s depth exceeds max, aborting.",
 +          db->url ? db->url : db->sock);
-     return (CJ_CB_ABORT);
++    return CJ_CB_ABORT;
 +  }
 +  db->depth++;
-   return (CJ_CB_CONTINUE);
++  return CJ_CB_CONTINUE;
  }
  
  static int cj_cb_end_map(void *ctx) { return cj_cb_end(ctx); }
  
  static int cj_cb_start_array(void *ctx) {
    cj_t *db = (cj_t *)ctx;
 -  cj_cb_inc_array_index(ctx, /* update_key = */ 1);
 -  if (db->depth + 1 < YAJL_MAX_DEPTH) {
 -    db->state[db->depth + 1].in_array = 1;
 -    db->state[db->depth + 1].index = 0;
 +
 +  if ((db->depth + 1) >= YAJL_MAX_DEPTH) {
 +    ERROR("curl_json plugin: %s depth exceeds max, aborting.",
 +          db->url ? db->url : db->sock);
 +    return CJ_CB_ABORT;
    }
 -  return cj_cb_start(ctx);
 +  db->depth++;
 +  db->state[db->depth].in_array = 1;
 +  db->state[db->depth].index = 0;
 +
 +  cj_load_key(db, "0");
 +
 +  return CJ_CB_CONTINUE;
  }
  
  static int cj_cb_end_array(void *ctx) {
@@@ -422,7 -412,7 +420,7 @@@ static void cj_free(void *arg) /* {{{ *
  /* Configuration handling functions {{{ */
  
  static c_avl_tree_t *cj_avl_create(void) {
 -  return c_avl_create((int(*)(const void *, const void *))strcmp);
 +  return c_avl_create((int (*)(const void *, const void *))strcmp);
  }
  
  static int cj_config_append_string(const char *name,
    struct curl_slist *temp = NULL;
    if ((ci->values_num != 1) || (ci->values[0].type != OCONFIG_TYPE_STRING)) {
      WARNING("curl_json plugin: `%s' needs exactly one string argument.", name);
-     return (-1);
+     return -1;
    }
  
    temp = curl_slist_append(*dest, ci->values[0].value.string);
    if (temp == NULL)
-     return (-1);
+     return -1;
  
    *dest = temp;
  
-   return (0);
+   return 0;
  } /* }}} int cj_config_append_string */
  
 +/* cj_append_key adds key to the configuration stored in db.
 + *
 + * For example:
 + * "httpd/requests/count",
 + * "httpd/requests/current" ->
 + * { "httpd": { "requests": { "count": $key, "current": $key } } }
 + */
 +static int cj_append_key(cj_t *db, cj_key_t *key) { /* {{{ */
 +  if (db->tree == NULL)
 +    db->tree = cj_avl_create();
 +
 +  c_avl_tree_t *tree = db->tree;
 +
 +  char const *start = key->path;
 +  if (*start == '/')
 +    ++start;
 +
 +  char const *end;
 +  while ((end = strchr(start, '/')) != NULL) {
 +    char name[PATH_MAX];
 +
 +    size_t len = end - start;
 +    if (len == 0)
 +      break;
 +
 +    len = COUCH_MIN(len, sizeof(name) - 1);
 +    sstrncpy(name, start, len + 1);
 +
 +    c_avl_tree_t *value;
 +    if (c_avl_get(tree, name, (void *)&value) != 0) {
 +      value = cj_avl_create();
 +      c_avl_insert(tree, strdup(name), value);
 +    }
 +
 +    tree = value;
 +    start = end + 1;
 +  }
 +
 +  if (strlen(start) == 0) {
 +    ERROR("curl_json plugin: invalid key: %s", key->path);
 +    return -1;
 +  }
 +
 +  c_avl_insert(tree, strdup(start), key);
 +  return 0;
 +} /* }}} int cj_append_key */
 +
  static int cj_config_add_key(cj_t *db, /* {{{ */
                               oconfig_item_t *ci) {
    cj_key_t *key;
    if ((ci->values_num != 1) || (ci->values[0].type != OCONFIG_TYPE_STRING)) {
      WARNING("curl_json plugin: The `Key' block "
              "needs exactly one string argument.");
-     return (-1);
+     return -1;
    }
  
    key = calloc(1, sizeof(*key));
    if (key == NULL) {
      ERROR("curl_json plugin: calloc failed.");
-     return (-1);
+     return -1;
    }
    key->magic = CJ_KEY_MAGIC;
  
      status = cf_util_get_string(ci, &key->path);
      if (status != 0) {
        sfree(key);
-       return (status);
+       return status;
      }
    } else {
      ERROR("curl_json plugin: cj_config: "
            "Invalid key: %s",
            ci->key);
      cj_key_free(key);
-     return (-1);
+     return -1;
    }
  
    status = 0;
  
    if (status != 0) {
      cj_key_free(key);
-     return (-1);
+     return -1;
    }
  
    if (key->type == NULL) {
      WARNING("curl_json plugin: `Type' missing in `Key' block.");
      cj_key_free(key);
-     return (-1);
+     return -1;
    }
  
 -  /* store path in a tree that will match the json map structure, example:
 -   * "httpd/requests/count",
 -   * "httpd/requests/current" ->
 -   * { "httpd": { "requests": { "count": $key, "current": $key } } }
 -   */
 -  char *ptr;
 -  char *name;
 -  c_avl_tree_t *tree;
 -
 -  if (db->tree == NULL)
 -    db->tree = cj_avl_create();
 -
 -  tree = db->tree;
 -  ptr = key->path;
 -  if (*ptr == '/')
 -    ++ptr;
 -
 -  name = ptr;
 -  while ((ptr = strchr(name, '/')) != NULL) {
 -    char ent[PATH_MAX];
 -    c_avl_tree_t *value;
 -    size_t len;
 -
 -    len = ptr - name;
 -    if (len == 0)
 -      break;
 -
 -    len = COUCH_MIN(len, sizeof(ent) - 1);
 -    sstrncpy(ent, name, len + 1);
 -
 -    if (c_avl_get(tree, ent, (void *)&value) != 0) {
 -      value = cj_avl_create();
 -      c_avl_insert(tree, strdup(ent), value);
 -    }
 -
 -    tree = value;
 -    name = ptr + 1;
 -  }
 -
 -  if (strlen(name) == 0) {
 -    ERROR("curl_json plugin: invalid key: %s", key->path);
 +  status = cj_append_key(db, key);
 +  if (status != 0) {
      cj_key_free(key);
      return -1;
    }
  
 -  c_avl_insert(tree, strdup(name), key);
 -  return status;
 +  return 0;
  } /* }}} int cj_config_add_key */
  
  static int cj_init_curl(cj_t *db) /* {{{ */
    db->curl = curl_easy_init();
    if (db->curl == NULL) {
      ERROR("curl_json plugin: curl_easy_init failed.");
-     return (-1);
+     return -1;
    }
  
    curl_easy_setopt(db->curl, CURLOPT_NOSIGNAL, 1L);
      db->credentials = malloc(credentials_size);
      if (db->credentials == NULL) {
        ERROR("curl_json plugin: malloc failed.");
-       return (-1);
+       return -1;
      }
  
      ssnprintf(db->credentials, credentials_size, "%s:%s", db->user,
                       (long)CDTIME_T_TO_MS(plugin_get_interval()));
  #endif
  
-   return (0);
+   return 0;
  } /* }}} int cj_init_curl */
  
  static int cj_config_add_url(oconfig_item_t *ci) /* {{{ */
    if ((ci->values_num != 1) || (ci->values[0].type != OCONFIG_TYPE_STRING)) {
      WARNING("curl_json plugin: The `URL' block "
              "needs exactly one string argument.");
-     return (-1);
+     return -1;
    }
  
    db = calloc(1, sizeof(*db));
    if (db == NULL) {
      ERROR("curl_json plugin: calloc failed.");
-     return (-1);
+     return -1;
    }
  
    db->timeout = -1;
            "Invalid key: %s",
            ci->key);
      cj_free(db);
-     return (-1);
+     return -1;
    }
    if (status != 0) {
      sfree(db);
-     return (status);
+     return status;
    }
  
    /* Fill the `cj_t' structure.. */
      sfree(cb_name);
    } else {
      cj_free(db);
-     return (-1);
+     return -1;
    }
  
-   return (0);
+   return 0;
  }
  /* }}} int cj_config_add_database */
  
@@@ -768,10 -751,10 +766,10 @@@ static int cj_config(oconfig_item_t *ci
  
    if ((success == 0) && (errors > 0)) {
      ERROR("curl_json plugin: All statements failed.");
-     return (-1);
+     return -1;
    }
  
-   return (0);
+   return 0;
  } /* }}} int cj_config */
  
  /* }}} End of configuration handling functions */
@@@ -784,7 -767,7 +782,7 @@@ static const char *cj_host(cj_t *db) /
    return db->host;
  } /* }}} cj_host */
  
 -static void cj_submit(cj_t *db, cj_key_t *key, value_t *value) /* {{{ */
 +static void cj_submit_impl(cj_t *db, cj_key_t *key, value_t *value) /* {{{ */
  {
    value_list_t vl = VALUE_LIST_INIT;
  
      vl.interval = db->interval;
  
    plugin_dispatch_values(&vl);
 -} /* }}} int cj_submit */
 +} /* }}} int cj_submit_impl */
  
  static int cj_sock_perform(cj_t *db) /* {{{ */
  {
    char errbuf[1024];
 -  struct sockaddr_un sa_unix = {0};
 -  sa_unix.sun_family = AF_UNIX;
 +  struct sockaddr_un sa_unix = {
 +      .sun_family = AF_UNIX,
 +  };
    sstrncpy(sa_unix.sun_path, db->sock, sizeof(sa_unix.sun_path));
  
    int fd = socket(AF_UNIX, SOCK_STREAM, 0);
    if (fd < 0)
-     return (-1);
+     return -1;
    if (connect(fd, (struct sockaddr *)&sa_unix, sizeof(sa_unix)) < 0) {
      ERROR("curl_json plugin: connect(%s) failed: %s",
            (db->sock != NULL) ? db->sock : "<null>",
            sstrerror(errno, errbuf, sizeof(errbuf)));
      close(fd);
-     return (-1);
+     return -1;
    }
  
    ssize_t red;
              (db->sock != NULL) ? db->sock : "<null>",
              sstrerror(errno, errbuf, sizeof(errbuf)));
        close(fd);
-       return (-1);
+       return -1;
      }
      if (!cj_curl_callback(buffer, red, 1, db))
        break;
    } while (red > 0);
    close(fd);
-   return (0);
+   return 0;
  } /* }}} int cj_sock_perform */
  
  static int cj_curl_perform(cj_t *db) /* {{{ */
    if (status != CURLE_OK) {
      ERROR("curl_json plugin: curl_easy_perform failed with status %i: %s (%s)",
            status, db->curl_errbuf, url);
-     return (-1);
+     return -1;
    }
    if (db->stats != NULL)
      curl_stats_dispatch(db->stats, db->curl, cj_host(db), "curl_json",
      ERROR("curl_json plugin: curl_easy_perform failed with "
            "response code %ld (%s)",
            rc, url);
-     return (-1);
+     return -1;
    }
-   return (0);
+   return 0;
  } /* }}} int cj_curl_perform */
  
  static int cj_perform(cj_t *db) /* {{{ */
    if (db->yajl == NULL) {
      ERROR("curl_json plugin: yajl_alloc failed.");
      db->yajl = yprev;
-     return (-1);
+     return -1;
    }
  
    if (db->url)
    if (status < 0) {
      yajl_free(db->yajl);
      db->yajl = yprev;
-     return (-1);
+     return -1;
    }
  
  #if HAVE_YAJL_V2
      yajl_free_error(db->yajl, errmsg);
      yajl_free(db->yajl);
      db->yajl = yprev;
-     return (-1);
+     return -1;
    }
  
    yajl_free(db->yajl);
    db->yajl = yprev;
-   return (0);
+   return 0;
  } /* }}} int cj_perform */
  
  static int cj_read(user_data_t *ud) /* {{{ */
  
    if ((ud == NULL) || (ud->data == NULL)) {
      ERROR("curl_json plugin: cj_read: Invalid user data.");
-     return (-1);
+     return -1;
    }
  
    db = (cj_t *)ud->data;
    db->depth = 0;
    memset(&db->state, 0, sizeof(db->state));
    db->state[db->depth].tree = db->tree;
 -  db->key = NULL;
  
    return cj_perform(db);
  } /* }}} int cj_read */
@@@ -950,7 -933,7 +948,7 @@@ static int cj_init(void) /* {{{ *
    /* Call this while collectd is still single-threaded to avoid
     * initialization issues in libgcrypt. */
    curl_global_init(CURL_GLOBAL_SSL);
-   return (0);
+   return 0;
  } /* }}} int cj_init */
  
  void module_register(void) {
diff --combined src/perl.c
@@@ -494,15 -494,16 +494,15 @@@ static int av2data_set(pTHX_ AV *array
   *   meta     => [ { name => <name>, value => <value> }, ... ]
   * }
   */
 -static int av2notification_meta(pTHX_ AV *array, notification_meta_t **meta) {
 -  notification_meta_t **m = meta;
 +static int av2notification_meta(pTHX_ AV *array, notification_meta_t **ret_meta) {
 +  notification_meta_t *tail = NULL;
  
    int len = av_len(array);
  
    for (int i = 0; i <= len; ++i) {
      SV **tmp = av_fetch(array, i, 0);
 -    HV *hash;
  
 -    if (NULL == tmp)
 +    if (tmp == NULL)
        return -1;
  
      if (!(SvROK(*tmp) && (SVt_PVHV == SvTYPE(SvRV(*tmp))))) {
        continue;
      }
  
 -    hash = (HV *)SvRV(*tmp);
 +    HV *hash = (HV *)SvRV(*tmp);
  
 -    *m = smalloc(sizeof(**m));
 +    notification_meta_t *m = calloc(1, sizeof(*m));
 +    if (m == NULL)
 +      return ENOMEM;
  
 -    if (NULL == (tmp = hv_fetch(hash, "name", 4, 0))) {
 +    SV **name = hv_fetch(hash, "name", strlen("name"), 0);
 +    if (name == NULL) {
        log_warn("av2notification_meta: Skipping invalid "
                 "meta information.");
 -      free(*m);
 +      sfree(m);
        continue;
      }
 -    sstrncpy((*m)->name, SvPV_nolen(*tmp), sizeof((*m)->name));
 +    sstrncpy(m->name, SvPV_nolen(*name), sizeof(m->name));
  
 -    if (NULL == (tmp = hv_fetch(hash, "value", 5, 0))) {
 +    SV **value = hv_fetch(hash, "value", strlen("value"), 0);
 +    if (value == NULL) {
        log_warn("av2notification_meta: Skipping invalid "
                 "meta information.");
 -      free(*m);
 +      sfree(m);
        continue;
      }
  
 -    if (SvNOK(*tmp)) {
 -      (*m)->nm_value.nm_double = SvNVX(*tmp);
 -      (*m)->type = NM_TYPE_DOUBLE;
 -    } else if (SvUOK(*tmp)) {
 -      (*m)->nm_value.nm_unsigned_int = SvUVX(*tmp);
 -      (*m)->type = NM_TYPE_UNSIGNED_INT;
 -    } else if (SvIOK(*tmp)) {
 -      (*m)->nm_value.nm_signed_int = SvIVX(*tmp);
 -      (*m)->type = NM_TYPE_SIGNED_INT;
 +    if (SvNOK(*value)) {
 +      m->nm_value.nm_double = SvNVX(*value);
 +      m->type = NM_TYPE_DOUBLE;
 +    } else if (SvUOK(*value)) {
 +      m->nm_value.nm_unsigned_int = SvUVX(*value);
 +      m->type = NM_TYPE_UNSIGNED_INT;
 +    } else if (SvIOK(*value)) {
 +      m->nm_value.nm_signed_int = SvIVX(*value);
 +      m->type = NM_TYPE_SIGNED_INT;
      } else {
 -      (*m)->nm_value.nm_string = sstrdup(SvPV_nolen(*tmp));
 -      (*m)->type = NM_TYPE_STRING;
 +      m->nm_value.nm_string = sstrdup(SvPV_nolen(*value));
 +      m->type = NM_TYPE_STRING;
      }
  
 -    (*m)->next = NULL;
 -    m = &((*m)->next);
 +    m->next = NULL;
 +    if (tail == NULL)
 +      *ret_meta = m;
 +    else
 +      tail->next = m;
 +    tail = m;
    }
 +
    return 0;
  } /* static int av2notification_meta (AV *, notification_meta_t *) */
  
@@@ -885,8 -877,8 +885,8 @@@ static char *get_module_name(char *buf
    else
      status = ssnprintf(buf, buf_len, "%s::%s", base_name, module);
    if ((status < 0) || ((unsigned int)status >= buf_len))
-     return (NULL);
-   return (buf);
+     return NULL;
+   return buf;
  } /* char *get_module_name */
  
  /*
@@@ -2497,7 -2489,7 +2497,7 @@@ static int perl_config_loadplugin(pTHX
  
    if (NULL == get_module_name(module_name, sizeof(module_name), value)) {
      log_err("Invalid module name %s", value);
-     return (1);
+     return 1;
    }
  
    if (0 != init_pi(perl_argc, perl_argv))
diff --combined src/testing.h
@@@ -71,7 -71,7 +71,7 @@@ static int check_count__ = 0
      if (strcmp(expect, got__) != 0) {                                          \
        printf("not ok %i - %s = \"%s\", want \"%s\"\n", ++check_count__,        \
               #actual, got__, expect);                                          \
-       return (-1);                                                             \
+       return -1;                                                               \
      }                                                                          \
      printf("ok %i - %s = \"%s\"\n", ++check_count__, #actual, got__);          \
    } while (0)
@@@ -83,7 -83,7 +83,7 @@@
      if (got__ != want__) {                                                     \
        printf("not ok %i - %s = %d, want %d\n", ++check_count__, #actual,       \
               got__, want__);                                                   \
-       return (-1);                                                             \
+       return -1;                                                               \
      }                                                                          \
      printf("ok %i - %s = %d\n", ++check_count__, #actual, got__);              \
    } while (0)
@@@ -95,7 -95,7 +95,7 @@@
      if (got__ != want__) {                                                     \
        printf("not ok %i - %s = %" PRIu64 ", want %" PRIu64 "\n",               \
               ++check_count__, #actual, got__, want__);                         \
-       return (-1);                                                             \
+       return -1;                                                               \
      }                                                                          \
      printf("ok %i - %s = %" PRIu64 "\n", ++check_count__, #actual, got__);     \
    } while (0)
    do {                                                                         \
      double want__ = (double)expect;                                            \
      double got__ = (double)actual;                                             \
 -    if (isnan(want__) && !isnan(got__)) {                                      \
 +    if ((isnan(want__) && !isnan(got__)) ||                                    \
 +        (!isnan(want__) && isnan(got__))) {                                    \
        printf("not ok %i - %s = %.15g, want %.15g\n", ++check_count__, #actual, \
               got__, want__);                                                   \
-       return (-1);                                                             \
+       return -1;                                                               \
      } else if (!isnan(want__) && (((want__ - got__) < -DBL_PRECISION) ||       \
                                    ((want__ - got__) > DBL_PRECISION))) {       \
        printf("not ok %i - %s = %.15g, want %.15g\n", ++check_count__, #actual, \
               got__, want__);                                                   \
-       return (-1);                                                             \
+       return -1;                                                               \
      }                                                                          \
      printf("ok %i - %s = %.15g\n", ++check_count__, #actual, got__);           \
    } while (0)