Re: [PATCH v3] Add inotify monitoring .config file.

2011-01-03 Thread Marcel Holtmann
Hi Mohamed,

> Reflect new and modify *.config to connman config list. with
> patch any modified or added .config file will be read by connman
> and add these configuration for new provisioning.
> ---
>  src/config.c |  174 -
>  1 files changed, 170 insertions(+), 4 deletions(-)
> 
> diff --git a/src/config.c b/src/config.c
> index d203935..980182f 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -24,8 +24,10 @@
>  #endif
>  
>  #include 
> +#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "connman.h"
> @@ -56,6 +58,11 @@ struct connman_config {
>  
>  static GHashTable *config_table = NULL;
>  
> +static int inotify_wd = -1;
> +
> +static GIOChannel *inotify_channel = NULL;
> +static uint channel_watch = 0;

Just call it inotify_watch here. Makes it a bit more clear.

>  /* Definition of possible strings in the .config files */
>  #define CONFIG_KEY_NAME"Name"
>  #define CONFIG_KEY_DESC"Description"
> @@ -362,8 +369,6 @@ static int create_config(const char *ident)
>  
>   connman_info("Adding configuration %s", config->ident);
>  
> - load_config(config);
> -
>   return 0;
>  }
>  
> @@ -395,8 +400,14 @@ static int read_configs(void)
>   ident = g_string_free(str, FALSE);
>  
>   if (connman_dbus_validate_ident(ident) == TRUE)
> - create_config(ident);
> -
> + if (create_config(ident) == 0) {
> + struct connman_config *config;
> +
> + config = g_hash_table_lookup(
> + config_table, ident);
> + if (config != NULL)
> + load_config(config);
> + }

As a general rule you should put a { } around the if statement. I know
that the compiler gets this right, but for humans it is harder to read.

Also why not modify create_config in a way that it return the struct
connman_config. That would avoid the extra lookup.

>   g_free(ident);
>   }
>  
> @@ -406,6 +417,157 @@ static int read_configs(void)
>   return 0;
>  }
>  
> +static gboolean inotify_data(GIOChannel *channel, GIOCondition cond,
> + gpointer user_data)
> +{
> + char buffer[4096];

This buffer is a bit of overhead. sizeof(inotify_event) + MAX_PATHNAME
should be enough. So something around 256 bytes is plenty.

> + char *next_event;
> + gsize bytes_read;
> + GIOError err;
> +
> + if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> + channel_watch = 0;
> + return FALSE;
> + }
> +
> + err = g_io_channel_read(channel, (gchar *) buffer,

This gchar cast is not needed since your buffer is already char.

> + sizeof(buffer) - 1, &bytes_read);
> +
> + if (err != G_IO_ERROR_NONE) {
> + if (err == G_IO_ERROR_AGAIN)
> + return TRUE;
> +
> + connman_error("Reading from inotify channel failed");
> + channel_watch = 0;
> + return FALSE;
> + }
> +
> + next_event = buffer;
> +
> + while (bytes_read > 0) {
> + struct inotify_event *event;
> + gchar *file;
> + GString *str;
> + gchar *ident;
> + gsize len;
> +
> + event = (struct inotify_event *) next_event;
> + if (event->len)
> + file = next_event + sizeof(struct inotify_event);
> + else
> + file = NULL;
> +
> + len = sizeof(struct inotify_event) + event->len;
> +
> + /* check if inotify_event block fit */
> + if (len > bytes_read)
> + break;
> +
> + next_event += len;
> + bytes_read -= len;
> +
> + if (file == NULL)
> + continue;
> +
> + if (g_str_has_suffix(file, ".config") == FALSE)
> + continue;
> +
> + ident = g_strrstr(file, ".config");
> + if (ident == NULL)
> + continue;
> +
> + str = g_string_new_len(file, ident - file);
> + if (str == NULL)
> + continue;
> +
> + ident = g_string_free(str, FALSE);

This is a bit pointless here. You are using a GString with no real need
for a GString. You do own the event buffer here and could just put a
'\0' at the position of the '.'. And then use that string directly.

> + if (connman_dbus_validate_ident(ident) == FALSE) {
> + g_free(ident);
> + continue;
> + }
> +
> + if (event->mask & IN_CREATE)
> + crea

[PATCH v3] Add inotify monitoring .config file.

2011-01-03 Thread Mohamed Abbas
Reflect new and modify *.config to connman config list. with
patch any modified or added .config file will be read by connman
and add these configuration for new provisioning.
---
 src/config.c |  174 -
 1 files changed, 170 insertions(+), 4 deletions(-)

diff --git a/src/config.c b/src/config.c
index d203935..980182f 100644
--- a/src/config.c
+++ b/src/config.c
@@ -24,8 +24,10 @@
 #endif
 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 
 #include "connman.h"
@@ -56,6 +58,11 @@ struct connman_config {
 
 static GHashTable *config_table = NULL;
 
+static int inotify_wd = -1;
+
+static GIOChannel *inotify_channel = NULL;
+static uint channel_watch = 0;
+
 /* Definition of possible strings in the .config files */
 #define CONFIG_KEY_NAME"Name"
 #define CONFIG_KEY_DESC"Description"
@@ -362,8 +369,6 @@ static int create_config(const char *ident)
 
connman_info("Adding configuration %s", config->ident);
 
-   load_config(config);
-
return 0;
 }
 
@@ -395,8 +400,14 @@ static int read_configs(void)
ident = g_string_free(str, FALSE);
 
if (connman_dbus_validate_ident(ident) == TRUE)
-   create_config(ident);
-
+   if (create_config(ident) == 0) {
+   struct connman_config *config;
+
+   config = g_hash_table_lookup(
+   config_table, ident);
+   if (config != NULL)
+   load_config(config);
+   }
g_free(ident);
}
 
@@ -406,6 +417,157 @@ static int read_configs(void)
return 0;
 }
 
+static gboolean inotify_data(GIOChannel *channel, GIOCondition cond,
+   gpointer user_data)
+{
+   char buffer[4096];
+   char *next_event;
+   gsize bytes_read;
+   GIOError err;
+
+   if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+   channel_watch = 0;
+   return FALSE;
+   }
+
+   err = g_io_channel_read(channel, (gchar *) buffer,
+   sizeof(buffer) - 1, &bytes_read);
+
+   if (err != G_IO_ERROR_NONE) {
+   if (err == G_IO_ERROR_AGAIN)
+   return TRUE;
+
+   connman_error("Reading from inotify channel failed");
+   channel_watch = 0;
+   return FALSE;
+   }
+
+   next_event = buffer;
+
+   while (bytes_read > 0) {
+   struct inotify_event *event;
+   gchar *file;
+   GString *str;
+   gchar *ident;
+   gsize len;
+
+   event = (struct inotify_event *) next_event;
+   if (event->len)
+   file = next_event + sizeof(struct inotify_event);
+   else
+   file = NULL;
+
+   len = sizeof(struct inotify_event) + event->len;
+
+   /* check if inotify_event block fit */
+   if (len > bytes_read)
+   break;
+
+   next_event += len;
+   bytes_read -= len;
+
+   if (file == NULL)
+   continue;
+
+   if (g_str_has_suffix(file, ".config") == FALSE)
+   continue;
+
+   ident = g_strrstr(file, ".config");
+   if (ident == NULL)
+   continue;
+
+   str = g_string_new_len(file, ident - file);
+   if (str == NULL)
+   continue;
+
+   ident = g_string_free(str, FALSE);
+
+   if (connman_dbus_validate_ident(ident) == FALSE) {
+   g_free(ident);
+   continue;
+   }
+
+   if (event->mask & IN_CREATE)
+   create_config(ident);
+
+   if (event->mask & IN_MODIFY) {
+   struct connman_config *config;
+
+   config = g_hash_table_lookup(config_table, ident);
+   if (config != NULL) {
+   g_hash_table_remove_all(config->service_table);
+   load_config(config);
+   }
+   }
+
+   if (event->mask & IN_DELETE)
+   g_hash_table_remove(config_table, ident);
+
+   g_free(ident);
+   }
+
+   return TRUE;
+}
+
+static int create_watch(void)
+{
+   int fd;
+
+   fd = inotify_init();
+   if (fd < 0)
+   return -EIO;
+
+   inotify_wd = inotify_add_watch(fd, STORAGEDIR,
+   IN_MODIFY | IN_CREATE | IN_DELETE);
+   

Re: [PATCH] Add inotify monitoring .config file.

2011-01-03 Thread Marcel Holtmann
Hi Mohamed,

> Reflect new and modify *.config to connman config list. with
> patch any modified or added .config file will be read by connman
> and add these configuration for new provisioning.
> 
> P.S
> I will submit another patch to refelect new provisioning on current
> avialable network.
> ---

just a quick hint. Please place notes after the --- mark. Otherwise they
end up in the commit message.

Everything between --- and the diff keyword will be removed. These are
just extra information for the reviewer (like changelog etc.).

>  src/config.c |  157 -
>  1 files changed, 154 insertions(+), 3 deletions(-)
> 
> diff --git a/src/config.c b/src/config.c
> index d203935..59f5fe3 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -24,8 +24,13 @@
>  #endif
>  
>  #include 
> +#include 
> +#include 
>  #include 
> +#include 

What do you need an ioctl for?

>  #include 
> +#include 
> +#include 
>  #include 
>  
>  #include "connman.h"
> @@ -56,6 +61,10 @@ struct connman_config {
>  
>  static GHashTable *config_table = NULL;
>  
> +static int inotify_watch = 0;
> +static GIOChannel *inotify_channel = NULL;
> +static uint channel_watch = 0;

So 0 is actually a valid file descriptor. So that would need to be -1.

I am also not a big fan of mixing inotify watch and GLib watch
definitions here. Maybe using _wd is a bit better for the inotify watch
descriptor.

>  /* Definition of possible strings in the .config files */
>  #define CONFIG_KEY_NAME"Name"
>  #define CONFIG_KEY_DESC"Description"
> @@ -340,7 +349,7 @@ static int load_config(struct connman_config *config)
>   return 0;
>  }
>  
> -static int create_config(const char *ident)
> +static int create_config(const char *ident, connman_bool_t load)
>  {
>   struct connman_config *config;
>  
> @@ -362,7 +371,8 @@ static int create_config(const char *ident)
>  
>   connman_info("Adding configuration %s", config->ident);
>  
> - load_config(config);
> + if (load == TRUE)
> + load_config(config);

I think doing this in read_configs() is a bit cleaner.

if (connman_dbus_validate_ident(ident) == TRUE) {
if (create_config(ident) == 0)
load_config(config);
}
 
> +static gboolean inotify_data(GIOChannel *channel, GIOCondition cond,
> + gpointer user_data)
> +{
> + char buffer[4096];
> + gsize i, ret;
> + GIOError err;
> +
> + if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> + channel_watch = 0;
> + return FALSE;
> + }
> +
> + err = g_io_channel_read(channel, (gchar *) buffer,
> + sizeof(buffer) - 1, &ret);

Call it bytes_read or count instead of ret.

> + if (err != G_IO_ERROR_NONE) {
> + if (err == G_IO_ERROR_AGAIN)
> + return TRUE;
> +
> + connman_error("Reading from inotify channel failed");
> + channel_watch = 0;
> + return FALSE;
> + }
> +
> + if (ret <= 0)
> + return TRUE;

This check is rather pointless here.

> + i = 0;
> +
> + while (i < ret) {

Doing while (count > 0) seems a bit more easier. And then you just
decrement count and increment current ptr.

> + struct inotify_event *event;
> + const gchar *file = NULL;
> + GString *str;
> + gchar *ident;
> +
> + event = (struct inotify_event *) &buffer[i];
> + if (event->len)
> + file = &buffer[i] + sizeof(struct inotify_event);
> +
> + i += sizeof(struct inotify_event) + event->len;

You do need to check that the inotify_event block fits. Otherwise you
potentially read from invalid memory.

> + if (g_str_has_suffix(file, ".config") == FALSE)
> + continue;
> +
> + ident = g_strrstr(file, ".config");
> + if (ident == NULL)
> + continue;
> +
> + str = g_string_new_len(file, ident - file);
> + if (str == NULL)
> + continue;
> +
> + ident = g_string_free(str, FALSE);
> +
> + if (connman_dbus_validate_ident(ident) == FALSE)
> + continue;
> +
> + if (event->mask & IN_CREATE)
> + create_config(ident, FALSE);
> +
> + if (event->mask & IN_MODIFY) {
> + struct connman_config *config;
> +
> + config = g_hash_table_lookup(config_table, ident);
> + if (config != NULL) {
> + g_hash_table_remove_all(config->service_table);
> + load_config(config);
> + }
> + }
> +
> + if (event->mask & IN_DELETE)
> + g_hash_table_remove(config_table,

[PATCH v2] Add inotify monitoring .config file.

2011-01-03 Thread Mohamed Abbas
Reflect new and modify *.config to connman config list. with
patch any modified or added .config file will be read by connman
and add these configuration for new provisioning.

P.S.
I will submit another patch to refelect new provisioning on current
avialable network.
---
 src/config.c |  165 -
 1 files changed, 162 insertions(+), 3 deletions(-)

diff --git a/src/config.c b/src/config.c
index d203935..52a602b 100644
--- a/src/config.c
+++ b/src/config.c
@@ -24,8 +24,13 @@
 #endif
 
 #include 
+#include 
+#include 
 #include 
+#include 
 #include 
+#include 
+#include 
 #include 
 
 #include "connman.h"
@@ -56,6 +61,10 @@ struct connman_config {
 
 static GHashTable *config_table = NULL;
 
+static int inotify_watch = 0;
+static GIOChannel *inotify_channel = NULL;
+static uint channel_watch = 0;
+
 /* Definition of possible strings in the .config files */
 #define CONFIG_KEY_NAME"Name"
 #define CONFIG_KEY_DESC"Description"
@@ -340,7 +349,7 @@ static int load_config(struct connman_config *config)
return 0;
 }
 
-static int create_config(const char *ident)
+static int create_config(const char *ident, connman_bool_t load)
 {
struct connman_config *config;
 
@@ -362,7 +371,8 @@ static int create_config(const char *ident)
 
connman_info("Adding configuration %s", config->ident);
 
-   load_config(config);
+   if (load == TRUE)
+   load_config(config);
 
return 0;
 }
@@ -395,7 +405,7 @@ static int read_configs(void)
ident = g_string_free(str, FALSE);
 
if (connman_dbus_validate_ident(ident) == TRUE)
-   create_config(ident);
+   create_config(ident, TRUE);
 
g_free(ident);
}
@@ -406,6 +416,151 @@ static int read_configs(void)
return 0;
 }
 
+static gboolean inotify_data(GIOChannel *channel, GIOCondition cond,
+   gpointer user_data)
+{
+   char buffer[4096];
+   gsize i, ret;
+   GIOError err;
+
+   if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+   channel_watch = 0;
+   return FALSE;
+   }
+
+   err = g_io_channel_read(channel, (gchar *) buffer,
+   sizeof(buffer) - 1, &ret);
+
+   if (err != G_IO_ERROR_NONE) {
+   if (err == G_IO_ERROR_AGAIN)
+   return TRUE;
+
+   connman_error("Reading from inotify channel failed");
+   channel_watch = 0;
+   return FALSE;
+   }
+
+   if (ret <= 0)
+   return TRUE;
+
+   i = 0;
+
+   while (i < ret) {
+   struct inotify_event *event;
+   gchar *file;
+   GString *str;
+   gchar *ident;
+
+   event = (struct inotify_event *) &buffer[i];
+   if (event->len)
+   file = &buffer[i] + sizeof(struct inotify_event);
+   else
+   file = NULL;
+
+   i += sizeof(struct inotify_event) + event->len;
+
+   if (file == NULL)
+   continue;
+
+   if (g_str_has_suffix(file, ".config") == FALSE)
+   continue;
+
+   ident = g_strrstr(file, ".config");
+   if (ident == NULL)
+   continue;
+
+   str = g_string_new_len(file, ident - file);
+   if (str == NULL)
+   continue;
+
+   ident = g_string_free(str, FALSE);
+
+   if (connman_dbus_validate_ident(ident) == FALSE) {
+   g_free(ident);
+   continue;
+   }
+
+   if (event->mask & IN_CREATE)
+   create_config(ident, FALSE);
+
+   if (event->mask & IN_MODIFY) {
+   struct connman_config *config;
+
+   config = g_hash_table_lookup(config_table, ident);
+   if (config != NULL) {
+   g_hash_table_remove_all(config->service_table);
+   load_config(config);
+   }
+   }
+
+   if (event->mask & IN_DELETE)
+   g_hash_table_remove(config_table, ident);
+
+   g_free(ident);
+   }
+
+   return TRUE;
+}
+
+static int create_watch(void)
+{
+   int fd;
+
+   fd = inotify_init();
+
+   if (fd < 0)
+   return -EIO;
+
+   inotify_watch = inotify_add_watch(fd, STORAGEDIR,
+   IN_MODIFY | IN_CREATE | IN_DELETE);
+   if (inotify_watch < 0) {
+   connman_error("Creation of STORAGEDIR  watch failed");
+   close(fd);
+   return -EIO;
+   }
+
+   

[PATCH] Add inotify monitoring .config file th correct file.

2011-01-03 Thread Mohamed Abbas
Please ignore the previous patch, I sent the wrong one please use
this one.

Reflect new and modify *.config to connman config list. with
patch any modified or added .config file will be read by connman
and add these configuration for new provisioning.

P.S.
I will submit another patch to refelect new provisioning on current
avialable network.
---
 src/config.c |  162 -
 1 files changed, 159 insertions(+), 3 deletions(-)

diff --git a/src/config.c b/src/config.c
index d203935..c4be8b1 100644
--- a/src/config.c
+++ b/src/config.c
@@ -24,8 +24,13 @@
 #endif
 
 #include 
+#include 
+#include 
 #include 
+#include 
 #include 
+#include 
+#include 
 #include 
 
 #include "connman.h"
@@ -56,6 +61,10 @@ struct connman_config {
 
 static GHashTable *config_table = NULL;
 
+static int inotify_watch = 0;
+static GIOChannel *inotify_channel = NULL;
+static uint channel_watch = 0;
+
 /* Definition of possible strings in the .config files */
 #define CONFIG_KEY_NAME"Name"
 #define CONFIG_KEY_DESC"Description"
@@ -340,7 +349,7 @@ static int load_config(struct connman_config *config)
return 0;
 }
 
-static int create_config(const char *ident)
+static int create_config(const char *ident, connman_bool_t load)
 {
struct connman_config *config;
 
@@ -362,7 +371,8 @@ static int create_config(const char *ident)
 
connman_info("Adding configuration %s", config->ident);
 
-   load_config(config);
+   if (load == TRUE)
+   load_config(config);
 
return 0;
 }
@@ -395,7 +405,7 @@ static int read_configs(void)
ident = g_string_free(str, FALSE);
 
if (connman_dbus_validate_ident(ident) == TRUE)
-   create_config(ident);
+   create_config(ident, TRUE);
 
g_free(ident);
}
@@ -406,6 +416,148 @@ static int read_configs(void)
return 0;
 }
 
+static gboolean inotify_data(GIOChannel *channel, GIOCondition cond,
+   gpointer user_data)
+{
+   char buffer[4096];
+   gsize i, ret;
+   GIOError err;
+
+   if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+   channel_watch = 0;
+   return FALSE;
+   }
+
+   err = g_io_channel_read(channel, (gchar *) buffer,
+   sizeof(buffer) - 1, &ret);
+
+   if (err != G_IO_ERROR_NONE) {
+   if (err == G_IO_ERROR_AGAIN)
+   return TRUE;
+
+   connman_error("Reading from inotify channel failed");
+   channel_watch = 0;
+   return FALSE;
+   }
+
+   if (ret <= 0)
+   return TRUE;
+
+   i = 0;
+
+   while (i < ret) {
+   struct inotify_event *event;
+   gchar *file;
+   GString *str;
+   gchar *ident;
+
+   event = (struct inotify_event *) &buffer[i];
+   if (event->len)
+   file = &buffer[i] + sizeof(struct inotify_event);
+   else
+   file = NULL;
+
+   i += sizeof(struct inotify_event) + event->len;
+
+   if (file == NULL)
+   continue;
+
+   if (g_str_has_suffix(file, ".config") == FALSE)
+   continue;
+
+   ident = g_strrstr(file, ".config");
+   if (ident == NULL)
+   continue;
+
+   str = g_string_new_len(file, ident - file);
+   if (str == NULL)
+   continue;
+
+   ident = g_string_free(str, FALSE);
+
+   if (connman_dbus_validate_ident(ident) == FALSE)
+   continue;
+
+   if (event->mask & IN_CREATE)
+   create_config(ident, FALSE);
+
+   if (event->mask & IN_MODIFY) {
+   struct connman_config *config;
+
+   config = g_hash_table_lookup(config_table, ident);
+   if (config != NULL) {
+   g_hash_table_remove_all(config->service_table);
+   load_config(config);
+   }
+   }
+
+   if (event->mask & IN_DELETE)
+   g_hash_table_remove(config_table, ident);
+
+   }
+
+   return TRUE;
+}
+
+static int create_watch(void)
+{
+   int fd;
+
+   fd = inotify_init();
+
+   if (fd < 0)
+   return -EIO;
+
+   inotify_watch = inotify_add_watch(fd, STORAGEDIR,
+   IN_MODIFY | IN_CREATE | IN_DELETE);
+   if (inotify_watch < 0) {
+   connman_error("Creation of STORAGEDIR  watch failed");
+   close(fd);
+   return -EIO;
+   }
+
+   inotify_chann

[PATCH] Add inotify monitoring .config file.

2011-01-03 Thread Mohamed Abbas
Reflect new and modify *.config to connman config list. with
patch any modified or added .config file will be read by connman
and add these configuration for new provisioning.

P.S
I will submit another patch to refelect new provisioning on current
avialable network.
---
 src/config.c |  157 -
 1 files changed, 154 insertions(+), 3 deletions(-)

diff --git a/src/config.c b/src/config.c
index d203935..59f5fe3 100644
--- a/src/config.c
+++ b/src/config.c
@@ -24,8 +24,13 @@
 #endif
 
 #include 
+#include 
+#include 
 #include 
+#include 
 #include 
+#include 
+#include 
 #include 
 
 #include "connman.h"
@@ -56,6 +61,10 @@ struct connman_config {
 
 static GHashTable *config_table = NULL;
 
+static int inotify_watch = 0;
+static GIOChannel *inotify_channel = NULL;
+static uint channel_watch = 0;
+
 /* Definition of possible strings in the .config files */
 #define CONFIG_KEY_NAME"Name"
 #define CONFIG_KEY_DESC"Description"
@@ -340,7 +349,7 @@ static int load_config(struct connman_config *config)
return 0;
 }
 
-static int create_config(const char *ident)
+static int create_config(const char *ident, connman_bool_t load)
 {
struct connman_config *config;
 
@@ -362,7 +371,8 @@ static int create_config(const char *ident)
 
connman_info("Adding configuration %s", config->ident);
 
-   load_config(config);
+   if (load == TRUE)
+   load_config(config);
 
return 0;
 }
@@ -395,7 +405,7 @@ static int read_configs(void)
ident = g_string_free(str, FALSE);
 
if (connman_dbus_validate_ident(ident) == TRUE)
-   create_config(ident);
+   create_config(ident, TRUE);
 
g_free(ident);
}
@@ -406,6 +416,143 @@ static int read_configs(void)
return 0;
 }
 
+static gboolean inotify_data(GIOChannel *channel, GIOCondition cond,
+   gpointer user_data)
+{
+   char buffer[4096];
+   gsize i, ret;
+   GIOError err;
+
+   if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+   channel_watch = 0;
+   return FALSE;
+   }
+
+   err = g_io_channel_read(channel, (gchar *) buffer,
+   sizeof(buffer) - 1, &ret);
+
+   if (err != G_IO_ERROR_NONE) {
+   if (err == G_IO_ERROR_AGAIN)
+   return TRUE;
+
+   connman_error("Reading from inotify channel failed");
+   channel_watch = 0;
+   return FALSE;
+   }
+
+   if (ret <= 0)
+   return TRUE;
+
+   i = 0;
+
+   while (i < ret) {
+   struct inotify_event *event;
+   const gchar *file = NULL;
+   GString *str;
+   gchar *ident;
+
+   event = (struct inotify_event *) &buffer[i];
+   if (event->len)
+   file = &buffer[i] + sizeof(struct inotify_event);
+
+   i += sizeof(struct inotify_event) + event->len;
+
+   if (g_str_has_suffix(file, ".config") == FALSE)
+   continue;
+
+   ident = g_strrstr(file, ".config");
+   if (ident == NULL)
+   continue;
+
+   str = g_string_new_len(file, ident - file);
+   if (str == NULL)
+   continue;
+
+   ident = g_string_free(str, FALSE);
+
+   if (connman_dbus_validate_ident(ident) == FALSE)
+   continue;
+
+   if (event->mask & IN_CREATE)
+   create_config(ident, FALSE);
+
+   if (event->mask & IN_MODIFY) {
+   struct connman_config *config;
+
+   config = g_hash_table_lookup(config_table, ident);
+   if (config != NULL) {
+   g_hash_table_remove_all(config->service_table);
+   load_config(config);
+   }
+   }
+
+   if (event->mask & IN_DELETE)
+   g_hash_table_remove(config_table, ident);
+
+   }
+
+   return TRUE;
+}
+
+static int create_watch(void)
+{
+   int fd;
+
+   fd = inotify_init();
+
+   if (fd < 0)
+   return -EIO;
+
+   inotify_watch = inotify_add_watch(fd, STORAGEDIR,
+   IN_MODIFY | IN_CREATE | IN_DELETE);
+   if (inotify_watch < 0) {
+   connman_error("Creation of STORAGEDIR  watch failed");
+   close(fd);
+   return -EIO;
+   }
+
+   inotify_channel = g_io_channel_unix_new(fd);
+   if (inotify_channel == NULL) {
+   connman_error("Creation of inotify channel failed");
+   inotify_rm_watch(fd, inotify_watch);

Re: [PATCH memleak resend v3 0/2] Fixing call to __connman_ipconfig_disable()

2011-01-03 Thread Marcel Holtmann
Hi Jukka,

> I just noticed that the v2 resend was incorrect (the first patch was over my 
> previous patch and not the original).
> So this one is correct and as mentioned in v2 patch description, the second 
> patch is optional.

I saw the v3 too late. So ignore my other email. Both patches have been
applied now. Thanks.

Regards

Marcel


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman


Re: [PATCH memleak resend v2 1/2] memoryleak: check return value correctly before clearing pointer

2011-01-03 Thread Marcel Holtmann
Hi Jukka,

>  src/service.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

this patch does not apply cleanly.

Applying: memoryleak: check return value correctly before clearing pointer
error: patch failed: src/service.c:3406
error: src/service.c: patch does not apply
Patch failed at 0001 memoryleak: check return value correctly before clearing 
pointer

Regards

Marcel


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman


ConnMan Development History

2011-01-03 Thread Daniel Wagner

Hi,

I found a cool tool for visualization of commit histories. Of course, I 
tried it on ConnMan :)


http://www.youtube.com/watch?v=dScztWtB7Z4

cheers,
daniel
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman


[PATCH memleak resend v3 2/2] Make calls to __connman_ipconfig_disable() consistent.

2011-01-03 Thread Jukka Rissanen
---
 src/service.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/service.c b/src/service.c
index 18b0340..09cc4eb 100644
--- a/src/service.c
+++ b/src/service.c
@@ -2233,11 +2233,11 @@ static gboolean connect_timeout(gpointer user_data)
__connman_network_disconnect(service->network);
 
if (service->ipconfig_ipv4)
-   if (!__connman_ipconfig_disable(service->ipconfig_ipv4))
+   if (__connman_ipconfig_disable(service->ipconfig_ipv4) == 0)
service->ipconfig_ipv4 = NULL;
 
if (service->ipconfig_ipv6)
-   if (!__connman_ipconfig_disable(service->ipconfig_ipv6))
+   if (__connman_ipconfig_disable(service->ipconfig_ipv6) == 0)
service->ipconfig_ipv6 = NULL;
 
__connman_stats_service_unregister(service);
@@ -3352,13 +3352,13 @@ int __connman_service_connect(struct connman_service 
*service)
if (err < 0) {
if (err != -EINPROGRESS) {
if (service->ipconfig_ipv4)
-   if (!__connman_ipconfig_disable(
-   service->ipconfig_ipv4))
+   if (__connman_ipconfig_disable(
+   service->ipconfig_ipv4) == 0)
service->ipconfig_ipv4 = NULL;
 
if (service->ipconfig_ipv6)
-   if (!__connman_ipconfig_disable(
-   service->ipconfig_ipv6))
+   if (__connman_ipconfig_disable(
+   service->ipconfig_ipv6) == 0)
service->ipconfig_ipv6 = NULL;
 
__connman_stats_service_unregister(service);
-- 
1.7.0.4

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman


[PATCH memleak resend v3 1/2] memoryleak: check return value correctly before clearing pointer

2011-01-03 Thread Jukka Rissanen
---
 src/service.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/service.c b/src/service.c
index b90cc0c..18b0340 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3406,10 +3406,10 @@ int __connman_service_disconnect(struct connman_service 
*service)
__connman_ipconfig_clear_address(service->ipconfig_ipv4);
__connman_ipconfig_clear_address(service->ipconfig_ipv6);
 
-   if (__connman_ipconfig_disable(service->ipconfig_ipv4) < 0)
+   if (__connman_ipconfig_disable(service->ipconfig_ipv4) == 0)
service->ipconfig_ipv4 = NULL;
 
-   if (__connman_ipconfig_disable(service->ipconfig_ipv6) < 0)
+   if (__connman_ipconfig_disable(service->ipconfig_ipv6) == 0)
service->ipconfig_ipv6 = NULL;
 
__connman_stats_service_unregister(service);
-- 
1.7.0.4

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman


[PATCH memleak resend v3 0/2] Fixing call to __connman_ipconfig_disable()

2011-01-03 Thread Jukka Rissanen
Hi Marcel,

I just noticed that the v2 resend was incorrect (the first patch was over my 
previous patch and not the original).
So this one is correct and as mentioned in v2 patch description, the second 
patch is optional.

Regards,

Jukka


Jukka Rissanen (2):
  memoryleak: check return value correctly before clearing pointer
  Make calls to __connman_ipconfig_disable() consistent.

 src/service.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman


[PATCH memleak resend v2 2/2] Make calls to __connman_ipconfig_disable() consistent.

2011-01-03 Thread Jukka Rissanen
---
 src/service.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/service.c b/src/service.c
index e58c08f..6b4ede8 100644
--- a/src/service.c
+++ b/src/service.c
@@ -2233,11 +2233,11 @@ static gboolean connect_timeout(gpointer user_data)
__connman_network_disconnect(service->network);
 
if (service->ipconfig_ipv4)
-   if (!__connman_ipconfig_disable(service->ipconfig_ipv4))
+   if (__connman_ipconfig_disable(service->ipconfig_ipv4) == 0)
service->ipconfig_ipv4 = NULL;
 
if (service->ipconfig_ipv6)
-   if (!__connman_ipconfig_disable(service->ipconfig_ipv6))
+   if (__connman_ipconfig_disable(service->ipconfig_ipv6) == 0)
service->ipconfig_ipv6 = NULL;
 
__connman_stats_service_unregister(service);
@@ -3352,13 +3352,13 @@ int __connman_service_connect(struct connman_service 
*service)
if (err < 0) {
if (err != -EINPROGRESS) {
if (service->ipconfig_ipv4)
-   if (!__connman_ipconfig_disable(
-   service->ipconfig_ipv4))
+   if (__connman_ipconfig_disable(
+   service->ipconfig_ipv4) == 0)
service->ipconfig_ipv4 = NULL;
 
if (service->ipconfig_ipv6)
-   if (!__connman_ipconfig_disable(
-   service->ipconfig_ipv6))
+   if (__connman_ipconfig_disable(
+   service->ipconfig_ipv6) == 0)
service->ipconfig_ipv6 = NULL;
 
__connman_stats_service_unregister(service);
-- 
1.7.0.4

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman


[PATCH memleak resend v2 1/2] memoryleak: check return value correctly before clearing pointer

2011-01-03 Thread Jukka Rissanen
---
 src/service.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/service.c b/src/service.c
index 61e065b..e58c08f 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3406,10 +3406,10 @@ int __connman_service_disconnect(struct connman_service 
*service)
__connman_ipconfig_clear_address(service->ipconfig_ipv4);
__connman_ipconfig_clear_address(service->ipconfig_ipv6);
 
-   if (!__connman_ipconfig_disable(service->ipconfig_ipv4))
+   if (__connman_ipconfig_disable(service->ipconfig_ipv4) == 0)
service->ipconfig_ipv4 = NULL;
 
-   if (!__connman_ipconfig_disable(service->ipconfig_ipv6))
+   if (__connman_ipconfig_disable(service->ipconfig_ipv6) == 0)
service->ipconfig_ipv6 = NULL;
 
__connman_stats_service_unregister(service);
-- 
1.7.0.4

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman


[PATCH memleak resend v2 0/2] Fixing call to __connman_ipconfig_disable()

2011-01-03 Thread Jukka Rissanen
Hi Marcel,

here is a new version of the earlier memory leak patch.
The second patch is optional and it changes the existing
calls to __connman_ipconfig_disable() in service.c to use
the " == 0" style so that the those calls look the same.

Regards,

Jukka


Jukka Rissanen (2):
  memoryleak: check return value correctly before clearing pointer
  Make calls to __connman_ipconfig_disable() consistent.

 src/service.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman