Re: [PATCH 1/3] hs20: Debugging and comments.

On Fri, Mar 06, 2020 at 02:58:11PM -0800, [email protected] wrote:
> Try to make some of the debugging messages a bit more unique so it
> is easier to figure out what is happening.

> diff --git a/hs20/client/osu_client.c b/hs20/client/osu_client.c
> @@ -2276,7 +2276,7 @@ static int osu_connect(struct hs20_osu_client *ctx, const char *bssid,
>  	if (res < 0) {
> -		wpa_printf(MSG_INFO, "Could not connect");
> +		wpa_printf(MSG_INFO, "Could not connect to OSU network");
>  		write_summary(ctx, "Could not connect to OSU network");
>  		wpa_printf(MSG_INFO, "Remove OSU network connection");
>  		snprintf(buf, sizeof(buf), "REMOVE_NETWORK %d", id);

This part would seem to make sense.

> @@ -3470,7 +3470,7 @@ int main(int argc, char *argv[])
> -		wpa_printf(MSG_INFO, "Launch web browser to URL %s",
> +		wpa_printf(MSG_INFO, "main: Launch web browser to URL %s",
>  			   argv[optind + 1]);
>  		ret = hs20_web_browser(argv[optind + 1], ctx.ignore_tls);

But this looks quite pointless taken into account this is entered only
in the special case of hs20-osu-client being used as a standalone web
browser, i.e., never during normal Hotspot 2.0 sequences.

> diff --git a/hs20/client/spp_client.c b/hs20/client/spp_client.c
> @@ -545,8 +545,8 @@ static int hs20_spp_exec(struct hs20_osu_client *ctx, xml_node_t *exec,
> -		wpa_printf(MSG_INFO, "Launch browser to URI '%s'", uri);
> -		write_summary(ctx, "Launch browser to URI '%s'", uri);
> +		wpa_printf(MSG_INFO, "spp-exec: Launch browser to URI '%s'", uri);
> +		write_summary(ctx, "spp-exec: Launch browser to URI '%s'", uri);
>  		res = hs20_web_browser(uri, 1);

This is the normal Hotspot 2.0 sequence case for the browser..

> diff --git a/hs20/server/ca/ocsp-update-cache.sh b/hs20/server/ca/ocsp-update-cache.sh
> index 8ddef9b9c..8a9a665f4 100755
> --- a/hs20/server/ca/ocsp-update-cache.sh
> +++ b/hs20/server/ca/ocsp-update-cache.sh
> @@ -1,5 +1,6 @@
>  #!/bin/sh
>  
> +# NOTE:  You may need to replace 'localhost' with your ocsp hostname instead.
>  openssl ocsp \
>  	-no_nonce \
>  	-CAfile ca.pem \

This does not seem to belong into this patch.

> diff --git a/src/utils/browser.c b/src/utils/browser.c
> @@ -63,6 +63,9 @@ static void process_request_starting_uri(struct browser_context *ctx,
>  {
>  	int quit = 0;
>  
> +	wpa_printf(MSG_DEBUG, "BROWSER: process-request-starting-uri: %s  started: %d\n",
> +		   uri, ctx->gtk_main_started);

And this looks repetitive.. All the callers of this function are already
debug printing the URL. And there should not be a \n at the end of the
wpa_printf() string.

-- 
Jouni Malinen                                            PGP id EFC895FA