[waffle] [RFC 3/3] wflinfo.c: Add a --json flag that prints json

[Dylan Baker]

Okay. I'll send out a v2 shortly.

On Tue, Jan 5, 2016 at 9:24 AM, Chad Versace <chad.versace at intel.com> wrote:

> On 12/31/2015 09:51 AM, Dylan Baker wrote:
> >
> >
> > On Wed, Dec 30, 2015 at 4:32 PM, Chad Versace <chad.versace at intel.com
> <mailto:chad.versace at intel.com>> wrote:
> >
> >     On 12/27/2015 07:49 AM, Frank Henigman wrote:
> >     > On Wed, Dec 16, 2015 at 8:37 PM,  <baker.dylan.c at gmail.com
> <mailto:baker.dylan.c at gmail.com>> wrote:
> >     >> From: Dylan Baker <baker.dylan.c at gmail.com <mailto:
> baker.dylan.c at gmail.com>>
> >     >>
> >     >> This adds some code to print a JSON formatted version of data
> provided
> >     >> by wflinfo.
> >     >>
> >     >> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com <mailto:
> dylanx.c.baker at intel.com>>
> >     >> ---
> >     >>  src/utils/wflinfo.c | 174
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >
> >
> >
> >     >> +static void
> >     >> +json_print_context_flags(void)
> >     >> +{
> >     >> +    int flag_count = sizeof(flags) / sizeof(flags[0]);
> >
> >     This should be
> >
> >             const int flag_count = ARRAY_SIZE(flags);
> >
> >     Optionally, you could even remove the flag_count variable altogether
> >     and use ARRAY_SIZE(flags) everywhere. But it doesn't really matter.
> >
> >
> > This was copied from the original print_context_flags function. Should I
> chang ethat to ARRAY_SIZE as well (presumably in a separate patch)?
>
> Yes. It would be nice if you cleaned up print_context_flags in a separate
> patch.
>
> >     >> +
> >     >> +    printf("[\n");
> >     >> +    for (int i = 0; i < flag_count; i++) {
> >     >> +        if ((flags[i].flag & context_flags) != 0) {
> >     >> +            printf("        \"%s\"", flags[i].str);
> >     >> +            context_flags = context_flags & ~flags[i].flag;
> >     >> +            if (i != flag_count) {
> >     >> +                printf(",\n");
> >     >> +            }
> >     >> +        }
> >     >> +    }
> >     >> +    for (int i = 0; context_flags != 0; context_flags >>= 1,
> i++) {
> >     >> +        if ((context_flags & 1) != 0) {
> >     >> +            printf(",\n");
> >     >> +            printf("        0x%x", 1 << i);
> >     >> +        }
> >     >> +    }
> >     >> +    printf("    ]");
> >
> >     Like Frank said, it would be nice if the json code didn't duplicate
> so much of the
> >     original code. But, because you're not confident yet in C (as you
> say in the cover letter),
> >     I'll accept the duplication in this function. I'll offer to
> deduplicate the code myself if the patches
> >     go upstream.
> >
> >
> > I'm curious what the (or a) good approach would be to fix this. I
> > tried making a similar implementaiton in python (which I'm more
> > comfortable with) and still had a hard time coming up with a good way
> > to share code.
> >
> > Personally I find being thrown in the deep in the best way to learn
> > to swim, so if you could point me in the right (or at least a good)
> > direction I'd like to take a stab at it.
>
> Honestly, I also don't see a clean way to deduplicate the code for the
> variable-length fields. Perhaps careful use of template strings that the
> loops fill out with sprintf.
>
> I suggest you don't worry about deduplication in this patch series, and
> we can revisit it after the patches land. It would be silly to stall the
> json
> output due to duplication of a tiny chunk of code.
>
>
> >     > I think some key strings should be a bit more specific or nested,
> like
> >     > "opengl version:" or "opengl : { version :" instead of just
> "version"
> >     > in case we expand some day to include things like egl version.
> >
> >     I agree. I also think the non-OpenGL key strings ("platform" and
> "api")
> >     should be prefixed with "waffle", just as they are in the normal
> wflinfo
> >     output.
> >
> >
> > I started changing this locally after Frank's comments. My plan was
> > to nest waffle specific information inside a waffle dict, OpenGL
> > (including ES) inside of an opengl dict, and then leave room for
> > other information to be in it's own data structures (say glx or egl)
> >
> > something like:
> > {
> >      "waffle": {
> >           "platform": "gbm"
> >      },
> >      "gl": {
> >           "version": 3.1,
> >           "profile": "core"
> >      }
> > }
> >
> > Does that seem reasonable?
>
> That looks good to me.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/waffle/attachments/20160105/2cd2a99f/attachment.html>