Re: [PATCH v5] Add JSON output options to 'iw' for scan results

Isaev Ruslan
2024-03-05T11:32:19Z

On Tue, 5 Mar 2024 at 13:31, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Tue, 2024-03-05 at 11:17 +0100, Nicolas Escande wrote:
>
> > For what it's worth jansson has been very good to me. It has printf() like
> > object creation which usually integrates well with c programs.
>
> Nice :)
>
> > With MIT license https://github.com/akheron/jansson/blob/master/LICENSE
>
> If we link it as a library rather than copying it, that wouldn't even
> matter (well unless it was GPL too, but even then you could still make a
> build without it if you don't want the result to become GPL.)
>
> > For me, having a "good" json representation means, as you pointed out, using the
> > right json underlying type and most of the time I'm afraid that means a complete
> > different code path because of the underlying type/container. It's always a
> > blurry line obviously but duplicating in a complete seperate function that only
> > does the right thing for json output may end up being cleaner codewise.
>
> Yeah, depends, I guess. I'd rather see less duplicating, and in the
> cases like in this patch it seems like it wouldn't even matter so much.
> But agree it depends on the objects.

I think i could make a universal printf() like function with
additional args for json representation.

Something like this:
before:
printf("\t * Manufacturer: %.*s\n", sublen, data + 4);

after:
printf_json("\t * Manufacturer", "%.*s", sublen, data + 4);

void printf_json(const char* key, const char* fmt, ...) {
char buf[1024];
va_list args;
va_start(args, format);

if (iw_json) {
vsnprintf(buf, sizeof(buf), format, args);
print_string(PRINT_JSON, key, "%s", buf);
} else {
printf("\t * %s: ", key);
vprintf(fmt, args);
printf("\n");
}

va_end(args);
}

>
> Just things like
>
> iw_json ? print_string(PRINT_JSON, "manufacturer", "%.*s", sublen, data + 4) : printf("\t * Manufacturer: %.*s\n", sublen, data + 4);
>
> seem like the worst of both worlds... More obviously perhaps where it's
> not even different:
>
> iw_json ? print_string(PRINT_JSON, NULL, "FT/IEEE 802.1X") : printf("FT/IEEE 802.1X");
>
> or
>
> iw_json ? print_string(PRINT_JSON, NULL, "%.02x-%.02x-%.02x:%d", data[0], data[1] ,data[2], data[3]) : printf("%.02x-%.02x-%.02x:%d", data[0], data[1] ,data[2], data[3]);
>
>
> Whereas with something like
>
> iw_json ? print_string(PRINT_JSON, "response_type", "%d%s", val, val == 3 ? " (AP)" : "") : printf("\t * Response Type: %d%s\n", val, val == 3 ? " (AP)" : "");
>
> (ugh, I don't like those long lines)
>
> you probably don't even want to have the " (AP)" thing in the JSON, but
> make it a real integer type rather than a string with print_string()!
>
> But we could make that
>
> output_int("response_type", "Response Type", val);
> if (val == 3)
> hint("(AP)");
>
> and the hint() just doesn't do anything in JSON?
>
> And maybe even "response_type" is translated to "Response Type" for the
> human output so you don't need to add the other "Response Type" there.
> Dunno. I really wouldn't mind if in some places that meant we'd now
> output "Response Type" instead of "response type" or "Response type" or
> something.
>
> Or perhaps do it the other way around, so "Response Type" is lower-cased
> and illegal chars like " " are replaced by "_", although that makes the
> JSON representation less obvious in the code.
>
> Picking one arbitrarily, we'd have something
>
> output_int("Response Type", val);
> if (val == 3)
> hint("(AP)");
>
> which seems far nicer than
>
> iw_json ? print_string(PRINT_JSON, "response_type", "%d%s", val, val == 3 ? " (AP)" : "") : printf("\t * Response Type: %d%s\n", val, val == 3 ? " (AP)" : "");
>
> in both code and JSON representation.
>
> Yes we'll have to do something about the \n and indentation and I know
> I'm handwaving about that, but it seems like with open_object() and
> close_object() etc. that need to keep a state stack anyway, that should
> be solvable?
>
> I'd argue that we should try to get somewhere around 80-90% of the cases
> unified in this matter, and then have special things for the rest that
> are special kinds of objects, etc.
>
> Right now it's pretty much 100% special cases.
>
> johannes