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

[Jose E. Marchesi]

>> 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?

Sure.  The code as it is wont work with -DNDEBUG.