cleanup of extension API

- removed TALER_extensions_init()
- added TALER_extension_age_restriction_register()

src/exchange-tools/taler-exchange-offline.c

@@ -3967,8 +3967,6 @@ do_extensions_sign (char *const *args)
   struct TALER_MasterSignatureP sig;
   const struct TALER_Extension *it;
 
-  TALER_extensions_init ();
-
   if (GNUNET_OK != TALER_extensions_load_taler_config (kcfg))
   {
     GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
@@ -4202,7 +4200,9 @@ run (void *cls,
   }
 
   /* load age mask, if age restriction is enabled */
-  TALER_extensions_init ();
+  GNUNET_assert (GNUNET_OK ==
+                 TALER_extension_age_restriction_register ());
+
   if (GNUNET_OK != TALER_extensions_load_taler_config (kcfg))
   {
     GNUNET_log (GNUNET_ERROR_TYPE_ERROR,

src/exchange/taler-exchange-httpd_extensions.c

@@ -143,7 +143,8 @@ extension_update_event_cb (void *cls,
 enum GNUNET_GenericReturnValue
 TEH_extensions_init ()
 {
-  TALER_extensions_init ();
+  GNUNET_assert (GNUNET_OK ==
+                 TALER_extension_age_restriction_register ());
 
   /* Set the event handler for updates */
   struct GNUNET_DB_EventHeaderP ev = {

src/extensions/extension_age_restriction.c

@@ -35,7 +35,7 @@ struct age_restriction_config
 /**
  * Global config for this extension
  */
-static struct age_restriction_config _config = {0};
+static struct age_restriction_config TE_age_restriction_config = {0};
 
 /**
  * @param groups String representation of the age groups. Must be of the form
@@ -141,7 +141,6 @@ TALER_age_mask_to_string (
  * ==================================================
  */
 
-
 /**
  * @brief implements the TALER_Extension.disable interface.
  */
@@ -160,8 +159,8 @@ age_restriction_disable (
     this->config_json = NULL;
   }
 
-  _config.mask.bits = 0;
+  TE_age_restriction_config.mask.bits = 0;
-  _config.num_groups = 0;
+  TE_age_restriction_config.num_groups = 0;
 }
 
 
@@ -227,13 +226,14 @@ age_restriction_load_taler_config (
     GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                 "setting age mask to %x with #groups: %d\n", mask.bits,
                 __builtin_popcount (mask.bits) - 1);
-    _config.mask.bits = mask.bits;
+    TE_age_restriction_config.mask.bits = mask.bits;
-    _config.num_groups = __builtin_popcount (mask.bits) - 1; /* no underflow, first bit always set */
+    TE_age_restriction_config.num_groups = __builtin_popcount (mask.bits) - 1; /* no underflow, first bit always set */
-    this->config = &_config;
+    this->config = &TE_age_restriction_config;
 
-    /* Note: we do now have _config set, however this->config_json is NOT set,
+    /* Note: we do now have TE_age_restriction_config set, however
-     * i.e. the extension is not yet active! For age restriction to become
+     * this->config_json is NOT set, i.e. the extension is not yet active! For
-     * active, load_json_config must have been called. */
+     * age restriction to become active, load_json_config must have been
+     * called. */
   }
 
 
@@ -266,8 +266,8 @@ age_restriction_load_json_config (
   if (TALER_Extension_AgeRestriction != this->type)
     return GNUNET_SYSERR;
 
-  _config.mask.bits = mask.bits;
+  TE_age_restriction_config.mask.bits = mask.bits;
-  _config.num_groups = 0;
+  TE_age_restriction_config.num_groups = 0;
 
   if (mask.bits > 0)
   {
@@ -275,10 +275,10 @@ age_restriction_load_json_config (
     if (0 == (mask.bits & 1))
       return GNUNET_SYSERR;
 
-    _config.num_groups = __builtin_popcount (mask.bits) - 1;
+    TE_age_restriction_config.num_groups = __builtin_popcount (mask.bits) - 1;
   }
 
-  this->config = &_config;
+  this->config = &TE_age_restriction_config;
 
   if (NULL != this->config_json)
     json_decref (this->config_json);
@@ -313,7 +313,7 @@ age_restriction_config_to_json (
     return json_copy (this->config_json);
   }
 
-  mask_str = TALER_age_mask_to_string (&_config.mask);
+  mask_str = TALER_age_mask_to_string (&TE_age_restriction_config.mask);
   conf = GNUNET_JSON_PACK (
     GNUNET_JSON_pack_string ("age_groups", mask_str)
     );
@@ -340,7 +340,7 @@ age_restriction_test_json_config (
 
 
 /* The extension for age restriction */
-struct TALER_Extension _extension_age_restriction = {
+struct TALER_Extension TE_age_restriction = {
   .next = NULL,
   .type = TALER_Extension_AgeRestriction,
   .name = "age_restriction",
@@ -355,24 +355,31 @@ struct TALER_Extension _extension_age_restriction = {
   .load_taler_config = &age_restriction_load_taler_config,
 };
 
+enum GNUNET_GenericReturnValue
+TALER_extension_age_restriction_register ()
+{
+  return TALER_extensions_add (&TE_age_restriction);
+}
+
+
 bool
 TALER_extensions_age_restriction_is_configured ()
 {
-  return (0 != _config.mask.bits);
+  return (0 != TE_age_restriction_config.mask.bits);
 }
 
 
 struct TALER_AgeMask
 TALER_extensions_age_restriction_ageMask ()
 {
-  return _config.mask;
+  return TE_age_restriction_config.mask;
 }
 
 
 size_t
 TALER_extensions_age_restriction_num_groups ()
 {
-  return _config.num_groups;
+  return TE_age_restriction_config.num_groups;
 }
 
 

src/extensions/extensions.c

@@ -26,77 +26,58 @@
 
 
 /* head of the list of all registered extensions */
-// FIXME: remove unnecessary initializers.
+static struct TALER_Extension *TE_extensions = NULL;
-// FIXME: remove unncessary "_" prefix.
-static struct TALER_Extension *_extensions = NULL;
-static bool _initialized = false;
-
-
-void
-TALER_extensions_init ()
-{
-  // FIXME: a bit ugly. Why not have the age_restriction
-  // module have an initializer that registers itself here?
-  extern struct TALER_Extension _extension_age_restriction;
-  if (! _initialized)
-    _extensions = &_extension_age_restriction;
-
-  _initialized = true;
-}
 
 
 const struct TALER_Extension *
 TALER_extensions_get_head ()
 {
-  return _extensions;
+  return TE_extensions;
 }
 
 
-// FIXME: 'new' is a C++ keyword, to NOT use for variable names
 enum GNUNET_GenericReturnValue
 TALER_extensions_add (
-  const struct TALER_Extension *new)
+  const struct TALER_Extension *extension)
 {
-  struct TALER_Extension *ext; // FIXME: limit scope to for() loop
-
-  if (_initialized)
-    return GNUNET_SYSERR;
-
-  GNUNET_assert (NULL != _extensions);
-
   /* Sanity checks */
-  // FIXME: bracket all expressions
+  if ((NULL == extension) ||
-  if (NULL == new ||
+      (NULL == extension->name) ||
-      NULL == new->name ||
+      (NULL == extension->version) ||
-      NULL == new->version ||
+      (NULL == extension->disable) ||
-      NULL == new->disable ||
+      (NULL == extension->test_json_config) ||
-      NULL == new->test_json_config ||
+      (NULL == extension->load_json_config) ||
-      NULL == new->load_json_config ||
+      (NULL == extension->config_to_json) ||
-      NULL == new->config_to_json ||
+      (NULL == extension->load_taler_config))
-      NULL == new->load_taler_config ||
-      NULL == new->next)
   {
     GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
                 "invalid extension\n");
     return GNUNET_SYSERR;
   }
 
-  /* Check for collisions */
+  if (NULL == TE_extensions) /* first extension ?*/
-  for (ext = _extensions; NULL != ext; ext = ext->next)
+    TE_extensions = (struct TALER_Extension *) extension;
+  else
   {
-    if (new->type == ext->type ||
+    struct TALER_Extension *iter;
-        0 == strcmp (new->name,
+
-                     ext->name))
+    /* Check for collisions */
+    for (iter = TE_extensions; NULL != iter; iter = iter->next)
+    {
+      if (extension->type == iter->type ||
+          0 == strcasecmp (extension->name,
+                           iter->name))
       {
         GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
                     "extension collision for `%s'\n",
-                  new->name);
+                    extension->name);
         return GNUNET_NO;
       }
     }
 
     /* No collisions found, so add this extension to the list */
-  ext->next = (struct TALER_Extension *) new;
+    iter->next = (struct TALER_Extension *) extension;
+  }
 
   return GNUNET_OK;
 }
@@ -106,7 +87,7 @@ const struct TALER_Extension *
 TALER_extensions_get_by_type (
   enum TALER_Extension_Type type)
 {
-  for (const struct TALER_Extension *it = _extensions;
+  for (const struct TALER_Extension *it = TE_extensions;
        NULL != it;
        it = it->next)
   {
@@ -135,7 +116,7 @@ const struct TALER_Extension *
 TALER_extensions_get_by_name (
   const char *name)
 {
-  for (const struct TALER_Extension *it = _extensions;
+  for (const struct TALER_Extension *it = TE_extensions;
        NULL != it;
        it = it->next)
   {
@@ -169,37 +150,46 @@ TALER_extensions_verify_json_config_signature (
 }
 
 
-// FIXME: use CamelCase to follow conventions
+/*
-// FIXME: document struct and members
+ * Closure used in TALER_extensions_load_taler_config during call to
-struct load_conf_closure
+ * GNUNET_CONFIGURATION_iterate_sections with configure_extension.
+ */
+struct LoadConfClosure
 {
   const struct GNUNET_CONFIGURATION_Handle *cfg;
   enum GNUNET_GenericReturnValue error;
 };
 
 
-// FIXME: document
+/*
+ * Used in TALER_extensions_load_taler_config during call to
+ * GNUNET_CONFIGURATION_iterate_sections to load the configuration
+ * of supported extensions.
+ *
+ * @param cls Closure of type LoadConfClosure
+ * @param section name of the current section
+ */
 static void
-collect_extensions (
+configure_extension (
   void *cls,
   const char *section)
 {
-  struct load_conf_closure *col = cls;
+  struct LoadConfClosure *col = cls;
   const char *name;
   const struct TALER_Extension *extension;
 
   if (GNUNET_OK != col->error)
     return;
+
   if (0 != strncasecmp (section,
                         TALER_EXTENSION_SECTION_PREFIX,
                         sizeof(TALER_EXTENSION_SECTION_PREFIX) - 1))
-  {
     return;
-  }
 
   name = section + sizeof(TALER_EXTENSION_SECTION_PREFIX) - 1;
 
-  if (NULL == (extension = TALER_extensions_get_by_name (name)))
+  if (NULL ==
+      (extension = TALER_extensions_get_by_name (name)))
   {
     GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
                 "Unsupported extension `%s` (section [%s]).\n", name,
@@ -227,13 +217,13 @@ enum GNUNET_GenericReturnValue
 TALER_extensions_load_taler_config (
   const struct GNUNET_CONFIGURATION_Handle *cfg)
 {
-  struct load_conf_closure col = {
+  struct LoadConfClosure col = {
     .cfg = cfg,
     .error = GNUNET_OK,
   };
 
   GNUNET_CONFIGURATION_iterate_sections (cfg,
-                                         &collect_extensions,
+                                         &configure_extension,
                                          &col);
   return col.error;
 }
@@ -258,19 +248,17 @@ TALER_extensions_is_json_config (
     GNUNET_JSON_spec_end ()
   };
 
-  ret = GNUNET_JSON_parse (obj,
+  if (GNUNET_OK !=
+      (ret = GNUNET_JSON_parse (obj,
                                 spec,
                                 NULL,
-                           NULL);
+                                NULL)))
-  // FIXME: convention says, 'true' path is for
+    return ret;
-  // error handling.
+
-  if (GNUNET_OK == ret)
-  {
   *config = json_copy (cfg);
   GNUNET_JSON_parse_free (spec);
-  }
 
-  return ret;
+  return GNUNET_OK;
 }
 
 

src/include/taler_extensions.h

@@ -71,9 +71,6 @@ struct TALER_Extension
  * Generic functions for extensions
  */
 
-void
-TALER_extensions_init ();
-
 /*
  * Sets the configuration of the extensions from the given TALER configuration
  *
@@ -199,6 +196,12 @@ TALER_extensions_verify_json_config_signature (
                                                           | 1 << 21)
 #define TALER_EXTENSION_AGE_RESTRICTION_DEFAULT_AGE_GROUPS "8:10:12:14:16:18:21"
 
+/**
+ * @brief Registers the extension for age restriction to the list extensions
+ */
+enum GNUNET_GenericReturnValue
+TALER_extension_age_restriction_register ();
+
 /**
  * @brief Parses a string as a list of age groups.
  *

src/testing/test_exchange_api.c

@@ -1233,7 +1233,8 @@ main (int argc,
                     "INFO",
                     NULL);
 
-  TALER_extensions_init ();
+  GNUNET_assert (GNUNET_OK ==
+                 TALER_extension_age_restriction_register ());
 
   cipher = GNUNET_TESTING_get_testname_from_underscore (argv[0]);
   GNUNET_assert (NULL != cipher);

src/testing/test_exchange_p2p.c

@@ -211,7 +211,8 @@ main (int argc,
                     "INFO",
                     NULL);
 
-  TALER_extensions_init ();
+  GNUNET_assert (GNUNET_OK ==
+                 TALER_extension_age_restriction_register ());
 
   cipher = GNUNET_TESTING_get_testname_from_underscore (argv[0]);
   GNUNET_assert (NULL != cipher);