[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] pk_mi_json convert functions and corresponding tests.
From: |
Konstantinos Chasialis |
Subject: |
Re: [PATCH] pk_mi_json convert functions and corresponding tests. |
Date: |
Wed, 30 Sep 2020 14:17:30 +0300 |
User-agent: |
SquirrelMail/1.4.23 [email.uoa.gr] |
> 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.