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 | mtott...@akamai.com
Senior Software Engineer, Server Platform Engineering
/(* Akamai Technologies
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
