emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Accept plists when serializing and parsing JSON


From: Eli Zaretskii
Subject: Re: [PATCH] Accept plists when serializing and parsing JSON
Date: Sat, 02 Jun 2018 09:55:21 +0300

> From: João Távora <address@hidden>
> Cc: address@hidden
> Date: Sat, 02 Jun 2018 00:29:30 +0100
> 
> > Here I'd use intern_1 instead, it would allow you to avoid
> > unnecessarily consing Lisp objects.  (Yes, I realize that the same
> > comment applies to the existing code.)
> 
> Riight, intern_1 sounds good... I know I can't just malloc() stuff as
> usually right? But also, I have no idea what I'm doing, I aped this from
> somewhere where it looked more-or-less responsibly done.
> 
>   json_object_foreach (json, key_str, value)
>   {
>     USE_SAFE_ALLOCA;
>     char *keyword_key_str = SAFE_ALLOCA (1 + strlen(key_str) + 1);
>     keyword_key_str[0]=':';
>     strcpy(&keyword_key_str[1],key_str);
>     Lisp_Object key = intern_1(keyword_key_str, strlen(key_str)+1);
>     result = Fcons (key, result); /* build the plist as
>                                      value-key since
>                                      we're going to
>                                      reverse it in the
>                                      end.*/
>     result = Fcons (json_to_lisp (value, object_type), result);
>     SAFE_FREE ();
>   }

This looks OK to me, modulo a few stylistic gotchas:

  . use a temporary variable to compute strlen only once
  . leave one space _before_ an opening paren and around binary
    operators such as '+' and ',', as in, for example,

       intern_1 (keyword_key_str, strlen (key_str) + 1);
               ^                        ^         ^ ^
  . I personally prefer

        strcpy (keyword_key_str + 1, ...

     to

        strcpy (&keyword_key_str[1], ...

    although both do the same and are correct C
  . It is preferable to have long comments before the code, not at its
    side, split between several lines; and make the comment start with
    a capital letter, since it's a full sentence



reply via email to

[Prev in Thread] Current Thread [Next in Thread]