On Tue, 5 Mar 2024 at 13:31, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> 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
