From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1idHLM-0007Yw-N4 for mharc-grub-devel@gnu.org; Fri, 06 Dec 2019 12:24:40 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:58322) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idHLJ-0007St-1O for grub-devel@gnu.org; Fri, 06 Dec 2019 12:24:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1idHLH-0005FT-DP for grub-devel@gnu.org; Fri, 06 Dec 2019 12:24:36 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:57155) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1idHLG-000599-Oc for grub-devel@gnu.org; Fri, 06 Dec 2019 12:24:35 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id F058422A66; Fri, 6 Dec 2019 12:24:32 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Fri, 06 Dec 2019 12:24:32 -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=1mK5DrQmDpWDUsVLa/fclYSLm5a wEggXPOwUqRCVF+8=; b=4kuXj/vlr3JSx05hYlr4XpC6RuQ2zF0obo49viintrW b22ZYe2tzhS1H8QUmLHqH4092LRnXqctIYKHWh3klO91PLlGC7OelRObXc1CIJMk 9SUsCBRieRDHwqInUtLzynj+QqFSqXcN/VZSEeAFvtSDmitDRbW4YnuxnzS3SEdk MrxqlYqdrdWYDbu3JOnj0svzuU5Lms76coZEZgV7xuyp6UNcAr1OcpCY/Df7iJaD 3fobqIOhCnGqfkS4tlwxvs56ISAMgMJXD1cdFtlT+gGfrHpfJKWKhpLTxZ0CEBvm qHmKlmwB3C180wGgbjBuW8ShXoGm6uGuU+0ApMKvmyQ== 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=1mK5Dr QmDpWDUsVLa/fclYSLm5awEggXPOwUqRCVF+8=; b=juoKLlOxhZKGuReHMH8GO6 ijv/lurK2wn0SHGirziAH3W3UE52ox4ezqJvS60rgjzHHkx69zm1xlWmqP9KbUPz UYLl155nDhHbX/JlTBd0BUbC/cCjJdFsnxLlAYYvlyoxSvDBPITV/AvzUuk0FoP0 AWyOW9omLMZ+J7RW2oqsi0HKeYlouwvLM9LrxyQjQ8qeEdp87Sd6ftvQqaoW/ezl PEetv61R1atmEDO9iHqkVUq9D5c/Y6PrPCqqKEt1qsDofos4RIOmiWbEyzIxMuaI IWtTbq68wu97ztUQAYO7fXGKX7kyIiDOO+lSO76k6sk/um5da0I99liZ7+4IK80w == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrudekfedgleehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucfkphepjeejrdduke efrddvfeekrdduieeknecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm necuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from vm-mail (x4db7eea8.dyn.telefonica.de [77.183.238.168]) by mail.messagingengine.com (Postfix) with ESMTPA id 17EC430600AA; Fri, 6 Dec 2019 12:24:31 -0500 (EST) Received: from localhost ( [10.192.0.11]) by vm-mail (OpenSMTPD) with ESMTPSA id 34582057 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 6 Dec 2019 17:24:28 +0000 (UTC) Date: Fri, 6 Dec 2019 18:24:28 +0100 From: Patrick Steinhardt To: Daniel Kiper Cc: grub-devel@gnu.org, Max Tottenham Subject: Re: [PATCH v5 2/6] json: Implement wrapping interface Message-ID: <20191206172428.GA7418@ncase> References: <1859ff9824ecb60a8d5307a51ce7c0c938a7beba.1575010112.git.ps@pks.im> <20191129153458.rwjhtjjxanbpqso3@tomti.i.net-space.pl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="/04w6evG8XlLl3ft" Content-Disposition: inline In-Reply-To: <20191129153458.rwjhtjjxanbpqso3@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.26 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: Fri, 06 Dec 2019 17:24:39 -0000 --/04w6evG8XlLl3ft Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 29, 2019 at 04:34:58PM +0100, Daniel Kiper wrote: > On Fri, Nov 29, 2019 at 07:51:46AM +0100, Patrick Steinhardt wrote: [snip] > > +grub_err_t > > +grub_json_getsize (grub_size_t *out, const grub_json_t *json) > > +{ > > + int size; >=20 > I hope that ((jsmntok_t *)json->tokens)() returns int... Yeah, the JSON token's size is stored as an int. > > + if (!json) > > + return GRUB_ERR_BAD_ARGUMENT; >=20 > Hmmm... I am looking at this and I have a feeling that we are not > consistent. If we want to be consistent we should check "out" for NULL > too. Same below. However, this complicates the code needlessly. How jsmn > copes with NULL? Does it care at all? Maybe we should just drop these > checks and state clearly somewhere in docs/comments that the caller must > provide valid pointers. Full stop! jsmn doesn't care at all, as it doesn't provide any accessing functions but the parsing code, only. Thus every NULL pointer handling needs to be done by the user. I'm fine with just saying "Give us valid pointers" as it was in my initial design. > > + *out =3D (size_t) size; >=20 > s/size_t/grub_size_t/ >=20 > However, TBH I would prefer grub_ssize_t here... Instead of the error return code or in addition to that? It would require the caller to always check the returned size for negative values. > > +static grub_err_t > > +get_value (grub_json_type_t *out_type, const char **out_string, const = grub_json_t *parent, const char *key) > > +{ > > + const grub_json_t *p =3D parent; > > + grub_json_t child; > > + grub_err_t ret; > > + jsmntok_t *tok; > > + > > + if (!parent) > > + return GRUB_ERR_BAD_ARGUMENT; > > + > > + if (key) > > + { > > + ret =3D grub_json_getvalue (&child, parent, key); > > + if (ret) > > + return ret; > > + p =3D &child; > > + } > > + > > + tok =3D &((jsmntok_t *) p->tokens)[p->idx]; > > + p->string[tok->end] =3D '\0'; >=20 > Are you sure that tok is never NULL? Yeah, it cannot be as we're directly taking the address after indexing into the array. What _could_ happen is that somebody provided an invalid parent with a bogus index, but that would again only be possible by misuse of the API. Patrick --/04w6evG8XlLl3ft Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEtmscHsieVjl9VyNUEXxntp6r8SwFAl3qjssACgkQEXxntp6r 8SxNog//ZtAjoNzR1ERCKY1n/ihsX9Ym+yL2qTu/IzQCue0XjCGPHgR79O9RFIdv Bor+nah6ps8xL7cz6ybsVU31TF7bI4gShs3HTY1ZbublDWiC/O5g4V5IO0BTnB9G Ssr9BK4ZbPhnmvYogcDG7ua2sfq9wsS75lOKrewGS0yXgUmJzVF3CZvlajl2u10Q LGuPMNVM5mING1F247HUluFR+BSA5ExUJ8wE/0CDfXmh7XVl9iqAaB4v2xFUk7cE OpuzKxkjC6iSIAaL5yNwXTCFzbTF4i6p4Z18JC/06Apd19CHpZPaMfXxfMnx8I9f s4OERnZxOFWF/cnNYcvUtIVWbk/DjPUZMNdol3t4zZWSV8r3OvyAPW+Sd76ZgBR4 sgM0AYZ1Ljoo5DPCn+qmDhC8j5YR1qmz/TuryHRSSLc60Df9hAhBEkg9YOVos+g3 4o1xu+DO4N8UdRE3fAnG3h1bjoHQeKtGEYhp59Q/yae211JKXDBgtQ7LBjWsY5PM IEKjXwY5klWj5BwxopbOG3T3osxuqRjOxKQbdNp7ETM5qbmQGTQcrIR1G5tq7rMa cU8KP6464iUAzxFo3gmLOoL6qmTfM6EMFQjqiDBALz8Wj9pdwP3KFR/aAiUNfRBw WTsnO4P+mzxeRDGJIs37KZ0w7y0vv7wkPtmb/RYiZOG1JtLw7JY= =6rdM -----END PGP SIGNATURE----- --/04w6evG8XlLl3ft--