Received-SPF: pass (google.com: domain of linux-wireless+bounces-4373-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <37398c88130c487be656eed4e378d6b047a3d612.camel@sipsolutions.net>
Subject: Re: [PATCH v5] Add JSON output options to 'iw' for scan results
From: Johannes Berg <johannes@sipsolutions.net>
To: Nicolas Escande <nico.escande@gmail.com>, Isaev Ruslan
	 <legale.legale@gmail.com>, linux-wireless@vger.kernel.org
Date: Tue, 05 Mar 2024 11:31:22 +0100
In-Reply-To: <CZLQMMSSUWBF.1NPJJYDRIXGIR@gmail.com>
References: <5c5be485dcfceb44fc731e47758d6be3.legale.legale@gmail.com>
	 <12f91b008d6f2b4e6241794d2852c1c84fe6c12c.camel@sipsolutions.net>
	 <CZLQMMSSUWBF.1NPJJYDRIXGIR@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

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() lik=
e
> 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, us=
ing the
> right json underlying type and most of the time I'm afraid that means a c=
omplete
> different code path because of the underlying type/container. It's always=
 a
> blurry line obviously but duplicating in a complete seperate function tha=
t 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.

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/IE=
EE 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 =3D=
=3D 3 ? " (AP)" : "") : printf("\t * Response Type: %d%s\n", val, val =3D=
=3D 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 =3D=3D 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 =3D=3D 3)
		hint("(AP)");

which seems far nicer than

	iw_json ? print_string(PRINT_JSON, "response_type", "%d%s", val, val =3D=
=3D 3 ? " (AP)" : "") : printf("\t * Response Type: %d%s\n", val, val =3D=
=3D 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