[PATCH] Use get_key_arg to parse keyword arguments in json_parse_object_type

[PATCH] Use get_key_arg to parse keyword arguments in json_parse_object_type

[Vibhav Pant]

* src/fns.c (get_key_arg): Make function non-static.

* src/lisp.h: Add extern declaration for get_key_arg.

* src/json.c (json_parse_object_type): Use get_key_arg to get the
  value of `:object-type'.
---
 src/fns.c  |  2 +-
 src/json.c | 43 +++++++++++++++++++++++--------------------
 src/lisp.h |  1 +
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/src/fns.c b/src/fns.c
index 9db9bea9f7..6193319228 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -3558,7 +3558,7 @@ next_almost_prime (EMACS_INT n)
    0.  This function is used to extract a keyword/argument pair from
    a DEFUN parameter list.  */
 
-static ptrdiff_t
+ptrdiff_t
 get_key_arg (Lisp_Object key, ptrdiff_t nargs, Lisp_Object *args, char *used)
 {
   ptrdiff_t i;
diff --git a/src/json.c b/src/json.c
index 1c9bf6d49b..4fbeab186c 100644
--- a/src/json.c
+++ b/src/json.c
@@ -632,26 +632,29 @@ json_to_lisp (json_t *json, enum json_object_type object_type)
 static enum json_object_type
 json_parse_object_type (ptrdiff_t nargs, Lisp_Object *args)
 {
-  switch (nargs)
-    {
-    case 0:
-      return json_object_hashtable;
-    case 2:
-      {
-        Lisp_Object key = args[0];
-        Lisp_Object value = args[1];
-        if (!EQ (key, QCobject_type))
-          wrong_choice (list1 (QCobject_type), key);
-        if (EQ (value, Qhash_table))
-          return json_object_hashtable;
-        else if (EQ (value, Qalist))
-          return json_object_alist;
-        else
-          wrong_choice (list2 (Qhash_table, Qalist), value);
-      }
-    default:
-      wrong_type_argument (Qplistp, Flist (nargs, args));
-    }
+  ptrdiff_t i;
+  enum json_object_type type;
+
+  USE_SAFE_ALLOCA;
+  char *used = SAFE_ALLOCA (nargs * sizeof *used);
+  memset (used, 0, nargs * sizeof *used);
+  i = get_key_arg (QCobject_type, nargs, args, used);
+
+  if (!i)
+    type = json_object_hashtable;
+  else if (EQ (args[i], Qhash_table))
+    type = json_object_hashtable;
+  else if (EQ (args[i], Qalist))
+    type = json_object_alist;
+  else
+    wrong_choice (list2 (Qhash_table, Qalist), args[i]);
+
+  for (i = 0; i < nargs; i++)
+    if (!used[i])
+      signal_error ("Invalid argument list", args[i]);
+
+  SAFE_FREE ();
+  return type;
 }
 
 DEFUN ("json-parse-string", Fjson_parse_string, Sjson_parse_string, 1, MANY,
diff --git a/src/lisp.h b/src/lisp.h
index eb31ba209a..9c54913972 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3494,6 +3494,7 @@ extern void syms_of_syntax (void);
 /* Defined in fns.c.  */
 enum { NEXT_ALMOST_PRIME_LIMIT = 11 };
 extern EMACS_INT next_almost_prime (EMACS_INT) ATTRIBUTE_CONST;
+extern ptrdiff_t get_key_arg (Lisp_Object, ptrdiff_t, Lisp_Object *, char *);
 extern Lisp_Object larger_vector (Lisp_Object, ptrdiff_t, ptrdiff_t);
 extern void sweep_weak_hash_tables (void);
 extern char *extract_data_from_object (Lisp_Object, ptrdiff_t *, ptrdiff_t *);
-- 
2.15.1

Re: [PATCH] Use get_key_arg to parse keyword arguments in json_parse_object_type

[Vibhav Pant]

On Wed, Dec 20, 2017 at 12:09 PM, Vibhav Pant <vibhavp@gmail.com> wrote:
> * src/fns.c (get_key_arg): Make function non-static.
>
> * src/lisp.h: Add extern declaration for get_key_arg.
>
> * src/json.c (json_parse_object_type): Use get_key_arg to get the
>   value of `:object-type'.

Friendly ping, any feedback on this patch? I plan to submit another
patch to json.c,
which makes use of this one.

-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] Use get_key_arg to parse keyword arguments in json_parse_object_type

[John Wiegley]

>>>>> "VP" == Vibhav Pant <vibhavp@gmail.com> writes:

VP> Friendly ping, any feedback on this patch? I plan to submit another patch
VP> to json.c, which makes use of this one.

I approve of the patch, Vibhav. I'm suprised we don't have a SAFE_CALLOC
macro, though, to avoid having to use memset immediately after.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [PATCH] Use get_key_arg to parse keyword arguments in json_parse_object_type

[Paul Eggert]

John Wiegley wrote:
> I approve of the patch, Vibhav. I'm suprised we don't have a SAFE_CALLOC
> macro, though, to avoid having to use memset immediately after.

I guess Vibhav's future patches will make this issue moot, as the change doesn't 
make sense by itself; it merely substitutes a less-efficient implementation for 
a more-efficient one. Perhaps Vibhav's future changes will pass the argument 
without memsetting it, for example.

It would be helpful to know what the future changes are, as it's hard to tell 
whether the proposed patch is a good idea without seeing it in the context of 
the future patches.