Re: [PATCH v2 2/6] json: Implement wrapping interface

[Max Tottenham]

On 11/07, Patrick Steinhardt wrote:
> On Thu, Nov 07, 2019 at 02:51:30AM +0000, Max Tottenham via Grub-devel wrote:
> > On 11/06, Patrick Steinhardt wrote:
> > > 
> > > The reason is that we'd have to include "jsmn.h" in
> > > "include/grub/json.h" to make `jsmntok_t` available. I definitely
> > > want to avoid this in order to not leak any decls from jsmn into
> > > users of "json.h" and to be able to have "jsmn.h" live in
> > > "grub-core/lib/json". It's also not possible to have a forward
> > > declaration here due to how `jsmntok_t` is declared.
> > > 
> > > Patrick
> > 
> > 
> > In which case, looking again at this patchset - grub_json_t
> > is always used as an opaque value by consumers of json.h .
> > 
> > Why not put a forward declaration of grub_json_t in include/grub/json.h,
> > and stick the implementation in grub-core/lib/json.c :
> > 
> >   include/grub/json.h:
> > 
> >     typedef struct grub_json_t grub_json_t;
> >   
> >   grub-core/lib/json.c:
> >   ...
> >   #include <grub/json.h>
> >   #include "jsmn.h"
> >   ...
> >   struct grub_json_t {
> >         jsmntok_t* tokens;
> >         ...
> >   };
> > 
> > 
> > Unless there is something I'm missing doesn't the above achieve the
> > desired result? 
> 
> If we'd do that, all calls to `grub_json_get{child,value}` would
> have to return a dynamically allocated `grub_json_t` structure as
> the storage size of `grub_json_t` would not be known at compile
> time and thus cannot be allocated on the caller's stack. Right
> now, the caller can just declare the structure on-stack and have
> the interface fill up these structures for him, without any need
> for dynamic memory allocation.
> 
> Patrick

Ah, good point. I've been thinking about this for a while and I think
dealing with casts from void* really is the best we can do.

Perhaps jsmn upstream would accept a patch that does:

  - typedef struct {
  + typedef struct jsmntok_t {
    ...
    } jsmntok_t;

If they do we could revisit this later.
  
-- 
Max Tottenham       | address@hidden
Senior Software Engineer, Server Platform Engineering
/(* Akamai Technologies