On Fri, Nov 08, 2019 at 01:30:28PM +0000, Max Tottenham wrote:
> 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.I've created a pull request upstream to change exactly that -- let's wait and see. In the meantime we'll have to use the current `void *` workaround, but I'll make sure to update this as soon as the PR got merged (if ever). Patrick
signature.asc
Description: PGP signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
