[PATCH 1/1] Avoid invalid reads in dhcp_invalidate

2014-04-17 Thread Eduardo Abinader
dhcp_invalidate was freeing dhcp and duplicating network unref
(reported by Tomasz Bursztyka), thus causinhg invalid reads,
when ipv4ll_announce_timeout was triggered. The patch consists
of freeing dhcp only when dhcp is stopped and network removal
and unref are previously checked against network_list.
---
 src/dhcp.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index e4bac67..eb37cfe 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -86,7 +86,6 @@ static void dhcp_invalidate(struct connman_dhcp *dhcp, bool 
callback)
 {
struct connman_service *service;
struct connman_ipconfig *ipconfig;
-   bool network_removed = false;
int i;
 
DBG("dhcp %p callback %u", dhcp, callback);
@@ -132,18 +131,14 @@ static void dhcp_invalidate(struct connman_dhcp *dhcp, 
bool callback)
__connman_ipconfig_set_gateway(ipconfig, NULL);
__connman_ipconfig_set_prefixlen(ipconfig, 0);
 
-   if (dhcp->callback && callback) {
-   g_hash_table_remove(network_table, dhcp->network);
-   network_removed = true;
+   if (dhcp->callback && callback)
dhcp->callback(dhcp->network, false, NULL);
-   }
 
 out:
-   if (!network_removed)
+   if (g_hash_table_contains(network_table, dhcp->network)) {
g_hash_table_remove(network_table, dhcp->network);
-
-   connman_network_unref(dhcp->network);
-   dhcp_free(dhcp);
+   connman_network_unref(dhcp->network);
+   }
 }
 
 static void dhcp_valid(struct connman_dhcp *dhcp)
@@ -627,6 +622,7 @@ void __connman_dhcp_stop(struct connman_network *network)
if (dhcp) {
dhcp_release(dhcp);
dhcp_invalidate(dhcp, false);
+   dhcp_free(dhcp);
}
 }
 
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH 2/2] Avoid network duplicate unref for eth_dev_remote

2014-04-17 Thread Eduardo Abinader
Surely.

anyway, please check my first patch, applied in dhcp_invalidate.
I going to investigate further this issue you just mentioned.


BR.


On Thu, Apr 17, 2014 at 10:08 AM, Tomasz Bursztyka <
tomasz.burszt...@linux.intel.com> wrote:

> Thanks,
>
> Now it's possible to pin point which part is bogus:
>
> - connman_network_create() network 0x749d890
> - connman_network_ref_debug() 0x749d890 name Wired ref 2 by
> src/device.c:858:connman_device_add_network()
> - connman_network_ref_debug() 0x749d890 name Wired ref 3 by
> src/service.c:6670:update_from_network()
> - connman_network_ref_debug() 0x749d890 name Wired ref 4 by
> src/network.c:570:autoconf_ipv6_set()
> - connman_network_ref_debug() 0x749d890 name Wired ref 5 by
> src/dhcp.c:608:__connman_dhcp_start()
> - connman_network_unref_debug() 0x749d890 name Wired ref 4 by
> src/network.c:455:check_dhcpv6()
> - connman_network_unref_debug() 0x749d890 name Wired ref 3 by
> src/dhcp.c:145:dhcp_invalidate()
> - connman_network_unref_debug() 0x749d890 name Wired ref 2 by
> src/dhcp.c:145:dhcp_invalidate()
> - connman_network_unref_debug() 0x749d890 name Wired ref 1 by
> src/service.c:4455:service_free()
> - connman_network_unref_debug() 0x749d890 name Wired ref 0 by
> src/device.c:374:free_network()
> - connman_network_unref_debug() 0x749d890 name Wired ref -1 by
> plugins/ethernet.c:131:remove_network()
>
> Looks like the dhcp process is the culprit: it's unreferencing the network
> twice though it referenced it only once.
> 2 times dhcp_invalidate().
>
> Can you check what's the proper fix there and resend a patch?
>
> Thanks,
>
>
> Tomasz
>
>
> ___
> connman mailing list
> connman@connman.net
> https://lists.connman.net/mailman/listinfo/connman
>
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH 2/2] Avoid network duplicate unref for eth_dev_remote

2014-04-17 Thread Tomasz Bursztyka

Thanks,

Now it's possible to pin point which part is bogus:

- connman_network_create() network 0x749d890
- connman_network_ref_debug() 0x749d890 name Wired ref 2 by 
src/device.c:858:connman_device_add_network()
- connman_network_ref_debug() 0x749d890 name Wired ref 3 by 
src/service.c:6670:update_from_network()
- connman_network_ref_debug() 0x749d890 name Wired ref 4 by 
src/network.c:570:autoconf_ipv6_set()
- connman_network_ref_debug() 0x749d890 name Wired ref 5 by 
src/dhcp.c:608:__connman_dhcp_start()
- connman_network_unref_debug() 0x749d890 name Wired ref 4 by 
src/network.c:455:check_dhcpv6()
- connman_network_unref_debug() 0x749d890 name Wired ref 3 by 
src/dhcp.c:145:dhcp_invalidate()
- connman_network_unref_debug() 0x749d890 name Wired ref 2 by 
src/dhcp.c:145:dhcp_invalidate()
- connman_network_unref_debug() 0x749d890 name Wired ref 1 by 
src/service.c:4455:service_free()
- connman_network_unref_debug() 0x749d890 name Wired ref 0 by 
src/device.c:374:free_network()
- connman_network_unref_debug() 0x749d890 name Wired ref -1 by 
plugins/ethernet.c:131:remove_network()


Looks like the dhcp process is the culprit: it's unreferencing the 
network twice though it referenced it only once.

2 times dhcp_invalidate().

Can you check what's the proper fix there and resend a patch?

Thanks,

Tomasz


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH 2/2] Avoid network duplicate unref for eth_dev_remote

2014-04-17 Thread Eduardo Abinader
Hi Tomasz,

Connman logs: http://pastebin.com/qFGaw29x


Cheers.


On Thu, Apr 17, 2014 at 9:35 AM, Tomasz Bursztyka <
tomasz.burszt...@linux.intel.com> wrote:

> Hi Eduardo,
>
>
>  As you mentioned, the network reference device is freeing is the the one
>> it
>> creates, in this case, it is created in eth_newlink. I could not see any
>> network referenced by eth which is not via device, so that's the reason I
>> patched by removing network_unref and letting device manage it by its own.
>>
>
> connman_network_create() sets a reference to 1. That's the first reference.
> The one who create - then get that first reference - should unreference it.
> This is what ethernet plugin is doing, logically in
> add_network/remove_network.
>
> According to your valgrind output, you have found a reference issue, but
> your
> fix is not proper.
>
> There is most probably a place which is unreferencing one time too much the
> network, thus breaking the logic in plugins/ethernet.c
>
> Hopefully, it's possible to follow who calls ref/unref functions.
> Could you send your connman debug logs? It will help to clarify the
> valgrind output.
>
> Cheers,
>
>
> Tomasz
> ___
> connman mailing list
> connman@connman.net
> https://lists.connman.net/mailman/listinfo/connman
>
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH 2/2] Avoid network duplicate unref for eth_dev_remote

2014-04-17 Thread Tomasz Bursztyka

Hi Eduardo,


As you mentioned, the network reference device is freeing is the the one it
creates, in this case, it is created in eth_newlink. I could not see any
network referenced by eth which is not via device, so that's the reason I
patched by removing network_unref and letting device manage it by its own.


connman_network_create() sets a reference to 1. That's the first reference.
The one who create - then get that first reference - should unreference it.
This is what ethernet plugin is doing, logically in 
add_network/remove_network.


According to your valgrind output, you have found a reference issue, but 
your

fix is not proper.

There is most probably a place which is unreferencing one time too much the
network, thus breaking the logic in plugins/ethernet.c

Hopefully, it's possible to follow who calls ref/unref functions.
Could you send your connman debug logs? It will help to clarify the 
valgrind output.


Cheers,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH 2/2] Avoid network duplicate unref for eth_dev_remote

2014-04-17 Thread Eduardo Abinader
Hi Tomasz,

Below you find my valgrind output.

As you mentioned, the network reference device is freeing is the the one it
creates, in this case, it is created in eth_newlink. I could not see any
network referenced by eth which is not via device, so that's the reason I
patched by removing network_unref and letting device manage it by its own.


Cheers.

=
==23771== Invalid read of size 4
==23771==at 0x445A0F: connman_network_unref_debug (network.c:1037)
==23771==by 0x41F5A5: remove_network (ethernet.c:131)
==23771==by 0x41F86E: eth_dev_remove (ethernet.c:197)
==23771==by 0x4420C6: remove_device (device.c:295)
==23771==by 0x442149: remove_driver (device.c:310)
==23771==by 0x44229C: connman_device_driver_unregister (device.c:363)
==23771==by 0x41FCBD: ethernet_exit (ethernet.c:365)
==23771==by 0x440C45: __connman_plugin_cleanup (plugin.c:200)
==23771==by 0x43F5FB: main (main.c:697)
==23771==  Address 0x7917d20 is 0 bytes inside a block of size 232 free'd
==23771==at 0x4C2BCD7: free (vg_replace_malloc.c:469)
==23771==by 0x445863: network_destruct (network.c:968)
==23771==by 0x445A98: connman_network_unref_debug (network.c:1045)
==23771==by 0x44230B: free_network (device.c:374)
==23771==by 0x4E6D8F9: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3800.1)
==23771==by 0x44310F: connman_device_remove_network (device.c:901)
==23771==by 0x41F586: remove_network (ethernet.c:130)
==23771==by 0x41F86E: eth_dev_remove (ethernet.c:197)
==23771==by 0x4420C6: remove_device (device.c:295)
==23771==by 0x442149: remove_driver (device.c:310)
==23771==by 0x44229C: connman_device_driver_unregister (device.c:363)
==23771==by 0x41FCBD: ethernet_exit (ethernet.c:365)
==23771==
==23771== Invalid read of size 8
==23771==at 0x445A18: connman_network_unref_debug (network.c:1037)
==23771==by 0x41F5A5: remove_network (ethernet.c:131)
==23771==by 0x41F86E: eth_dev_remove (ethernet.c:197)
==23771==by 0x4420C6: remove_device (device.c:295)
==23771==by 0x442149: remove_driver (device.c:310)
==23771==by 0x44229C: connman_device_driver_unregister (device.c:363)
==23771==by 0x41FCBD: ethernet_exit (ethernet.c:365)
==23771==by 0x440C45: __connman_plugin_cleanup (plugin.c:200)
==23771==by 0x43F5FB: main (main.c:697)
==23771==  Address 0x7917d38 is 24 bytes inside a block of size 232 free'd
==23771==at 0x4C2BCD7: free (vg_replace_malloc.c:469)
==23771==by 0x445863: network_destruct (network.c:968)
==23771==by 0x445A98: connman_network_unref_debug (network.c:1045)
==23771==by 0x44230B: free_network (device.c:374)
==23771==by 0x4E6D8F9: ??? (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.3800.1)
==23771==by 0x44310F: connman_device_remove_network (device.c:901)
==23771==by 0x41F586: remove_network (ethernet.c:130)
==23771==by 0x41F86E: eth_dev_remove (ethernet.c:197)
==23771==by 0x4420C6: remove_device (device.c:295)
==23771==by 0x442149: remove_driver (device.c:310)
==23771==by 0x44229C: connman_device_driver_unregister (device.c:363)
==23771==by 0x41FCBD: ethernet_exit (ethernet.c:365)




On Thu, Apr 17, 2014 at 3:37 AM, Tomasz Bursztyka <
tomasz.burszt...@linux.intel.com> wrote:

> Hi Eduardo,
>
>
>  Network unreference is already being done by free_network,
>> called by g_hash_table_remove. This patche prevents from
>> an invalid read during nework removal.
>>
>
> I would be curious to see your valgrind output.
>
> The reference ethernet.c is removing is the one which is set when the
> network is created.
> device.c remove it's own reference (added in connman_device_add_network).
>
> If there is a reference bug, it does not seem to be where you found it.
>
> Tomasz
> ___
> connman mailing list
> connman@connman.net
> https://lists.connman.net/mailman/listinfo/connman
>
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


[PATCH 0/2] Configurable IPv4/6-status urls & support for http-status response code 204 for online checks.

2014-04-17 Thread Pasi Sjöholm
here are the reworked patches which provide configurable IPv4/6-status urls for 
ConnMan together with support for using http-status response code 204 instead 
of the default "X-ConnMan-Status: online"-check.

Pasi Sjöholm (2):
  Support for configurable ipv4/6 status urls.
  Support for online check based on HTTP-status code 204.

 include/setting.h |  3 +++
 src/6to4.c|  9 +
 src/main.c| 36 
 src/main.conf |  6 ++
 src/wispr.c   | 14 +-
 5 files changed, 59 insertions(+), 9 deletions(-)

-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 2/2] Support for online check based on HTTP-status code 204.

2014-04-17 Thread Pasi Sjöholm
Unfortunately there are numerous of ISP's or organizations which
want to modify/strip X-ConnMan-Status-header away from the
http-responses even the connection is otherwise just fine (eg. no
portals). However they do not modify http-status response codes and
that's when the "204" comes in handy. With this patch ConnMan-users
can use alternative way to check whether their devices are
actually "online"
---
 src/main.conf | 5 +++--
 src/wispr.c   | 5 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/main.conf b/src/main.conf
index 0247d0f..0c6720e 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -96,7 +96,8 @@
 # Default value is false.
 # PersistentTetheringMode = false
 
-# Set the online status url which returns at least "X-ConnMan-Status: 
-# online" HTTP-header.
+# Set the online status url which either returns at least 
+# "X-ConnMan-Status: online" HTTP-header 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 4ba2b28..5e8912f 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -257,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";
case 255:
return "Access gateway internal error";
}
@@ -722,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;
case 302:
if (!g_web_supports_tls() ||
!g_web_result_get_header(result, "Location",
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


[PATCH 1/2] Support for configurable ipv4/6 status urls.

2014-04-17 Thread Pasi Sjöholm
Ipv4StatusUrl and Ipv6StatusUrl can be defined to main.conf to
use other than ConnMan's default online check server(s).
---
 include/setting.h |  3 +++
 src/6to4.c|  9 +
 src/main.c| 36 
 src/main.conf |  5 +
 src/wispr.c   |  9 -
 5 files changed, 53 insertions(+), 9 deletions(-)

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..0c880e9 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,8 +315,11 @@ 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,
-   web_result, NULL,  NULL);
+   const char *url = connman_option_get_string(
+   CONF_STATUS_URL_IPV6);
+
+   web_request_id = g_web_request_get(web, url,
+   web_result, NULL,  NULL);
 
newlink_timeout(NULL);
}
diff --git a/src/main.c b/src/main.c
index 4f635de..8b93ece 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"
@@ -100,6 +104,8 @@ static struct {
 #define CONF_SINGLE_TECH"SingleConnectedTechnology"
 #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,7 @@ static void parse_config(GKeyFile *config)
char **interfaces;
char **str_list;
char **tethering;
+   char *status_url;
gsize len;
int timeout;
 
@@ -354,6 +363,27 @@ static void parse_config(GKeyFile *config)
connman_settings.persistent_tethering_mode = boolean;
 
g_clear_error(&error);
+
+status_url = __connman_config_get_string(config, "General",
+   CONF_STATUS_URL_IPV4, &error);
+if (!error)
+connman_settings.ipv4_status_url = status_url;
+else
+connman_settings.ipv4_status_url =
+   "http://ipv4.connman.net/online/status.html";;
+
+g_clear_error(&error);
+
+status_url = __connman_config_get_string(config, "General",
+   CONF_STATUS_URL_IPV6, &error);
+if (!error)
+connman_settings.ipv6_status_url = status_url;
+else
+connman_settings.ipv6_status_url =
+   "http://ipv6.connman.net/online/status.html";;
+
+g_clear_error(&error);
+
 }
 
 static int config_init(const char *file)
@@ -511,6 +541,12 @@ const char *connman_option_get_string(const char *key)
return option_wifi;
}
 
+   if (g_str_equal(key, CONF_STATUS_URL_IPV4) == TRUE)
+   return connman_settings.ipv4_status_url;
+
+   if (g_str_equal(key, CONF_STATUS_URL_IPV6) == TRUE)
+   return connman_settings.ipv6_status_url;
+
return NULL;
 }
 
diff --git a/src/main.conf b/src/main.conf
index 93c7a50..0247d0f 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -95,3 +95,8 @@
 # re-enabling a technology, and after restarts and reboots.
 # De

Re: [PATCH 2/2] Avoid network duplicate unref for eth_dev_remote

2014-04-17 Thread Tomasz Bursztyka

Hi Eduardo,


Network unreference is already being done by free_network,
called by g_hash_table_remove. This patche prevents from
an invalid read during nework removal.


I would be curious to see your valgrind output.

The reference ethernet.c is removing is the one which is set when the 
network is created.

device.c remove it's own reference (added in connman_device_add_network).

If there is a reference bug, it does not seem to be where you found it.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH] Configurable IPv4/6-status urls & support for http-status response code 204 for online checks.

2014-04-17 Thread Tomasz Bursztyka

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 con