Re: [PATCH] Respect EnableOnlineCheck globally

[Daniel Wagner <wagi@monom.org>]

Hi Страхиња,

On Sat, Aug 28, 2021 at 07:34:27PM +0200, Страхиња Радић wrote:
> On 21/08/28 07:16, Страхиња Радић wrote:
> > +	if (connman_setting_get_bool("EnableOnlineCheck")) {
> 
> This should read
> 
> +       if (!connman_setting_get_bool("EnableOnlineCheck")) {
> 
> sorry. I'm sending the modified patch with this email.
> 
> ---
>  src/wispr.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/wispr.c b/src/wispr.c
> index c63dc819..7b52914e 100644
> --- a/src/wispr.c
> +++ b/src/wispr.c
> @@ -916,6 +916,11 @@ static int wispr_portal_detect(struct
> connman_wispr_portal_context *wp_context)
>                 wp_context->status_url = STATUS_URL_IPV6;
>         }
> 
> +       /* Respect EnableOnlineCheck globally */
> +       if (!connman_setting_get_bool("EnableOnlineCheck")) {
> +               wp_context->status_url = "localhost";
> +       }
> +

This looks like the wrong place to break the online check. We should not
start the online check state machine at all. What about something like:

diff --git a/src/service.c b/src/service.c
index eb74a85d6e32..d04933070c61 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1618,15 +1618,18 @@ static void default_changed(void)
 				connman_setting_get_bool("AllowDomainnameUpdates"))
 			__connman_utsname_set_domainname(service->domainname);
 
-		if (__connman_service_is_connected_state(service,
+		if (connman_setting_get_bool("EnableOnlineCheck")) {
+
+			if (__connman_service_is_connected_state(service,
 						CONNMAN_IPCONFIG_TYPE_IPV4))
-			__connman_service_wispr_start(service,
-						CONNMAN_IPCONFIG_TYPE_IPV4);
+				__connman_service_wispr_start(service,
+							CONNMAN_IPCONFIG_TYPE_IPV4);
 
-		if (__connman_service_is_connected_state(service,
+			if (__connman_service_is_connected_state(service,
 						CONNMAN_IPCONFIG_TYPE_IPV6))
-			__connman_service_wispr_start(service,
-						CONNMAN_IPCONFIG_TYPE_IPV6);
+				__connman_service_wispr_start(service,
+							CONNMAN_IPCONFIG_TYPE_IPV6);
+		}
 
 		/*
 		 * Connect VPN automatically when new default service


This was the only place I found where we call
__connman_service_wispr_start() without the EnableOnlineCheck test.

Could you give this patch a spin?

Daniel