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

[Chad Versace]

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> wrote:
>> From: Dylan Baker <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>
>> ---
>>  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.

>> +    GLint context_flags = 0;
>> +
>> +    printf("    \"context flags\": ");
>> +
>> +    glGetIntegerv(GL_CONTEXT_FLAGS, &context_flags);
>> +    if (glGetError() != GL_NO_ERROR) {
>> +        printf("\"WFLINFO_GL_ERROR\"\n");
>> +        return;
>> +    }
>> +
>> +    if (context_flags == 0) {
>> +        printf("\"0x0\"\n");
>> +        return;
>> +    }

The json type of the "context flags" value is sometimes a string, sometimes an array.
It should always be the same type, an array. When there are no flags, I think it should
be an empty array.

>> +
>> +    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.

>> +
>> +    return;

In C, (at least in the C projects I've worked with), the dominant code style omits
the final return in void functions. The return is implicit.

>> +}



>> +    const char * vendor =  get_vendor();
>> +    const char * renderer = get_renderer();
>> +    const char * version_str = get_version();

In Waffle, there should be no trailing space after the '*'. In other words, the
correct style is:

    const char *vendor = ...



> 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.