From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1iSL3R-0008HQ-Tk for mharc-grub-devel@gnu.org; Wed, 06 Nov 2019 08:08:57 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:37288) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iSL3O-0008Ba-3X for grub-devel@gnu.org; Wed, 06 Nov 2019 08:08:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iSL3M-0006UU-Ed for grub-devel@gnu.org; Wed, 06 Nov 2019 08:08:53 -0500 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:59125) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iSL3L-0006Tr-Tf for grub-devel@gnu.org; Wed, 06 Nov 2019 08:08:52 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 3F878220DD; Wed, 6 Nov 2019 08:08:51 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Wed, 06 Nov 2019 08:08:51 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm2; bh=iyfJ1K5Qx7zfEciIBR6NM0o5zkV jqyF6l37mn4XVMAk=; b=C1d1OjnYSD/7LrZdTnPR4GhoeSwrekTdA1QYWfdYVfq 0gs0VsRDrbAaRvyofnG122hOnv7RJYksTlwok5EQpCj1aNFK0TUzj2DukMqUnKZd PYqmkdvi0D9ZvXFlchkYHkjdJH5xjhBVq51IxIPnO7vqONNUoiu2NQnmTcAVkRaX Mg88kLnhKYlCbdbdnPCyX5Iq/Mz5/7EqmpUDsWjASX6S8UzRDF4caDeyPEhZzrw3 IaaqrQK90lsjxNdX2pbtuAcBSpdNduHqnDJDwDG7N6aJzY8bRJtP1mHPPX9G1gSy GIPPchXMnY7I+IAJLi+ODmAhxGMUEhDQ5XKxStrMVzA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=iyfJ1K 5Qx7zfEciIBR6NM0o5zkVjqyF6l37mn4XVMAk=; b=rY3mCLPgV5jhrday2px+/y YzI4UUddW+0dU0EQ+rgKc0HqSczjzvd7kX4bnFdE/4fGJtbC1xb0lnuVBEQrvd/T XKJV+EDQY7Y2XBx+wP2yQtC9wVtmTiOb0wQsl0WaFDhst3/gvbKYUF6UdE+Wankd bIy4GSdbfvpT3sR/h67hdj7aEYBOCGPUWUhPh6rjvObpMoh4VqJIJUK0mdZEXzn9 IK9wTKzmOtHtrpiye6gasd+CySbDe1cfc9FD7UBtnL2+vT9+c6da1nS0whoNYmZc 4sEB36eMETUBD4EaIO4Kq0QlvoaS9LghbMAP1a2YwKmIYQDbBqfqMHtrG6pPTZIQ == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedruddujedggeelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucfkphepkeelrdduvd drudduhedrudeinecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmnecu vehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from NSJAIL (x590c7310.dyn.telefonica.de [89.12.115.16]) by mail.messagingengine.com (Postfix) with ESMTPA id 13D4A3060060; Wed, 6 Nov 2019 08:08:49 -0500 (EST) Received: from localhost (10.192.0.11 [10.192.0.11]) by NSJAIL (OpenSMTPD) with ESMTPSA id 646d2bf4 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 6 Nov 2019 13:08:48 +0000 (UTC) Date: Wed, 6 Nov 2019 14:08:48 +0100 From: Patrick Steinhardt To: Daniel Kiper Cc: Max Tottenham , The development of GNU GRUB Subject: Re: [PATCH 2/6] jsmn: Add convenience functions Message-ID: <20191106130848.GA38529@ncase> References: <20191104102620.GA923@akamai.com> <20191104110053.GA2673@ncase> <20191104174251.u2y7dtkj4rvg5zkt@tomti.i.net-space.pl> <20191104185648.GA2540@ncase> <20191106114458.a3452zya7p2nm3tt@tomti.i.net-space.pl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="dDRMvlgZJXvWKvBx" Content-Disposition: inline In-Reply-To: <20191106114458.a3452zya7p2nm3tt@tomti.i.net-space.pl> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.111.4.27 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Nov 2019 13:08:56 -0000 --dDRMvlgZJXvWKvBx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 06, 2019 at 12:44:58PM +0100, Daniel Kiper wrote: > On Mon, Nov 04, 2019 at 07:56:48PM +0100, Patrick Steinhardt wrote: > > On Mon, Nov 04, 2019 at 06:42:51PM +0100, Daniel Kiper wrote: > > > On Mon, Nov 04, 2019 at 12:00:53PM +0100, Patrick Steinhardt wrote: > > > > On Mon, Nov 04, 2019 at 10:26:21AM +0000, Max Tottenham wrote: > > > > > On 11/02, Patrick Steinhardt wrote: > > > > > > The newly added jsmn library is a really bare-bones library that > > > > > > focusses on simplicity. Because of that, it is lacking some fun= ctions > > > > > > for convenience to abstract away some of its inner workings and= to make > > > > > > code easier to read. As such, we're now adding some functions t= hat are > > > > > > going to be used by the LUKS2 implementation later on. > > > > > > > > > > > > Signed-off-by: Patrick Steinhardt > > > > > > --- > > > > > > include/grub/jsmn.h | 108 ++++++++++++++++++++++++++++++++++++= ++++++++ > > > > > > 1 file changed, 108 insertions(+) > > > > > > > > > > > > > > > > Would it not make sense to keep the additions in a separate heade= r from > > > > > the vendored upstream library? That way it'll likely be easier to= pull > > > > > in any updates. > > > > > > > > Yeah, I thought about that, too. I wasn't sure about where > > > > "jsmn.h" and our own extension should live, though, which is why > > > > I just bailed for now and waited on some feedback. I could > > > > imagine two things: > > > > > > > > - We create "include/grub/json.h" with an API that uses > > > > GRUB's coding style. It would thus work as a wrapper around > > > > the jsmn API and contain functions like "grub_json_parse", > > > > "grub_json_get_object" and also a struct "grub_json_t" that > > > > contains all things required to work with the parsed JSON > > > > object. The implementation would be contained in > > > > "gurb-core/lib/json.c" and use "grub-core/lib/jsmn.h", > > > > which would be the unmodified upstream library. This would > > > > allow us to swap out the JSON parser in the future more > > > > easily without breaking any users and also make the API > > > > feel less foreign. > > > > > > > > - Or there is both a "include/grub/jsmn.h" and > > > > "include/grub/json.h", where the former one is the > > > > unmodified JSMN dependency and the latter one provides our > > > > own extended functions. > > > > > > I would like to see JSON functionality in a module. Good example is > > > in commit 461f1d8af (zstd: Import upstream zstd-1.3.6). So, please > > > create grub-core/lib/json and put jsmn.h there in unmodified form. > > > Then create json.c and put all required module and convenience > > > functions there. If you need some globally available stuff please > > > put it into include/grub/json.h. Last but not least, I want to ask > > > you to describe jsmn.h import steps in docs/grub-dev.texi. Good examp= le > > > is in commit 35b909062 (gnulib: Upgrade Gnulib and switch to bootstrap > > > tool). > > > > Thanks a lot for your advice. > > > > Just to make sure we're on the same page about the globally > > available stuff. I take you'd like to have "json.h" in > > "include/grub/json.h", but "json.h" will need to include "jsmn.h" > > with `#define JSMN_HEADER` in order to make struct and function > > declarations available. I guess it's kind of backwards to have > > headers in "include/" include headers from "grub-core/lib", but I > > can't really see a way around it without either re-declaring the > > same things as in "jsmn.h" or by creating a wrapping API around > > "jsmn.h". >=20 > Yeah, you are right that looks strange. So, let's go zstd way and > put everything into grub-core/lib/json. Then add required CC/CPP > flags to the Makefile as zstd module does. >=20 > Daniel Well, since writing this I've posted v2, which implements a complete wrapper around jsmn that looks and feels more like GRUB-style functions (as proposed above in my first bullet point). This actually does allow us to move "jsmn.h" into lib/ and the "json.h" lives in include/grub now. So while it does add a few more lines compared to v1, it seems a lot cleaner to me. Patrick --dDRMvlgZJXvWKvBx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEtmscHsieVjl9VyNUEXxntp6r8SwFAl3Cxd8ACgkQEXxntp6r 8Sy27w//WbSnghbwjM9x3Ka5zn4o4dUh2rfFRFbjT7qF/QfgWE1ROy7vmG+s6JXH kcE54BAjl27k8bi1uLYzE2Qg7SBVv51Zz3WENapPMiZcUNBobtsqxR9RA5avc0dg lbd/GwlRqaK3au1NAWFK+2XpYvsGV/XwQkvoDSDBwmjjleCf9ZbyTz4ws/JFD1YQ 5MxXN/Rd27erbgOMvtwlrcQd8qmPCRbs+thH9/iPwNXnTHmQkQllhHlA6ajoKDi3 kia/quxYiQ605xie5X5u4y2C+4Gl7HCixu331IsdQSexm+n3XtFO/vLk57EZqm8e 2aDaERfisY6HsgLkpBrRjVzw2Dza9CLpZAYaZ6hcPPlXsE3+StOh5e+Duw+2lIEO D78rwytyHHy/m6japtDAw/HXFQ0+AmVZ+KlyuSaIVPM8mliq7F1Zj3ZiKBZeIe+P xpImXCQyOhqL9VGPj9VRQ+n2XxZzbZZGOXCyj84bF8xvesDhrm03bJSC6Twa0Mqg P7tsEXaXcaeVn7Xe0pENmT8H4J+mTinUDyE1wwqai4TCcS8pWVKMUycbfDV08G3U dcAvfpDa5TeIqFfErykBJfi1wRQOMVEG6Ll4xbEtzmFuugGI9RXZcLULPi5AzN2t Xyp0BloLIYotKmbP+JRWbTb8P1HN7AhrPekS68hWx849eTNfDK8= =l4Pt -----END PGP SIGNATURE----- --dDRMvlgZJXvWKvBx--