Re: [PATCH] JSON schema refactoring and adaptive code changes.

[Mohammad-Reza Nabipoor]

Hi, Kostas!

Other than my comments below (3 minor issues), this is OK for master.
Well done!


Regards,
Mohammad-Reza


diff --git a/poke/pk-mi-json.c b/poke/pk-mi-json.c
index 297de22b..4286a839 100644
--- a/poke/pk-mi-json.c
+++ b/poke/pk-mi-json.c


+static int
+json_to_pk_type (json_object *j_type, pk_val *pk_type, char **errmsg)
 {
   static const struct
   {
-    int type_code;
     const char *name;
-  } TYPES[] = {
-    { PK_UNKNOWN, "\"Unknown\"" },
-    { PK_INT, "\"Integer\"" },
-    { PK_UINT, "\"UnsignedInteger\"" },
-    { PK_STRING, "\"String\"" },
-    { PK_OFFSET, "\"Offset\"" },
-    { PK_ARRAY, "\"Array\"" },
-    { PK_STRUCT, "\"Struct\"" },
-    { PK_CLOSURE, "\"Closure\"" },
-    { PK_ANY, "\"Any\"" },
+    json_to_pk_type_func func;
+  } JSON_TO_PK_TYPE_FUNCTIONS[] = {
+    {"Integral", json_to_pk_type_integral},
+    {"String", json_to_pk_type_string},
+    {"Offset", json_to_pk_type_offset},
+    {"Struct", json_to_pk_type_struct},
+    {"Array", json_to_pk_type_array},
+    {"Any", json_to_pk_type_any}
   };
-  static const int TYPES_LEN = sizeof (TYPES) / sizeof (TYPES[0]);
-  int type_code;
-  const char *type_str;
-  json_object *j_type;
+  const char *code;
+  json_object *j_code;
+  int ret;
 
-  if (obj == NULL) {
-    *pval = PK_NULL;
-    return J_OK;
-  }
+#define JSON_TO_PK_TYPE_FUNCTIONS_LEN                                         \
+  (sizeof (JSON_TO_PK_TYPE_FUNCTIONS) / sizeof (JSON_TO_PK_TYPE_FUNCTIONS[0]))
 


I think a local variable is more appropriate instead of this macro.
`JSON_TO_PK_TYPE_FUNCTIONS` is a local variable, so its length also
should be a local variable.

We also have two other "function arrays": please make them local if
appropriate.



+static int
+json_to_pk_val_array (json_object *j_arr, pk_val pk_arr_type, pk_val *pk_arr,
+                      char **errmsg)
+{
+  pk_val pk_arr_tmp;
+  json_object *j_elems, *j_boffsets, *j_mapping;
+  size_t nelems;
+  int ret;
 
-  /* FIXME no support for null items */
-  assert (j_elem != NULL);
+#define ARR_JERR(cond) FAIL_IF ((cond) == J_NOK, errmsg, "invalid Array")
 
-  ARR_JERR (pexpect_aelem (j_elem, &p_value, &p_boffset, errmsg));
-  p_aelem_type = pk_typeof (p_value);
-  p_arr_type = pk_make_array_type (p_aelem_type, PK_NULL);
-  p_arr = pk_make_array (pk_make_uint (elems_len, 64), p_arr_type);
+  ARR_JERR (jexpect (j_arr, "elements", json_type_array, &j_elems, errmsg));
+  nelems = json_object_array_length (j_elems);
 
-  pk_array_insert_elem (p_arr, 0, p_value);
-  RETURN_ERR_IF (!pk_val_equal_p (pk_array_elem_boffset (p_arr, 0), p_boffset),
-                 errmsg, "invalid Array: boffset mismatch at index 0");
+  ARR_JERR (jexpect (j_arr, "boffsets", json_type_array, &j_boffsets, errmsg));
+  FAIL_IF (json_object_array_length (j_boffsets) != nelems, errmsg,
+           "invalid Struct: boffsets array length mismatch with type");
 
-  for (size_t i = 1; i < elems_len; ++i)
+  pk_arr_tmp = pk_make_array (pk_make_uint (0, 64), pk_arr_type);
+  for (size_t i = 0 ; i < nelems ; i++)
     {
+      pk_val elem, etype;
+      json_object *j_elem;
+
       j_elem = json_object_array_get_idx (j_elems, i);
-      assert (j_elem != NULL);
-      ARR_JERR (pexpect_aelem (j_elem, &p_value, &p_boffset, errmsg));
-      pk_array_insert_elem (p_arr, i, p_value);
-      RETURN_ERR_IF (
-          !pk_val_equal_p (pk_array_elem_boffset (p_arr, i), p_boffset),
-          errmsg, "invalid Array: boffset mismatch at index %zu", i);
+      etype = pk_array_type_etype (pk_arr_type);



Please move this line outside of the loop.
Because `pk_arr_type` doesn't change inside the loop, and so the `etype`.



+      ARR_JERR (json_to_pk_val (j_elem, etype, &elem, errmsg));
+      pk_array_insert_elem (pk_arr_tmp, i, elem);
     }
 
-  ARR_JERR (pexpect_map (j_mapping, p_arr, errmsg));
-  *p_array = p_arr;
+  ARR_JERR (jexpect (j_arr, "mapping", json_type_object, &j_mapping, errmsg));
+
+  ARR_JERR (json_to_pk_val_map (j_mapping, pk_arr_tmp, errmsg));
+  *pk_arr = pk_arr_tmp;
+
   return J_OK;
 
 #undef ARR_JERR
 }
 
diff --git a/testsuite/poke.mi-json/pk_string_fail.json 
b/testsuite/poke.mi-json/pk_string_fail.json
new file mode 100644
index 00000000..371a7295
--- /dev/null
+++ b/testsuite/poke.mi-json/pk_string_fail.json
@@ -0,0 +1,9 @@
+##
+"foo"
+##
+{
+    "type": {
+        "code": "String",


The "," at the end makes this JSON invalid.
Please remove the comma.


+    },
+    "value": "foobar"
+}