Re: [PATCH 2/6] jsmn: Add convenience functions

[Patrick Steinhardt]

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 functions
> > 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 that are
> > going to be used by the LUKS2 implementation later on.
> > 
> > Signed-off-by: Patrick Steinhardt <address@hidden>
> > ---
> >  include/grub/jsmn.h | 108 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 108 insertions(+)
> > 
> 
> Would it not make sense to keep the additions in a separate header 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 feel like the first one would be much cleaner, but I'd love
to hear what you think about this first.

Patrick

signature.asc
Description: PGP signature