implement memory management review feedback

1 files changed, 294 insertions, 302 deletions

diff --git a/src/backend/taler-merchant-httpd_private-post-orders.c b/src/backend/taler-merchant-httpd_private-post-orders.c
index 3a64d5a3..1488fe0d 100644
--- a/src/backend/taler-merchant-httpd_private-post-orders.c
+++ b/src/backend/taler-merchant-httpd_private-post-orders.c
@@ -642,6 +642,26 @@ clean_order (void *cls)
     json_decref (oc->set_exchanges.exchanges);
     oc->set_exchanges.exchanges = NULL;
   }
+  if (NULL != oc->parse_order.fulfillment_message_i18n)
+  {
+    json_decref (oc->parse_order.fulfillment_message_i18n);
+    oc->parse_order.fulfillment_message_i18n = NULL;
+  }
+  if (NULL != oc->parse_order.summary_i18n)
+  {
+    json_decref (oc->parse_order.summary_i18n);
+    oc->parse_order.summary_i18n = NULL;
+  }
+  if (NULL != oc->parse_order.delivery_location)
+  {
+    json_decref (oc->parse_order.delivery_location);
+    oc->parse_order.delivery_location = NULL;
+  }
+  if (NULL != oc->merge_inventory.products)
+  {
+    json_decref (oc->merge_inventory.products);
+    oc->merge_inventory.products = NULL;
+  }
   // TODO: Check if this is even correct
   for (unsigned int i = 0; i<oc->parse_choices.choices_len; i++)
   {
@@ -652,11 +672,6 @@ clean_order (void *cls)
                        oc->parse_choices.choices[i].outputs_len,
                        0);
   }
-  if (NULL != oc->merge_inventory.products)
-  {
-    json_decref (oc->merge_inventory.products);
-    oc->merge_inventory.products = NULL;
-  }
   GNUNET_array_grow (oc->parse_request.inventory_products,
                      oc->parse_request.inventory_products_length,
                      0);
@@ -2111,70 +2126,77 @@ parse_order (struct OrderContext *oc)
 static void
 parse_token_types (struct OrderContext *oc)
 {
-  if (NULL != oc->parse_order.token_types)
+  if (NULL == oc->parse_order.token_types)
+  {
+    oc->phase++;
+    return;
+  }
+
+  const char *key;
+  json_t *value;
+
+  json_object_foreach ((json_t*) oc->parse_order.token_types,
+                        key,
+                        value)
   {
-    const char *key;
-    json_t *value;
+    struct TALER_MerchantContractTokenAuthority authority;
+    const char *error_name;
+    unsigned int error_line;
+    // const json_t *jpublic_key = NULL;
 
-    json_object_foreach ((json_t*) oc->parse_order.token_types,
-                         key,
-                         value)
     {
-      struct TALER_MerchantContractTokenAuthority authority;
-      const json_t *jpublic_key = NULL;
+      struct GNUNET_JSON_Specification spec[] = {
+        // GNUNET_JSON_spec_object_const ("key", &jpublic_key),
+        GNUNET_JSON_spec_bool ("critical", &authority.critical),
+        GNUNET_JSON_spec_end ()
+      };
 
+      if (GNUNET_OK !=
+          GNUNET_JSON_parse (value,
+                             spec,
+                             &error_name,
+                             &error_line))
       {
-        struct GNUNET_JSON_Specification spec[] = {
-          GNUNET_JSON_spec_object_const ("key", &jpublic_key),
-          GNUNET_JSON_spec_bool ("critical", &authority.critical),
-          GNUNET_JSON_spec_end ()
-        };
+        GNUNET_break_op (0);
+        GNUNET_JSON_parse_free (spec);
+        reply_with_error (oc,
+                          MHD_HTTP_BAD_REQUEST,
+                          TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                          error_name);
+        return;
+      }
+    }
 
-        if (GNUNET_OK !=
-            GNUNET_JSON_parse (value,
+
+    {
+      struct GNUNET_JSON_Specification spec[] = {
+        GNUNET_JSON_spec_rsa_public_key ("rsa_pub", &authority.key.public_key.details.rsa_public_key),
+        GNUNET_JSON_spec_end ()
+      };
+
+      if (GNUNET_OK !=
+          GNUNET_JSON_parse (jpublic_key,
                               spec,
                               NULL,
                               NULL))
-        {
-          GNUNET_break_op (0);
-          GNUNET_JSON_parse_free (spec);
-          reply_with_error (oc,
-                            MHD_HTTP_BAD_REQUEST,
-                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                            "token_types failed to parse");
-          return;
-        }
-      }
-
-
       {
-        struct GNUNET_JSON_Specification spec[] = {
-          GNUNET_JSON_spec_rsa_public_key ("rsa_pub", &authority.key.public_key.details.rsa_public_key),
-          GNUNET_JSON_spec_end ()
-        };
-
-        if (GNUNET_OK !=
-            GNUNET_JSON_parse (jpublic_key,
-                               spec,
-                               NULL,
-                               NULL))
-        {
-          GNUNET_break_op (0);
-          GNUNET_JSON_parse_free (spec);
-          reply_with_error (oc,
-                            MHD_HTTP_BAD_REQUEST,
-                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                            "key object of token type is invalid");
-          return;
-        }
+        GNUNET_break_op (0);
+        GNUNET_JSON_parse_free (spec);
+        reply_with_error (oc,
+                          MHD_HTTP_BAD_REQUEST,
+                          TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                          "key object of token type is invalid");
+        return;
       }
+    }
 
-      authority.key.public_key.cipher = GNUNET_CRYPTO_BSA_RSA;
+    authority.key.public_key.cipher = GNUNET_CRYPTO_BSA_RSA;
 
-      GNUNET_CRYPTO_rsa_public_key_hash (
-        authority.key.public_key.details.rsa_public_key,
-        &authority.key.public_key.pub_key_hash);
+    GNUNET_CRYPTO_rsa_public_key_hash (
+      authority.key.public_key.details.rsa_public_key,
+      &authority.key.public_key.pub_key_hash);
 
+    {
       struct TALER_MERCHANTDB_TokenFamilyKeyDetails key_details;
       enum GNUNET_DB_QueryStatus qs;
 
@@ -2219,8 +2241,10 @@ parse_token_types (struct OrderContext *oc)
       }
 
       // TODO: Copy relevant fields from key_details to authority
-      strcpy((char *) authority.label, key);
-      strcpy((char *) authority.summary, key_details.token_family.description);
+      // TODO: Don't free key_details.token_family, because we steal the references here
+      authority.label = key;
+      authority.summary = key_details.token_family.description;
+      authority.summary_i18n = key_details.token_family.description_i18n;
 
       switch (key_details.token_family.kind) {
       case TALER_MERCHANTDB_TFK_Subscription:
@@ -2231,10 +2255,12 @@ parse_token_types (struct OrderContext *oc)
         break;
       }
 
-      GNUNET_array_append (oc->parse_token_types.authorities,
-                           oc->parse_token_types.authorities_len,
-                           authority);
+      // TODO: Free members of key_details which I didn't steal (private key)
     }
+
+    GNUNET_array_append (oc->parse_token_types.authorities,
+                          oc->parse_token_types.authorities_len,
+                          authority);
   }
 
   oc->phase++;
@@ -2249,279 +2275,243 @@ parse_token_types (struct OrderContext *oc)
 static void
 parse_choices (struct OrderContext *oc)
 {
-  if (NULL != oc->parse_order.choices)
+  if (NULL == oc->parse_order.choices)
   {
-    GNUNET_array_grow (oc->parse_choices.choices,
-                       oc->parse_choices.choices_len,
-                       json_array_size (oc->parse_order.choices));
-    for (unsigned int i = 0; i<oc->parse_choices.choices_len; i++)
+    oc->phase++;
+    return;
+  }
+
+  GNUNET_array_grow (oc->parse_choices.choices,
+                     oc->parse_choices.choices_len,
+                     json_array_size (oc->parse_order.choices));
+  for (unsigned int i = 0; i<oc->parse_choices.choices_len; i++)
+  {
+    const char *error_name;
+    unsigned int error_line;
+    const json_t *jinputs;
+    const json_t *joutputs;
+    struct GNUNET_JSON_Specification spec[] = {
+      GNUNET_JSON_spec_array_const ("inputs",
+                                    &jinputs),
+      GNUNET_JSON_spec_array_const ("outputs",
+                                    &joutputs),
+      GNUNET_JSON_spec_end ()
+    };
+    enum GNUNET_GenericReturnValue ret;
+
+    ret = GNUNET_JSON_parse (json_array_get (oc->parse_order.choices, i),
+                             spec,
+                             &error_name,
+                             &error_line);
+    if (GNUNET_OK != ret)
     {
-      const char *error_name;
-      unsigned int error_line;
-      const json_t *jinputs = NULL;
-      const json_t *joutputs = NULL;
-      struct GNUNET_JSON_Specification spec[] = {
-        GNUNET_JSON_spec_array_const ("inputs",
-                                      &jinputs),
-        GNUNET_JSON_spec_array_const ("outputs",
-                                      &joutputs),
-        GNUNET_JSON_spec_end ()
-      };
+      GNUNET_JSON_parse_free (spec);
+      GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                  "Choice parsing failed: %s:%u\n",
+                  error_name,
+                  error_line);
+      reply_with_error (oc,
+                        MHD_HTTP_BAD_REQUEST,
+                        TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                        "choice");
+      return;
+    }
 
-      enum GNUNET_GenericReturnValue ret;
-      ret = GNUNET_JSON_parse (json_array_get (oc->parse_order.choices, i),
-                               spec,
-                               &error_name,
-                               &error_line);
-      if (GNUNET_OK != ret)
-      {
-        GNUNET_JSON_parse_free (spec);
-        GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                    "Choice parsing failed: %s:%u\n",
-                    error_name,
-                    error_line);
-        reply_with_error (oc,
-                          MHD_HTTP_BAD_REQUEST,
-                          TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                          "choice json parse failed");
-        return;
-      }
+    if (! json_is_array (jinputs) ||
+        ! json_is_array (joutputs))
+    {
+      GNUNET_JSON_parse_free (spec);
+      GNUNET_break_op (0);
+      reply_with_error (oc,
+                        MHD_HTTP_BAD_REQUEST,
+                        TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                        "inputs or outputs");
+      return;
+    }
 
-      if (! json_is_array (jinputs) ||
-          ! json_is_array (joutputs))
+    {
+      // TODO: Maybe move to a separate function
+      const json_t *jinput;
+      size_t idx;
+      json_array_foreach ((json_t *) jinputs, idx, jinput)
       {
-        GNUNET_break_op (0);
-        reply_with_error (oc,
-                          MHD_HTTP_BAD_REQUEST,
-                          TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                          "choice");
-        return;
-      }
+        // TODO: Assuming this struct would have more fields, would i use GNUNET_new then?
+        //       Or should i use GNUNET_grow first and then get the element using the index?
+        //       Assuming you add a field in the future, it's easier to that way, so you don't
+        //       free it.
+        struct TALER_MerchantContractInput input = {.details.token.number = 1};
+        const char *kind;
+        const char *ierror_name;
+        unsigned int ierror_line;
+
+        struct GNUNET_JSON_Specification ispec[] = {
+          GNUNET_JSON_spec_string ("kind",
+                                   &kind),
+          GNUNET_JSON_spec_string ("authority_label",
+                                   &input.details.token.token_authority_label),
+          GNUNET_JSON_spec_mark_optional (
+            GNUNET_JSON_spec_uint32 ("number",
+                                     &input.details.token.number),
+            NULL),
+          GNUNET_JSON_spec_end()
+        };
 
-      {
-        const json_t *jinput;
-        size_t idx;
-        json_array_foreach ((json_t *) jinputs, idx, jinput)
+        if (GNUNET_OK !=
+            GNUNET_JSON_parse (jinput,
+                               ispec,
+                               &ierror_name,
+                               &ierror_line))
         {
-          struct TALER_MerchantContractInput input;
-          const char *kind = NULL;
-          bool no_number;
-
-          struct GNUNET_JSON_Specification spec[] = {
-            GNUNET_JSON_spec_string ("kind",
-                                    &kind),
-            GNUNET_JSON_spec_string ("authority_label",
-                                    &input.details.token.token_authority_label),
-            GNUNET_JSON_spec_mark_optional (
-              GNUNET_JSON_spec_uint32 ("number",
-                                      &input.details.token.number),
-              &no_number),
-            GNUNET_JSON_spec_end()
-          };
-
-          const char *ename;
-          unsigned int eline;
-
-          if (GNUNET_OK !=
-              GNUNET_JSON_parse (jinput,
-                                spec,
-                                &ename,
-                                &eline))
-          {
-            GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                        "Invalid input #%u for field %s\n",
-                        (unsigned int) idx,
-                        ename);
-            reply_with_error (oc,
-                              MHD_HTTP_BAD_REQUEST,
-                              TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                              "input json parse failed");
-            return;
-          }
+          // TODO: I have to call free here beucase the input is not yet appended to the array.
+          //       So its lifecycle ends here.
+          GNUNET_JSON_parse_free (spec);
+          GNUNET_JSON_parse_free (ispec);
+          GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                      "Invalid input #%u for field %s\n",
+                      (unsigned int) idx,
+                      ierror_name);
+          reply_with_error (oc,
+                            MHD_HTTP_BAD_REQUEST,
+                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                            ierror_name);
+          return;
+        }
 
-          /**
-          * Parse kind of input. For now, only 'token' is supported.
-          */
-          if (0 == strcmp("token", kind))
-          {
-            input.type = TALER_MCIT_TOKEN;
-          }
-          else
-          {
-            GNUNET_break_op (0);
-            GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                        "Invalid input #%u for field kind\n",
-                        (unsigned int) idx);
-            reply_with_error (oc,
-                              MHD_HTTP_BAD_REQUEST,
-                              TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                              "invalid kind field in input");
-            return;
-          }
+        input.type = TMH_string_to_contract_input_type (kind);
+        // TODO: Where / when do I have to free kind?
 
-          /**
-          * Set default number to 1 if not present.
-          */
-          if (no_number)
-          {
-            input.details.token.number = 1;
-          }
-          else if (1 > input.details.token.number)
-          {
-            GNUNET_break_op (0);
-            GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                        "Invalid input #%u for field number\n",
-                        (unsigned int) idx);
-            reply_with_error (oc,
-                              MHD_HTTP_BAD_REQUEST,
-                              TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                              "invalid number field in input");
-            return;
-          }
+        if (TALER_MCIT_INVALID == input.type)
+        {
+          GNUNET_JSON_parse_free (spec);
+          GNUNET_JSON_parse_free (ispec);
+          GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                      "Field 'kind' invalid in input #%u\n",
+                      (unsigned int) idx);
+          reply_with_error (oc,
+                            MHD_HTTP_BAD_REQUEST,
+                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                            "kind");
+          return;
+        }
 
-          bool found = false;
+        if (0 == input.details.token.number)
+        {
+          /* Ignore inputs with 'number' field set to 0 */
+          continue;
+        }
 
-          for (unsigned int i = 0; i<oc->parse_token_types.authorities_len; i++)
-          {
-            if (0 == strcmp (oc->parse_token_types.authorities[i].label,
-                            input.details.token.token_authority_label))
-            {
-              found = true;
-              break;
-            }
-          }
+        bool found = false;
 
-          if (! found)
+        for (unsigned int i = 0; i<oc->parse_token_types.authorities_len; i++)
+        {
+          if (0 == strcmp (oc->parse_token_types.authorities[i].label,
+                           input.details.token.token_authority_label))
           {
-            GNUNET_break_op (0);
-            GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                        "Invalid input #%u: authority_label not found in token_types\n",
-                        (unsigned int) idx);
-            reply_with_error (oc,
-                              MHD_HTTP_BAD_REQUEST,
-                              TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                              "invalid authority_label field in input");
-            return;
+            found = true;
+            break;
           }
+        }
 
-          GNUNET_array_append (oc->parse_choices.choices[i].inputs,
-                               oc->parse_choices.choices[i].inputs_len,
-                               input);
+        if (! found)
+        {
+          // TODO: Lookup token family by token_authority_label (slug),
+          //       append to authorities array.
         }
+
+        GNUNET_array_append (oc->parse_choices.choices[i].inputs,
+                             oc->parse_choices.choices[i].inputs_len,
+                             input);
       }
+    }
 
+    {
+      const json_t *joutput;
+      size_t idx;
+      json_array_foreach ((json_t *) joutputs, idx, joutput)
       {
-        const json_t *joutput;
-        size_t idx;
-        json_array_foreach ((json_t *) joutputs, idx, joutput)
+        struct TALER_MerchantContractOutput output = { .details.token.number = 1 };
+        const char *kind;
+        const char *ierror_name;
+        unsigned int ierror_line;
+
+        struct GNUNET_JSON_Specification ispec[] = {
+          GNUNET_JSON_spec_string ("kind",
+                                  &kind),
+          GNUNET_JSON_spec_string ("authority_label",
+                                  &output.details.token.token_authority_label),
+          GNUNET_JSON_spec_mark_optional (
+            GNUNET_JSON_spec_uint32 ("number",
+                                    &output.details.token.number),
+            NULL),
+          GNUNET_JSON_spec_end()
+        };
+
+        if (GNUNET_OK !=
+            GNUNET_JSON_parse (joutput,
+                              ispec,
+                              &ierror_name,
+                              &ierror_line))
         {
-          struct TALER_MerchantContractOutput output;
-          const char *kind = NULL;
-          bool no_number;
-
-          struct GNUNET_JSON_Specification spec[] = {
-            GNUNET_JSON_spec_string ("kind",
-                                    &kind),
-            GNUNET_JSON_spec_string ("authority_label",
-                                    &output.details.token.token_authority_label),
-            GNUNET_JSON_spec_mark_optional (
-              GNUNET_JSON_spec_uint32 ("number",
-                                      &output.details.token.number),
-              &no_number),
-            GNUNET_JSON_spec_end()
-          };
-
-          const char *ename;
-          unsigned int eline;
-
-          if (GNUNET_OK !=
-              GNUNET_JSON_parse (joutput,
-                                spec,
-                                &ename,
-                                &eline))
-          {
-            GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                        "Invalid output #%u for field %s\n",
-                        (unsigned int) idx,
-                        ename);
-            reply_with_error (oc,
-                              MHD_HTTP_BAD_REQUEST,
-                              TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                              "output json parse failed");
-            return;
-          }
+          GNUNET_JSON_parse_free (spec);
+          GNUNET_JSON_parse_free (ispec);
+          GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                      "Invalid output #%u for field %s\n",
+                      (unsigned int) idx,
+                      ierror_name);
+          reply_with_error (oc,
+                            MHD_HTTP_BAD_REQUEST,
+                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                            ierror_name);
+          return;
+        }
 
-          /**
-          * Parse kind of output. For now, only 'token' is supported.
-          */
-          if (0 == strcmp("token", kind))
-          {
-            output.type = TALER_MCOT_TOKEN;
-          }
-          else
-          {
-            GNUNET_break_op (0);
-            GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                        "Invalid output #%u for field kind\n",
-                        (unsigned int) idx);
-            reply_with_error (oc,
-                              MHD_HTTP_BAD_REQUEST,
-                              TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                              "invalid kind field in output");
-            return;
-          }
+        output.type = TMH_string_to_contract_output_type (kind);
 
-          /**
-          * Set default number to 1 if not present.
-          */
-          if (no_number)
-          {
-            output.details.token.number = 1;
-          }
-          else if (1 > output.details.token.number)
-          {
-            GNUNET_break_op (0);
-            GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                        "Invalid output #%u for field number\n",
-                        (unsigned int) idx);
-            reply_with_error (oc,
-                              MHD_HTTP_BAD_REQUEST,
-                              TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                              "invalid number field in output");
-            return;
-          }
+        if (TALER_MCOT_INVALID == output.type)
+        {
+          GNUNET_JSON_parse_free (spec);
+          GNUNET_JSON_parse_free (ispec);
+          GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                      "Field 'kind' invalid in output #%u\n",
+                      (unsigned int) idx);
+          reply_with_error (oc,
+                            MHD_HTTP_BAD_REQUEST,
+                            TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                            "kind");
+          return;
+        }
 
-          bool found = false;
+        if (0 == output.details.token.number)
+        {
+          /* Ignore outputs with 'number' field set to 0 */
+          continue;
+        }
 
-          for (unsigned int i = 0; i<oc->parse_token_types.authorities_len; i++)
-          {
-            if (0 == strcmp (oc->parse_token_types.authorities[i].label,
-                            output.details.token.token_authority_label))
-            {
-              found = true;
-              break;
-            }
-          }
+        bool found = false;
 
-          if (! found)
+        for (unsigned int i = 0; i<oc->parse_token_types.authorities_len; i++)
+        {
+          if (0 == strcmp (oc->parse_token_types.authorities[i].label,
+                           output.details.token.token_authority_label))
           {
-            GNUNET_break_op (0);
-            GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                        "Invalid output #%u: authority_label not found in token_types\n",
-                        (unsigned int) idx);
-            reply_with_error (oc,
-                              MHD_HTTP_BAD_REQUEST,
-                              TALER_EC_GENERIC_PARAMETER_MALFORMED,
-                              "invalid authority_label field in output");
-            return;
+            found = true;
+            break;
           }
+        }
 
-          GNUNET_array_append (oc->parse_choices.choices[i].outputs,
-                               oc->parse_choices.choices[i].outputs_len,
-                               output);
+        if (! found)
+        {
+          // TODO: Lookup token family by token_authority_label (slug),
+          //       append to authorities array.
         }
+
+        GNUNET_array_append (oc->parse_choices.choices[i].outputs,
+                             oc->parse_choices.choices[i].outputs_len,
+                             output);
       }
     }
+
+    // TODO: Do I have to free spec here?
   }
 
   oc->phase++;
@@ -2729,6 +2719,8 @@ parse_request (struct OrderContext *oc)
                                    spec);
   if (GNUNET_OK != ret)
   {
+    // TODO: Question: Why no GNUNET_JSON_parse_free (spec) here?
+    //       How are e.g. oc->parse_request.payment_target or otp_id freed?
     GNUNET_break_op (0);
     finalize_order2 (oc,
                      ret);