Hi Pasi,
Could you resend the patch, through git send-email, so it's under the
proper format?
With a proper commit message and so on.
My comments below. Mostly code style issues but also one fix that should
be in another patch:
---
diff --git a/include/setting.h b/include/setting.h
index a882021..32ccf56 100644
--- a/include/setting.h
+++ b/include/setting.h
@@ -28,6 +28,9 @@
extern "C" {
#endif
+#define CONF_STATUS_URL_IPV6 "Ipv6StatusUrl"
+#define CONF_STATUS_URL_IPV4 "Ipv4StatusUrl"
+
bool connman_setting_get_bool(const char *key);
char **connman_setting_get_string_list(const char *key);
unsigned int *connman_setting_get_uint_list(const char *key);
diff --git a/src/6to4.c b/src/6to4.c
index 0e3a7a1..c32cef9 100644
--- a/src/6to4.c
+++ b/src/6to4.c
@@ -53,8 +53,6 @@ static unsigned int newlink_watch;
static unsigned int newlink_flags;
static int newlink_timeout_id;
-#define STATUS_URL "http://ipv6.connman.net/online/status.html";
-
#ifndef IP_DF
#define IP_DF 0x4000 /* Flag: "Don't Fragment" */
#endif
@@ -317,7 +315,9 @@ static void tun_newlink(unsigned flags, unsigned change,
void *user_data)
if (getenv("CONNMAN_WEB_DEBUG"))
g_web_set_debug(web, web_debug, "6to4");
- web_request_id = g_web_request_get(web, STATUS_URL,
+ const char *url =
connman_option_get_string(CONF_STATUS_URL_IPV6);
Too long line: the limit is 80 characters.
+
+ web_request_id = g_web_request_get(web, url,
web_result, NULL, NULL);
two tabs more here (ok it was like that before, but since you are
refactoring this part, let's fix it)
newlink_timeout(NULL);
diff --git a/src/main.c b/src/main.c
index 4f635de..0151d39 100644
--- a/src/main.c
+++ b/src/main.c
@@ -73,6 +73,8 @@ static struct {
bool single_tech;
char **tethering_technologies;
bool persistent_tethering_mode;
+ char *ipv6_status_url;
+ char *ipv4_status_url;
} connman_settings = {
.bg_scan = true,
.pref_timeservers = NULL,
@@ -86,6 +88,8 @@ static struct {
.single_tech = false,
.tethering_technologies = NULL,
.persistent_tethering_mode = false,
+ .ipv4_status_url = NULL,
+ .ipv6_status_url = NULL,
};
#define CONF_BG_SCAN "BackgroundScanning"
@@ -98,8 +102,10 @@ static struct {
#define CONF_BLACKLISTED_INTERFACES "NetworkInterfaceBlacklist"
#define CONF_ALLOW_HOSTNAME_UPDATES "AllowHostnameUpdates"
#define CONF_SINGLE_TECH "SingleConnectedTechnology"
-#define CONF_TETHERING_TECHNOLOGIES "TetheringTechnologies"
+#define CONF_TETHERING_TECHNOLOGIES "TetheringTechnologies"
#define CONF_PERSISTENT_TETHERING_MODE "PersistentTetheringMode"
+#define CONF_STATUS_URL_IPV6 "Ipv6StatusUrl"
+#define CONF_STATUS_URL_IPV4 "Ipv4StatusUrl"
static const char *supported_options[] = {
CONF_BG_SCAN,
@@ -114,6 +120,8 @@ static const char *supported_options[] = {
CONF_SINGLE_TECH,
CONF_TETHERING_TECHNOLOGIES,
CONF_PERSISTENT_TETHERING_MODE,
+ CONF_STATUS_URL_IPV4,
+ CONF_STATUS_URL_IPV6,
NULL
};
@@ -236,6 +244,8 @@ static void parse_config(GKeyFile *config)
char **interfaces;
char **str_list;
char **tethering;
+ char *ipv4url;
+ char *ipv6url;
You can use only one variable named status_url here (for instance), no
need to specify 2 different ones.
gsize len;
int timeout;
@@ -354,6 +364,23 @@ static void parse_config(GKeyFile *config)
connman_settings.persistent_tethering_mode = boolean;
g_clear_error(&error);
+
+ ipv4url = __connman_config_get_string(config, "General",
CONF_STATUS_URL_IPV4, &error);
+ if (!error)
+ connman_settings.ipv4_status_url = ipv4url;
+ else
+ connman_settings.ipv4_status_url =
"http://ipv4.connman.net/online/status.html";;
+
+ g_clear_error(&error);
+
+ ipv6url = __connman_config_get_string(config, "General",
CONF_STATUS_URL_IPV6, &error);
+ if (!error)
+ connman_settings.ipv6_status_url = ipv6url;
+ else
+ connman_settings.ipv6_status_url =
"http://ipv6.connman.net/online/status.html";;
+
+ g_clear_error(&error);
Too long lines here too.
+
}
static int config_init(const char *file)
@@ -510,6 +537,11 @@ const char *connman_option_get_string(const char *key)
else
return option_wifi;
}
One empty line needed here
+ if (g_str_equal(key, CONF_STATUS_URL_IPV4) == TRUE) {
+ return connman_settings.ipv4_status_url;
+ }
No need of the { }
and add an empty line also
+ if (g_str_equal(key, CONF_STATUS_URL_IPV6) == TRUE)
+ return connman_settings.ipv6_status_url;
As you did here properly.
return NULL;
}
diff --git a/src/main.conf b/src/main.conf
index 93c7a50..3b04f36 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -95,3 +95,8 @@
# re-enabling a technology, and after restarts and reboots.
# Default value is false.
# PersistentTetheringMode = false
+
+# Set the online status url which either returns "X-ConnMan-Status: online"
HTTP-header
Too long line.
+# or HTTP-status code 204.
+Ipv4StatusUrl = http://ipv4.connman.net/online/status.html
+Ipv6StatusUrl = http://ipv6.connman.net/online/status.html
diff --git a/src/wispr.c b/src/wispr.c
index dcce93c..27d5162 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -30,9 +30,6 @@
#include "connman.h"
-#define STATUS_URL_IPV4 "http://ipv4.connman.net/online/status.html";
-#define STATUS_URL_IPV6 "http://ipv6.connman.net/online/status.html";
-
struct connman_wispr_message {
bool has_error;
const char *current_element;
@@ -260,6 +257,8 @@ static const char *response_code_to_string(int
response_code)
return "Proxy detection/repeat operation";
case 201:
return "Authentication pending";
+ case 204:
+ return "Walled garden check";
This is not related to your patch.
case 255:
return "Access gateway internal error";
}
@@ -725,6 +724,9 @@ static bool wispr_portal_web_result(GWebResult *result,
gpointer user_data)
wp_context->redirect_url, wp_context);
break;
+ case 204:
+ portal_manage_status(result, wp_context);
+ return false;
Same here.
Make another patch for this code handling.
case 302:
if (!g_web_supports_tls() ||
!g_web_result_get_header(result, "Location",
@@ -872,10 +874,10 @@ static int wispr_portal_detect(struct
connman_wispr_portal_context *wp_context)
if (wp_context->type == CONNMAN_IPCONFIG_TYPE_IPV4) {
g_web_set_address_family(wp_context->web, AF_INET);
- wp_context->status_url = STATUS_URL_IPV4;
+ wp_context->status_url =
connman_option_get_string(CONF_STATUS_URL_IPV4);
Too long line.
} else {
g_web_set_address_family(wp_context->web, AF_INET6);
- wp_context->status_url = STATUS_URL_IPV6;
+ wp_context->status_url =
connman_option_get_string(CONF_STATUS_URL_IPV6);
Too long line.
}
for (i = 0; nameservers[i]; i++)
Tomasz
_______________________________________________
connman mailing list
[email protected]
https://lists.connman.net/mailman/listinfo/connman
