[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] pk_mi_json convert functions and corresponding tests.
From: |
Jose E. Marchesi |
Subject: |
Re: [PATCH] pk_mi_json convert functions and corresponding tests. |
Date: |
Wed, 30 Sep 2020 13:46:36 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
>> 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.