From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Accept plists when serializing and parsing JSON Date: Sat, 02 Jun 2018 09:55:21 +0300 Message-ID: <8336y5fzja.fsf@gnu.org> References: <87sh6awls5.fsf@gmail.com> <83o9gug811.fsf@gnu.org> <87d0xaozl1.fsf@gmail.com> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1527922411 26749 195.159.176.226 (2 Jun 2018 06:53:31 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 2 Jun 2018 06:53:31 +0000 (UTC) Cc: emacs-devel@gnu.org To: =?utf-8?B?Sm/Do28gVMOhdm9yYQ==?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jun 02 08:53:27 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fP0Pn-0006ps-2k for ged-emacs-devel@m.gmane.org; Sat, 02 Jun 2018 08:53:27 +0200 Original-Received: from localhost ([::1]:58533 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fP0Rs-0007O2-5n for ged-emacs-devel@m.gmane.org; Sat, 02 Jun 2018 02:55:36 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:48730) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fP0Re-0007MR-OO for emacs-devel@gnu.org; Sat, 02 Jun 2018 02:55:23 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fP0Rb-0000Vc-LO for emacs-devel@gnu.org; Sat, 02 Jun 2018 02:55:22 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:41735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fP0Rb-0000VP-HS; Sat, 02 Jun 2018 02:55:19 -0400 Original-Received: from [176.228.60.248] (port=1555 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1fP0Ra-0001dl-Pl; Sat, 02 Jun 2018 02:55:19 -0400 In-reply-to: <87d0xaozl1.fsf@gmail.com> (message from =?utf-8?B?Sm/Do28g?= =?utf-8?B?VMOhdm9yYQ==?= on Sat, 02 Jun 2018 00:29:30 +0100) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:225911 Archived-At: > From: João Távora > Cc: emacs-devel@gnu.org > 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