Re: [PATCH] pk_mi_json convert functions and corresponding tests.

[Konstantinos Chasialis]

> Hi, Kostas.
>
> On Tue, Sep 29, 2020 at 08:05:44PM +0300, Konstantinos Chasialis wrote:
>>diff --git a/poke/pk-mi-json.c b/poke/pk-mi-json.c
>>index 3ec81924..d92444fd 100644
>>--- a/poke/pk-mi-json.c
>>+++ b/poke/pk-mi-json.c
>>@@ -26,6 +26,20 @@
>> #include "pk-mi.h"
>> #include "pk-mi-json.h"
>> #include "pk-mi-msg.h"
>>+#include "libpoke.h"
>>+
>>+#define PK_MI_CHECK(errmsg, A, M, ...) \
>>+   do                                  \
>>+    {                                  \
>>+      if (!(A))                        \
>>+        {                              \
>>+          if (errmsg == NULL)          \
>>+            goto error;                \
>>+          assert (asprintf (errmsg, "[ERROR] " M , ##__VA_ARGS__) !=
>> -1); \
>>+          goto error;                  \
>>+        }                              \
>>+    }                                  \
>>+    while (0)
>>
>
> If somebody compiles the code with `-DNDEBUG`, the `asprintf` will be
> removed.
>
> And man-page of `asprintf` says that:
>
>      If memory allocation wasn't possible, or some other error occurs,
>      these functions will return -1, and the contents of strp are
> undefined.
>
> So my suggestion is something like this:
>
> ```c
> #define PK_MI_CHECK(errmsg, A, M, ...)    \
>     do                                     \
>      {                                     \
>        int r;                              \
>        if (A)                              \
>          break;                            \
>        if (errmsg != NULL &&               \
>            (r = asprintf (errmsg, "[ERROR] " M , ##__VA_ARGS__)) == -1) {
> \
>          *errmsg = NULL;                   \
>          assert(0 && "asprintf() failed"); \
>        }                                   \
>        goto error;                         \
>      }                                     \
>      while (0)
> ```
>
>> /* Message::
>>    {
>>@@ -421,7 +435,7 @@ pk_mi_msg
>> pk_mi_json_to_msg (const char *str)
>> {
>>   pk_mi_msg msg = NULL;
>>-  struct json_tokener *tokener;
>>+  json_tokener *tokener;
>>   json_object *json;
>>
>>   tokener = json_tokener_new ();
>>@@ -432,8 +446,875 @@ pk_mi_json_to_msg (const char *str)
>>   json_tokener_free (tokener);
>>
>>   if (json)
>>-    msg = pk_mi_json_object_to_msg (json);
>>+    {
>>+      msg = pk_mi_json_object_to_msg (json);
>>+      assert (json_object_put (json) == 1);
>>+    }
>
> Likewise. `json_object_put` has side-effect.
>
>
> Regards,
> Mohammad-Reza
>

Thanks for your comments Mohammad!

Jose, what do you think of this change for master? Shall I apply it and
then commit?

Thanks.