From 1fd5d723fb52ecdef89df0e5697cabbe0448a534 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 14:09:11 +0300
Subject: [PATCH 01/23] [config] Fix return type of find_mounts() and
 find_alt_mounts()

The prototypes for these functions claim to return const string, while
in reality it is dynamic memory that should released via g_free().

Change the prototypes to match the reality.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-config.c | 6 +++---
 src/usb_moded-config.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/usb_moded-config.c b/src/usb_moded-config.c
index 635745c..50a3c8e 100644
--- a/src/usb_moded-config.c
+++ b/src/usb_moded-config.c
@@ -66,10 +66,10 @@ static int validate_ip(const char *ipadd)
   return 0;
 }
 
-const char *find_mounts(void)
+char *find_mounts(void)
 {
   
-  const char *ret = NULL;
+  char *ret = NULL;
 
   ret = get_conf_string(FS_MOUNT_ENTRY, FS_MOUNT_KEY);
   if(ret == NULL)
@@ -86,7 +86,7 @@ int find_sync(void)
   return(get_conf_int(FS_SYNC_ENTRY, FS_SYNC_KEY));
 }
 
-const char * find_alt_mount(void)
+char * find_alt_mount(void)
 {
   return(get_conf_string(ALT_MOUNT_ENTRY, ALT_MOUNT_KEY));
 }
diff --git a/src/usb_moded-config.h b/src/usb_moded-config.h
index 2afb1ff..b23e3c7 100644
--- a/src/usb_moded-config.h
+++ b/src/usb_moded-config.h
@@ -59,9 +59,9 @@
 #define ANDROID_PRODUCT_ID_KEY		"idProduct"
 #define MODE_HIDE_KEY			"hide"
 
-const char * find_mounts(void);
+char * find_mounts(void);
 int find_sync(void);
-const char * find_alt_mount(void);
+char * find_alt_mount(void);
 
 char * find_udev_path(void);
 char * find_udev_subsystem(void);

From a9cab1f5a0723979d5ab73f3e0219b61c779158f Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 14:23:05 +0300
Subject: [PATCH 02/23] [cleanup] Do not use const for pointers used for
 storing dynamic data

Probably in attempt to hide compilation warnings due to storing dynamically
allocated data in const pointers, there are multiple places where releasing
of dynamic memory uses casts. Some of these casts are incorrect and there
are still compilation issues.

Remove const attribute from pointers that are used to store dynamically
allocated data and remove the unnecessary casts at free() / g_free().

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-dbus.c        | 2 +-
 src/usb_moded-modesetting.c | 8 ++++----
 src/usb_moded-udev.c        | 8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/usb_moded-dbus.c b/src/usb_moded-dbus.c
index f7c485e..4bb6df1 100644
--- a/src/usb_moded-dbus.c
+++ b/src/usb_moded-dbus.c
@@ -294,7 +294,7 @@ static DBusHandlerResult msg_handler(DBusConnection *const connection, DBusMessa
 			{
 				if((reply = dbus_message_new_method_return(msg)))
 				dbus_message_append_args (reply, DBUS_TYPE_STRING, &config, DBUS_TYPE_STRING, &setting, DBUS_TYPE_INVALID);
-				free((void *)setting);
+				free(setting);
 			}
 			else
 				reply = dbus_message_new_error(msg, DBUS_ERROR_INVALID_ARGS, config);
diff --git a/src/usb_moded-modesetting.c b/src/usb_moded-modesetting.c
index 3bdca1b..3748bc6 100644
--- a/src/usb_moded-modesetting.c
+++ b/src/usb_moded-modesetting.c
@@ -98,7 +98,7 @@ static int set_mass_storage_mode(struct mode_list_elem *data)
 {
         gchar *command;
         char command2[256], *real_path = NULL, *mountpath;
-        const char *mount;
+        char *mount;
         gchar **mounts;
         int ret = 0, i = 0, mountpoints = 0, fua = 0, try = 0;
 
@@ -199,7 +199,7 @@ umount:                 command = g_strconcat("mount | grep ", mountpath, NULL);
 			}
                 }
                 g_strfreev(mounts);
-		g_free((gpointer *)mount);
+		g_free(mount);
 		if(real_path)
 			free(real_path);
 	}
@@ -216,7 +216,7 @@ static int unset_mass_storage_mode(struct mode_list_elem *data)
 {
         gchar *command;
         char command2[256], *real_path = NULL, *mountpath;
-        const char *mount;
+        char *mount;
         gchar **mounts;
         int ret = 1, i = 0;
 
@@ -277,7 +277,7 @@ static int unset_mass_storage_mode(struct mode_list_elem *data)
 			}
                  }
                  g_strfreev(mounts);
-		 g_free((gpointer *)mount);
+		 g_free(mount);
 		 if(real_path)
 			free(real_path);
         }
diff --git a/src/usb_moded-udev.c b/src/usb_moded-udev.c
index 62e2e9b..62c49c5 100644
--- a/src/usb_moded-udev.c
+++ b/src/usb_moded-udev.c
@@ -44,7 +44,7 @@ static struct udev *udev;
 static struct udev_monitor *mon;
 static GIOChannel *iochannel;
 static guint watch_id; 
-static const char *dev_name;
+static char *dev_name = 0;
 static int cleanup = 0;
 /* track cable and charger connects disconnects */
 static int cable = 0, charger = 0;
@@ -141,7 +141,7 @@ gboolean hwal_init(void)
   if(udev_path)
   {
 	dev = udev_device_new_from_syspath(udev, udev_path);
-	g_free((gpointer *)udev_path);
+	g_free(udev_path);
   }
   else
   	dev = udev_device_new_from_syspath(udev, "/sys/class/power_supply/usb");
@@ -195,7 +195,7 @@ gboolean hwal_init(void)
   if(udev_subsystem)
   {
 	  ret = udev_monitor_filter_add_match_subsystem_devtype(mon, udev_subsystem, NULL);
-	  g_free((gpointer *)udev_subsystem);
+	  g_free(udev_subsystem);
   }
   else
 	  ret = udev_monitor_filter_add_match_subsystem_devtype(mon, "power_supply", NULL);
@@ -289,7 +289,7 @@ void hwal_cleanup(void)
     iochannel = NULL;
   }
   cancel_cable_connection_timeout();
-  free((void *) dev_name);
+  free(dev_name);
   udev_monitor_unref(mon);
   udev_unref(udev);
 }

From 2bb04f4bc6490adade9826095783dab3dbc2ec94 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Thu, 7 Jul 2016 10:04:38 +0300
Subject: [PATCH 03/23] [modesetting] Fix memory leak

When fallback to alternate mount point is made, the pointer to the
dynamically allocated primary mount point is leaked.

Free string obtained via find_mounts() before assigning the pointer to
string returned by find_alt_mount().

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-modesetting.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/usb_moded-modesetting.c b/src/usb_moded-modesetting.c
index 3748bc6..9f77a87 100644
--- a/src/usb_moded-modesetting.c
+++ b/src/usb_moded-modesetting.c
@@ -247,6 +247,7 @@ static int unset_mass_storage_mode(struct mode_list_elem *data)
                                 	log_err("Mounting %s failed\n", mount);
 					if(ret)
 					{
+						g_free(mount);
 						mount = find_alt_mount();
 						if(mount)
 						{

From 58c11bca5e276f57547f0b741e289028970da478 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 14:28:45 +0300
Subject: [PATCH 04/23] [compilation] Remove -pedantic compiler option

Unless the code base is actively used with other than gcc compilers, use
of -pedantic just denies use of handy gcc extensions like use of "%m"
format parameter instead of non-thread-safe strerror().

Drop the '-pedantic' option and replace all strerror() calls with "%m".

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 configure.ac                | 2 +-
 src/usb_moded-modesetting.c | 6 ++----
 src/usb_moded-network.c     | 2 +-
 src/usb_moded.c             | 5 ++---
 4 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index b6c090e..b2501c1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -22,7 +22,7 @@ test_gcc_flag() {
 
 # We use gnu99 instead of c99 because many have interpreted the standard
 # in a way that int64_t isn't defined on non-64 bit platforms.
-CFLAGS="-Os -std=gnu99 -Wall -W -Wextra -pedantic -pipe -Wformat -Wold-style-definition -Wdeclaration-after-statement -Wfloat-equal -Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes -Wendif-labels -Wpointer-arith -Wcast-align -Wwrite-strings -Winline -Wno-unused-parameter -finline-small-functions -Wno-unused-result -fstack-protector -D_FORTIFY_SOURCE=2 -Wl,-z,relro,-z,now -fPIE -fpie -pie"
+CFLAGS="-Os -std=gnu99 -Wall -W -Wextra -pipe -Wformat -Wold-style-definition -Wdeclaration-after-statement -Wfloat-equal -Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes -Wendif-labels -Wpointer-arith -Wcast-align -Wwrite-strings -Winline -Wno-unused-parameter -finline-small-functions -Wno-unused-result -fstack-protector -D_FORTIFY_SOURCE=2 -Wl,-z,relro,-z,now -fPIE -fpie -pie"
 
 AC_ARG_ENABLE([debug], AS_HELP_STRING([--enable-debug],[Enable debug @<:@default=false@:>@]),
   [case "${enableval}" in
diff --git a/src/usb_moded-modesetting.c b/src/usb_moded-modesetting.c
index 9f77a87..e588c21 100644
--- a/src/usb_moded-modesetting.c
+++ b/src/usb_moded-modesetting.c
@@ -61,9 +61,7 @@ int write_to_file(const char *path, const char *text)
   /* no O_CREAT -> writes only to already existing files */
   if( (fd = TEMP_FAILURE_RETRY(open(path, O_WRONLY))) == -1 )
   {
-    /* gcc -pedantic does not like "%m"
-       log_warning("open(%s): %m", path); */
-    log_warning("open(%s): %s", path, strerror(errno));
+    log_warning("open(%s): %m", path);
     goto cleanup;
   }
 
@@ -72,7 +70,7 @@ int write_to_file(const char *path, const char *text)
     ssize_t n = TEMP_FAILURE_RETRY(write(fd, text, todo));
     if( n < 0 )
     {
-      log_warning("write(%s): %s", path, strerror(errno));
+      log_warning("write(%s): %m", path);
       goto cleanup;
     }
     todo -= n;
diff --git a/src/usb_moded-network.c b/src/usb_moded-network.c
index 18d7e12..fcaa5a3 100644
--- a/src/usb_moded-network.c
+++ b/src/usb_moded-network.c
@@ -400,7 +400,7 @@ static int write_udhcpd_conf(struct ipforward_data *ipforward, struct mode_list_
 link:
   if (symlink(UDHCP_CONFIG_PATH, UDHCP_CONFIG_LINK) == -1)
   {
-	log_debug("Error creating link "UDHCP_CONFIG_LINK" -> "UDHCP_CONFIG_PATH": %s\n", strerror(errno));
+	log_debug("Error creating link "UDHCP_CONFIG_LINK" -> "UDHCP_CONFIG_PATH": %m\n");
 	unlink(UDHCP_CONFIG_PATH);
 	return(1);
   }
diff --git a/src/usb_moded.c b/src/usb_moded.c
index 268dac9..674cc2c 100644
--- a/src/usb_moded.c
+++ b/src/usb_moded.c
@@ -938,14 +938,13 @@ static void write_to_sysfs_file(const char *path, const char *text)
 
 	if ((fd = open(path, O_WRONLY)) == -1) {
 		if (errno != ENOENT) {
-			log_warning("%s: open for writing failed: %s",
-				    path, strerror(errno));
+			log_warning("%s: open for writing failed: %m", path);
 		}
 		goto EXIT;
 	}
 
 	if (write(fd, text, strlen(text)) == -1) {
-		log_warning("%s: write failed : %s", path, strerror(errno));
+		log_warning("%s: write failed : %m", path);
 		goto EXIT;
 	}
 EXIT:

From 21400d504fd6abb0c3ac3744eda1272204976063 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 14:40:34 +0300
Subject: [PATCH 05/23] [network] Improve network interface availability
 checking

The buffer used for constructing sysfs path from interface name is rather
short, which can lead to false negatives due to truncation. Additionally
the actual names of the interfaces checked are not logged in case of
errors and the helper function can crash if fed null strings.

Make the check_interface() helper function tolerate null arguments
and enlarge the path expansion buffer to make truncation less likely.

In case get_interface() fails to find valid a interface name, make it
emit diagnostic message that identifies the actual names attempted.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-network.c | 49 ++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/src/usb_moded-network.c b/src/usb_moded-network.c
index fcaa5a3..6dc6252 100644
--- a/src/usb_moded-network.c
+++ b/src/usb_moded-network.c
@@ -81,41 +81,44 @@ static void free_ipforward_data (struct ipforward_data *ipforward)
 /* This function checks if the configured interface exists */
 static int check_interface(char *interface)
 {
-  char path[25];
-  int ret = 0;
+  int ret = -1;
 
-   snprintf(path, 25, "/sys/class/net/%s", interface );
-   ret = access(path, F_OK);
+  if(interface)
+  {
+    char path[256];
+    snprintf(path, sizeof path, "/sys/class/net/%s", interface);
+    ret = access(path, F_OK);
+  }
 
-   return(ret);
+  return ret;
 }
 
 static char* get_interface(struct mode_list_elem *data)
 {
-  char *interface = NULL;
-  int check = 0;
-
-  interface = get_network_setting(NETWORK_INTERFACE_KEY);
-  check = check_interface(interface);
+  char *interface = 0;
+  char *setting   = get_network_setting(NETWORK_INTERFACE_KEY);
 
-  if(check != 0)
+  if(check_interface(setting) == 0)
   {
-	if(interface != NULL)
-		free(interface);
-	/* no known interface configured and existing, falling back to usb0 */
-	interface = malloc(sizeof(default_interface)*sizeof(char));
-	strncpy(interface, default_interface, sizeof(default_interface));
+    /* Use the configured value */
+    interface = setting, setting = 0;
   }
-
-  check = check_interface(interface);
-  if(check)
+  else
   {
-	log_warning("Configured interface is incorrect, nor does usb0 exists. Check your config!\n");
-	free(interface);
-	return(0);
+    /* Fall back to default value */
+    interface = strdup(default_interface);
+    if(check_interface(interface) != 0)
+    {
+      log_warning("Neither configured %s nor fallback %s interface exists."
+                  " Check your config!",
+                  setting   ?: "NULL",
+                  interface ?: "NULL");
+      free(interface), interface = 0;
+    }
   }
 
-  log_debug("interface = %s\n", interface);
+  log_debug("interface = %s\n", interface ?: "NULL");
+  free(setting);
   return interface;
 }
 

From 2abea9532a48d04f4c903ce04225dc727fb51129 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 14:47:14 +0300
Subject: [PATCH 06/23] [appsync] Use correct function to release dynamic
 memory

There is data obtained via g_key_file_get_string() that is released
with free().

Use g_free() instead.

Also use a helper callback function with expected argument types
when releasing a list of elements.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-appsync.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/usb_moded-appsync.c b/src/usb_moded-appsync.c
index 8e66387..77d5e7a 100644
--- a/src/usb_moded-appsync.c
+++ b/src/usb_moded-appsync.c
@@ -55,21 +55,26 @@ static  int no_dbus = 0;
 #endif /* APP_SYNC_DBUS */
 
 
-static void free_elem(gpointer aptr)
+static void free_elem(struct list_elem *elem)
 {
-  struct list_elem *elem = aptr;
-  free(elem->name);
-  free(elem->launch);
-  free(elem->mode);
+  g_free(elem->name);
+  g_free(elem->launch);
+  g_free(elem->mode);
   free(elem);
 }
 
+static void free_elem_cb(gpointer elem, gpointer user_data)
+{
+  (void)user_data;
+  free_elem(elem);
+}
+
 void free_appsync_list(void)
 {
   if( sync_list != 0 )
   {
     /*g_list_free_full(sync_list, free_elem); */
-    g_list_foreach (sync_list, (GFunc) free_elem, NULL);
+    g_list_foreach (sync_list, free_elem_cb, NULL);
     g_list_free (sync_list);
     sync_list = 0;
     log_debug("Appsync list freed\n");

From ddc2942603344574fbeae1e941422ca7a5f5c279 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 14:50:32 +0300
Subject: [PATCH 07/23] [dbus] Fix free() vs g_free() issues

The get_hidden_modes() returns data allocated via g_key_file_get_string()
which should be released with g_free() and fallback uses strdup() which
should use free().

Use g_strdup() in the fallback case and release memory with g_free().

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-dbus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/usb_moded-dbus.c b/src/usb_moded-dbus.c
index 4bb6df1..d848bf0 100644
--- a/src/usb_moded-dbus.c
+++ b/src/usb_moded-dbus.c
@@ -248,10 +248,10 @@ static DBusHandlerResult msg_handler(DBusConnection *const connection, DBusMessa
         {
                 char *config = get_hidden_modes();
                 if(!config)
-                    config = strdup("");
+                    config = g_strdup("");
                 if((reply = dbus_message_new_method_return(msg)))
                         dbus_message_append_args (reply, DBUS_TYPE_STRING, &config, DBUS_TYPE_INVALID);
-                free(config);
+                g_free(config);
         }
 	else if(!strcmp(member, USB_MODE_NETWORK_SET))
 	{

From 20abe602509a29cfcc2825ae74e8d1dc4dcbec26 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 15:22:53 +0300
Subject: [PATCH 08/23] [config] Fix handling of hidden modes setting

The make_hidden_modes_string() function claims to return const string,
but that is true only as an exception to the rule. Since the callers of
the function do not release dynamic memory, this leads to leakage. Also
if no mode is hidden, request to unhide something least to that mode
getting hidden and hiding something already hidden returns NULL which
is rather unintuitive.

Fix make_hidden_modes_string() and make all callers of it to release
dynamically reserved memory / deal with possible null return value.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-config.c | 103 ++++++++++++++++++++++++++---------------
 1 file changed, 65 insertions(+), 38 deletions(-)

diff --git a/src/usb_moded-config.c b/src/usb_moded-config.c
index 50a3c8e..28422b4 100644
--- a/src/usb_moded-config.c
+++ b/src/usb_moded-config.c
@@ -416,75 +416,102 @@ set_config_result_t set_mode_setting(const char *mode)
 
 /* Builds the string used for hidden modes, when hide set to one builds the
    new string of hidden modes when adding one, otherwise it will remove one */
-static const char * make_hidden_modes_string(const char *hidden, int hide)
+static char * make_hidden_modes_string(const char *mode_name, int hide)
 {
-  GString *modelist_str;
-  char *hidden_modes_list;
-  gchar **hidden_mode_split;
+  char     *hidden_new = 0;
+  char     *hidden_old = 0;
+  gchar   **hidden_arr = 0;
+  GString  *hidden_tmp = 0;
   int i;
 
-
-  hidden_modes_list = get_hidden_modes();
-  if(hidden_modes_list)
-  {
-    hidden_mode_split = g_strsplit(hidden_modes_list, ",", 0);
-  }
-  else
+  /* Get current comma separated list of hidden modes */
+  hidden_old = get_hidden_modes();
+  if(!hidden_old)
   {
-    /* no hidden modes yet. So just return the original string */
-    return hidden;
+    hidden_old = g_strdup("");
   }
 
-  modelist_str = g_string_new(NULL);
+  hidden_arr = g_strsplit(hidden_old, ",", 0);
 
-  for(i = 0; hidden_mode_split[i] != NULL; i++)
+  hidden_tmp = g_string_new(NULL);
+
+  for(i = 0; hidden_arr[i] != NULL; i++)
   {
-     if(strlen(hidden_mode_split[i]) == 0)
-       continue;
-     if(!strcmp(hidden_mode_split[i], hidden))
-     {
-	/* if hiding a mode that is already hidden do nothing */
-	if(hide)
-		return(NULL);
-        if(!hide)
-        	continue;
-     }
-     if(strlen(modelist_str->str) != 0)
-       modelist_str = g_string_append(modelist_str, ",");
-     modelist_str = g_string_append(modelist_str, hidden_mode_split[i]);
+    if(strlen(hidden_arr[i]) == 0)
+    {
+      /* Skip any empty strings */
+      continue;
+    }
+
+    if(!strcmp(hidden_arr[i], mode_name))
+    {
+      /* When unhiding, just skip all matching entries */
+      if(!hide)
+        continue;
+
+      /* When hiding, keep the 1st match and ignore the rest */
+      hide = 0;
+    }
+
+    if(hidden_tmp->len > 0)
+      hidden_tmp = g_string_append(hidden_tmp, ",");
+    hidden_tmp = g_string_append(hidden_tmp, hidden_arr[i]);
   }
+
   if(hide)
   {
-     if(strlen(modelist_str->str) != 0)
-       modelist_str = g_string_append(modelist_str, ",");
-     modelist_str = g_string_append(modelist_str, hidden);
+    /* Adding a hidden mode and no matching entry was found */
+    if(hidden_tmp->len > 0)
+      hidden_tmp = g_string_append(hidden_tmp, ",");
+    hidden_tmp = g_string_append(hidden_tmp, mode_name);
   }
-  
-  g_strfreev(hidden_mode_split);
-  return(g_string_free(modelist_str, FALSE));
+
+  hidden_new = g_string_free(hidden_tmp, FALSE), hidden_tmp = 0;
+
+  g_strfreev(hidden_arr), hidden_arr = 0;
+
+  g_free(hidden_old), hidden_old = 0;
+
+  return hidden_new;
 }
 
 set_config_result_t set_hide_mode_setting(const char *mode)
 {
-  set_config_result_t ret;
+  set_config_result_t ret = SET_CONFIG_UNCHANGED;
+
+  char *hidden_modes = make_hidden_modes_string(mode, 1);
+
+  if( hidden_modes ) {
+    ret = set_config_setting(MODE_SETTING_ENTRY, MODE_HIDE_KEY, hidden_modes);
+  }
 
-  ret = set_config_setting(MODE_SETTING_ENTRY, MODE_HIDE_KEY, make_hidden_modes_string(mode, 1));
   if(ret == SET_CONFIG_UPDATED) {
       send_hidden_modes_signal();
       send_supported_modes_signal();
   }
+
+  g_free(hidden_modes);
+
   return(ret);
 }
 
 set_config_result_t set_unhide_mode_setting(const char *mode)
 {
-  set_config_result_t ret;
+  set_config_result_t ret = SET_CONFIG_UNCHANGED;
+
+  char *hidden_modes = make_hidden_modes_string(mode, 0);
+
+  if( hidden_modes ) {
+    ret = set_config_setting(MODE_SETTING_ENTRY, MODE_HIDE_KEY, hidden_modes);
+  }
 
-  ret = set_config_setting(MODE_SETTING_ENTRY, MODE_HIDE_KEY, make_hidden_modes_string(mode, 0));
   if(ret == SET_CONFIG_UPDATED) {
       send_hidden_modes_signal();
       send_supported_modes_signal();
   }
+
+  g_free(hidden_modes);
+
   return(ret);
 }
 

From 435e952bee32170d91a106ef8e501de3fe771db5 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 15:26:40 +0300
Subject: [PATCH 09/23] [config] Check correct paramter in
 config_value_changed() predicate

The function compares old value with ini file section name, which leads
to practically every new value being evaluated different from previous one.

Check old value vs new value instead.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/usb_moded-config.c b/src/usb_moded-config.c
index 28422b4..d8997eb 100644
--- a/src/usb_moded-config.c
+++ b/src/usb_moded-config.c
@@ -362,7 +362,7 @@ int config_value_changed(GKeyFile *settingsfile, const char *entry, const char *
   char *old = g_key_file_get_string(settingsfile, entry, key, NULL);
   if (old)
   {
-	gboolean unchanged = (g_strcmp0(old, entry) == 0);
+	gboolean unchanged = (g_strcmp0(old, new_value) == 0);
         g_free(old);
         if (unchanged)
         {

From 23ea040347823c5bff8d03caf73ecfd7ef56f865 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 15:32:31 +0300
Subject: [PATCH 10/23] [config] Reverse the meaning of config_value_changed()
 return value

Having a predicate function with name xxx_changed() that returns a
non-zero value when nothing has changed is confusing.

Switch the return value meaning so that non-zero means there is a
difference and zero that there is not.

Also treat old value of NULL and new value of NULL as no change.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-config.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/src/usb_moded-config.c b/src/usb_moded-config.c
index d8997eb..423cf0a 100644
--- a/src/usb_moded-config.c
+++ b/src/usb_moded-config.c
@@ -355,22 +355,14 @@ char * get_mode_setting(void)
  *  @param key: key value of the entry we want to read
  *  @new_value: potentially new value we want to compare against
  *
- *  @return: 1 when the old value is the same as the new one, 0 otherwise
+ *  @return: 0 when the old value is the same as the new one, 1 otherwise
  */
 int config_value_changed(GKeyFile *settingsfile, const char *entry, const char *key, const char *new_value)
 {
-  char *old = g_key_file_get_string(settingsfile, entry, key, NULL);
-  if (old)
-  {
-	gboolean unchanged = (g_strcmp0(old, new_value) == 0);
-        g_free(old);
-        if (unchanged)
-        {
-                return 1;
-        }
-  }
-	
-  return 0;
+  char *old_value = g_key_file_get_string(settingsfile, entry, key, NULL);
+  int changed = (g_strcmp0(old_value, new_value) != 0);
+  g_free(old_value);
+  return changed;
 }
 
 set_config_result_t set_config_setting(const char *entry, const char *key, const char *value)
@@ -384,7 +376,7 @@ set_config_result_t set_config_setting(const char *entry, const char *key, const
   test = g_key_file_load_from_file(settingsfile, FS_MOUNT_CONFIG_FILE, G_KEY_FILE_NONE, NULL);
   if(test)
   {
-      if(config_value_changed(settingsfile, entry, key, value))
+      if(!config_value_changed(settingsfile, entry, key, value))
       {
               g_key_file_free(settingsfile);
               return SET_CONFIG_UNCHANGED;
@@ -537,7 +529,7 @@ set_config_result_t set_network_setting(const char *config, const char *setting)
 	set_config_result_t ret = SET_CONFIG_ERROR;
 	if (test)
 	{
-		if(config_value_changed(settingsfile, NETWORK_ENTRY, config, setting))
+		if(!config_value_changed(settingsfile, NETWORK_ENTRY, config, setting))
 		{
 			g_key_file_free(settingsfile);
 			return SET_CONFIG_UNCHANGED;

From 839fda62be8085943d217b3dc7c83e9bc3f37bda Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 15:39:22 +0300
Subject: [PATCH 11/23] [modesetting] Clear timer id when network_retry timer
 is triggered

When network_retry() timer callback gets triggered, the associated
timer id delayed_network is not invalidated and usb moded can try
to remove it later on - which will lead to complaints from glib
being emitted to journal.

Clear the timer id when the timer callback gets called.

Also make sure already existing timer id is not left active when
scheduling a timeout.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-modesetting.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/usb_moded-modesetting.c b/src/usb_moded-modesetting.c
index e588c21..d9c3216 100644
--- a/src/usb_moded-modesetting.c
+++ b/src/usb_moded-modesetting.c
@@ -43,7 +43,7 @@
 #include "usb_moded-android.h"
 
 static void report_mass_storage_blocker(const char *mountpoint, int try);
-guint delayed_network = 0;
+static guint delayed_network = 0;
 
 int write_to_file(const char *path, const char *text)
 {
@@ -88,6 +88,7 @@ int write_to_file(const char *path, const char *text)
 
 static gboolean network_retry(gpointer data)
 {
+	delayed_network = 0;
 	usb_network_up(data);
 	return(FALSE);
 }
@@ -399,6 +400,8 @@ int set_dynamic_mode(void)
   if(network != 0 && data->network)
   {
 	log_debug("Retry setting up the network later\n");
+	if(delayed_network)
+		  g_source_remove(delayed_network);
 	delayed_network = g_timeout_add_seconds(3, network_retry, data);
   }
 

From 7bc44985442ef97ff8b3f97e5849a847b69c440d Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 16:03:46 +0300
Subject: [PATCH 12/23] [trigger] Fix udev iowatch issues

Various pointers are uninitialized / left to hold stale values after
releasing. Which could lead to hard to debug problems if trigger_stop() is
called multiple times / after unsuccessful trigger_init().

Zero initialize all pointers dealing with dynamic resources and clear them
when resources are released.

Also clear iowatch id when it is going to be implicitly removed due to the
return value from the callback so that removal is not attempted anymore
when/if trigger_stop() is called later on.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-trigger.c | 43 +++++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/src/usb_moded-trigger.c b/src/usb_moded-trigger.c
index d9d9ed3..f7e9114 100644
--- a/src/usb_moded-trigger.c
+++ b/src/usb_moded-trigger.c
@@ -43,10 +43,10 @@
 #endif /* MEEGOLOCK */
 
 /* global variables */
-static struct udev *udev;
-static struct udev_monitor *mon;
-static GIOChannel *iochannel;
-static guint watch_id; 
+static struct udev *udev = 0;
+static struct udev_monitor *mon = 0;
+static GIOChannel *iochannel = 0;
+static guint watch_id = 0;
 static const char *dev_name;
 
 /* static function definitions */
@@ -138,8 +138,11 @@ static gboolean monitor_udev(GIOChannel *iochannel G_GNUC_UNUSED, GIOCondition c
     if (dev) 
     {
       /* check if it is the actual device we want to check */
-      if(strcmp(dev_name, udev_device_get_sysname(dev)))
-	return 0;
+      if(strcmp(dev_name, udev_device_get_sysname(dev))) {
+        log_crit("name does not match, disabling udev trigger io-watch");
+        watch_id = 0;
+        return FALSE;
+      }
        
       if(!strcmp(udev_device_get_action(dev), "change"))
       {
@@ -152,7 +155,9 @@ static gboolean monitor_udev(GIOChannel *iochannel G_GNUC_UNUSED, GIOCondition c
     else
     {
    	log_debug("Bad trigger data. Stopping\n");
+        watch_id = 0;
         trigger_stop();
+        return FALSE;
     }
   }
   
@@ -162,12 +167,26 @@ static gboolean monitor_udev(GIOChannel *iochannel G_GNUC_UNUSED, GIOCondition c
 
 void trigger_stop(void)
 {
-  g_source_remove(watch_id);
-  watch_id = 0;
-  g_io_channel_unref(iochannel);
-  iochannel = NULL;
-  udev_monitor_unref(mon);
-  udev_unref(udev);
+  if(watch_id)
+  {
+    g_source_remove(watch_id);
+    watch_id = 0;
+  }
+  if(iochannel) {
+    g_io_channel_unref(iochannel);
+    iochannel = NULL;
+  }
+  if(mon)
+  {
+    udev_monitor_unref(mon);
+    mon = 0;
+  }
+  if(udev)
+  {
+    udev_unref(udev);
+    udev = 0;
+  }
+  dev_name = 0;
 }
 
 static void udev_parse(struct udev_device *dev)

From 6ec3a7308eb02cdbe654109be02f544da194a655 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 16:07:25 +0300
Subject: [PATCH 13/23] [udev] Fix udev iowatch issues

The io watch source id is not cleared when it is going to be implicitly
removed - which can lead to unsuccessful removal attempt when/if
hwal_cleanup() function is called later on.

Clear watch_id when return value from the callback function is going
to cause it to be implicitly removed.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-udev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/usb_moded-udev.c b/src/usb_moded-udev.c
index 62c49c5..7f7fae1 100644
--- a/src/usb_moded-udev.c
+++ b/src/usb_moded-udev.c
@@ -264,8 +264,9 @@ static gboolean monitor_udev(GIOChannel *iochannel G_GNUC_UNUSED, GIOCondition c
 
   release_wakelock(USB_MODED_WAKELOCK_PROCESS_INPUT);
 
-  if (!continue_watching)
+  if (!continue_watching && watch_id )
   {
+    watch_id = 0;
     log_crit("udev io watch disabled");
   }
 

From de044e25f758d66df78755bcd364eed62bcb69bc Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 16:11:30 +0300
Subject: [PATCH 14/23] [usb_moded] Make sure charging timeout timer ids are
 not leaked

If charging_timeout should already contain a valid timer id when the
timer is started, the old timer will be leaked.

Remove existing timer id before assigning new one.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/usb_moded.c b/src/usb_moded.c
index 674cc2c..44f85bb 100644
--- a/src/usb_moded.c
+++ b/src/usb_moded.c
@@ -294,6 +294,8 @@ void set_usb_connected_state(void)
 	 	*/
 		usb_moded_send_signal(USB_CONNECTED_DIALOG_SHOW);
 		/* fallback to charging mode after 3 seconds */
+		if( charging_timeout )
+			g_source_remove(charging_timeout);
 		charging_timeout = g_timeout_add_seconds(3, charging_fallback, NULL);
 		/* in case there was nobody listening for the UI, they will know 
 		   that the UI is needed by requesting the current mode */

From 95557203747f62ae34595ac203c76e85c90fdcb9 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 16:19:23 +0300
Subject: [PATCH 15/23] [systemd] Fix memory leak on systemd_control_service()
 error path

If dbus_connection_send_with_reply_and_block() should set the
dbus error, it is never freed.

Make sure dbus_error_free() is called before returning from the function.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-systemd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/usb_moded-systemd.c b/src/usb_moded-systemd.c
index 95581a3..4a27cfb 100644
--- a/src/usb_moded-systemd.c
+++ b/src/usb_moded-systemd.c
@@ -96,6 +96,7 @@ int systemd_control_service(const char *name, const char *method)
 quit:
   dbus_connection_close(bus);
   dbus_connection_unref(bus);
+  dbus_error_free(&error);
 
   return(ret); 
 }

From 3d6e10ec067ee84492dd06d3918f6077f2d3c37a Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 16:28:37 +0300
Subject: [PATCH 16/23] [modesetting] On debug verbosity log all sysfs writes
 made

When debugging mode setting issues, it is hard to tell when and what gets
written to android usb sysfs files.

When running usb-moded in debug verbosity, log sysfs paths that get written
to and the data that is written - and if possible also the value held by
the sysfs file before the write.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-modesetting.c | 66 +++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/usb_moded-modesetting.c b/src/usb_moded-modesetting.c
index d9c3216..33bbad5 100644
--- a/src/usb_moded-modesetting.c
+++ b/src/usb_moded-modesetting.c
@@ -45,6 +45,61 @@
 static void report_mass_storage_blocker(const char *mountpoint, int try);
 static guint delayed_network = 0;
 
+#if LOG_ENABLE_DEBUG
+static char *strip(char *str)
+{
+  unsigned char *src = (unsigned char *)str;
+  unsigned char *dst = (unsigned char *)str;
+
+  while( *src > 0 && *src <= 32 ) ++src;
+
+  for( ;; )
+  {
+    while( *src > 32 ) *dst++ = *src++;
+    while( *src > 0 && *src <= 32 ) ++src;
+    if( *src == 0 ) break;
+    *dst++ = ' ';
+  }
+  *dst = 0;
+  return str;
+}
+
+static char *read_from_file(const char *path, size_t maxsize)
+{
+  int      fd   = -1;
+  ssize_t  done = 0;
+  char    *data = 0;
+  char    *text = 0;
+
+  if((fd = open(path, O_RDONLY)) == -1)
+  {
+    /* Silently ignore things that could result
+     * from missing / read-only files */
+    if( errno != ENOENT && errno != EACCES )
+      log_warning("%s: open: %m", path);
+    goto cleanup;
+  }
+
+  if( !(data = malloc(maxsize + 1)) )
+    goto cleanup;
+
+  if((done = read(fd, data, maxsize)) == -1)
+  {
+    log_warning("%s: read: %m", path);
+    goto cleanup;
+  }
+
+  text = realloc(data, done + 1), data = 0;
+  text[done] = 0;
+  strip(text);
+
+cleanup:
+  free(data);
+  if(fd != -1) close(fd);
+  return text;
+}
+#endif /* LOG_ENABLE_DEBUG */
+
 int write_to_file(const char *path, const char *text)
 {
   int err = -1;
@@ -56,6 +111,15 @@ int write_to_file(const char *path, const char *text)
   if(!text || !path)
 	return err;
 
+#if LOG_ENABLE_DEBUG
+  if(log_level >= LOG_DEBUG)
+  {
+    char *prev = read_from_file(path, 0x1000);
+    log_debug("WRITE '%s' : '%s' --> '%s'", path, prev ?: "???", text);
+    free(prev);
+  }
+#endif
+
   todo  = strlen(text);
 
   /* no O_CREAT -> writes only to already existing files */
@@ -362,7 +426,6 @@ int set_dynamic_mode(void)
   if(data->sysfs_path)
   {
 	write_to_file(data->sysfs_path, data->sysfs_value);
-	log_debug("writing to file %s, value %s\n", data->sysfs_path, data->sysfs_value);
   }
   if(data->idProduct)
   {
@@ -469,7 +532,6 @@ void unset_dynamic_mode(void)
   if(data->sysfs_path)
   {
 	write_to_file(data->sysfs_path, data->sysfs_reset_value);
-	log_debug("writing to file %s, value %s\n", data->sysfs_path, data->sysfs_reset_value);
   }
   /* restore vendorid if the mode had an override */
   if(data->idVendorOverride)

From 358b23d673ba27d8e67236dcc4120838120580e8 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 16:38:40 +0300
Subject: [PATCH 17/23] [modesetting] Do not leave android usb disabled after
 unsetting mode. JB#35683

Seems that just writing enable=0 functions=none leaves the android usb in
some sort of limbo where cable detection does not work until enable=1
write is also made.

Do enable=1 write after clearing functions on cable disconnect.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-modesetting.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/usb_moded-modesetting.c b/src/usb_moded-modesetting.c
index 33bbad5..61e2eb1 100644
--- a/src/usb_moded-modesetting.c
+++ b/src/usb_moded-modesetting.c
@@ -541,6 +541,12 @@ void unset_dynamic_mode(void)
 	set_android_vendorid(id);
 	g_free(id);
   }
+
+  /* enable after the changes have been made */
+  if(data->softconnect)
+  {
+	write_to_file(data->softconnect_path, data->softconnect);
+  }
 }
 
 /** clean up mode changes or extra actions to perform after a mode change 

From 750ce667899ec90612308c966160f74d947e3ae8 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Wed, 6 Jul 2016 16:43:01 +0300
Subject: [PATCH 18/23] [android] Make sure android usb is enabled on usb-moded
 startup. JB#35683

If android usb is in enable=0 state, cable detection is persistently
broken as the existing workarounds in usb-moded are made when cable
disconnect is detected and that can't happen unless cable detect works.

Enable android usb on usb-moded startup.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-android.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/usb_moded-android.c b/src/usb_moded-android.c
index 565cadc..d0dafb9 100644
--- a/src/usb_moded-android.c
+++ b/src/usb_moded-android.c
@@ -81,6 +81,10 @@ void android_init_values(void)
   }
   /* For rndis to be discovered correctly in M$ Windows (vista and later) */
   write_to_file("/sys/class/android_usb/f_rndis/wceis", "1");
+
+  /* Make sure android_usb does not stay disabled in case usb moded
+   * has crashed / been killed in inconvenient time. */
+  write_to_file("/sys/class/android_usb/android0/enable", "1");
 }
 
 /* Set a charging mode for the android gadget

From 3ef00b3f7c032f982f354638f9b30baad8779f4f Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Thu, 7 Jul 2016 09:05:02 +0300
Subject: [PATCH 19/23] [appsync] Cancel enumeration timeout when enumerating

Enumeration timer is started every time activate_sync() is called and
is left running even if enumeration occurs / appsync_stop() is called.

Cache enumeration timer id and cancel it when it is no longer needed.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-appsync.c | 51 +++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/src/usb_moded-appsync.c b/src/usb_moded-appsync.c
index 77d5e7a..e72372b 100644
--- a/src/usb_moded-appsync.c
+++ b/src/usb_moded-appsync.c
@@ -41,10 +41,14 @@
 #include "usb_moded-systemd.h"
 
 static struct list_elem *read_file(const gchar *filename, int diag);
-static gboolean enumerate_usb(gpointer data);
+static void enumerate_usb(void);
+static gboolean enumerate_usb_cb(gpointer data);
+static void start_enumerate_usb_timer(void);
+static void cancel_enumerate_usb_timer(void);
 
 static GList *sync_list = NULL;
 
+static guint enumerate_usb_id = 0;
 static unsigned sync_tag = 0;
 static unsigned enum_tag = 0;
 static struct timeval sync_tv;
@@ -200,7 +204,7 @@ int activate_sync(const char *mode)
   if( sync_list == 0 )
   {
     log_debug("No sync list! Enumerating\n");
-    enumerate_usb(NULL);
+    enumerate_usb();
     return 0;
   }
 
@@ -224,7 +228,7 @@ int activate_sync(const char *mode)
   if(count == count2)
   {
       log_debug("Nothing to launch.\n");
-      enumerate_usb(NULL);
+      enumerate_usb();
       return(0);
    }
 
@@ -238,8 +242,7 @@ int activate_sync(const char *mode)
 #endif /* APP_SYNC_DBUS */
 
   /* start timer */
-  log_debug("Starting appsync timer\n");
-  g_timeout_add_seconds(2, enumerate_usb, NULL);
+  start_enumerate_usb_timer();
 
   /* go through list and launch apps */
   for( iter = sync_list; iter; iter = g_list_next(iter) )
@@ -376,17 +379,47 @@ int mark_active(const gchar *name, int post)
   if( !missing )
   {
     log_debug("All apps active. Let's enumerate\n");
-    enumerate_usb(NULL);
+    enumerate_usb();
   }
   
   /* -1=not found, 0=already active, 1=activated now */
   return ret; 
 }
 
-static gboolean enumerate_usb(gpointer data)
+static gboolean enumerate_usb_cb(gpointer data)
+{
+  (void)data;
+  enumerate_usb_id = 0;
+  log_debug("handling enumeration timeout");
+  enumerate_usb();
+  /* return false to stop the timer from repeating */
+  return FALSE;
+}
+
+static void start_enumerate_usb_timer(void)
+{
+  log_debug("scheduling enumeration timeout");
+  if( enumerate_usb_id )
+    g_source_remove(enumerate_usb_id), enumerate_usb_id = 0;
+  enumerate_usb_id = g_timeout_add_seconds(2, enumerate_usb_cb, NULL);
+}
+
+static void cancel_enumerate_usb_timer(void)
+{
+  if( enumerate_usb_id )
+  {
+    log_debug("canceling enumeration timeout");
+    g_source_remove(enumerate_usb_id), enumerate_usb_id = 0;
+  }
+}
+
+static void enumerate_usb(void)
 {
   struct timeval tv;
 
+  /* Stop the timer in case of explicit enumeration call */
+  cancel_enumerate_usb_timer();
+
   /* We arrive here twice: when app sync is done
    * and when the app sync timeout gets triggered.
    * The tags are used to filter out these repeats.
@@ -411,8 +444,6 @@ static gboolean enumerate_usb(gpointer data)
     usb_moded_appsync_cleanup();
 #endif /* APP_SYNC_DBUS */
   }
-  /* return false to stop the timer from repeating */
-  return FALSE;
 }
 
 int appsync_stop(void)
@@ -430,5 +461,7 @@ int appsync_stop(void)
     }
   }
 
+  /* Do not leave active timers behind */
+  cancel_enumerate_usb_timer();
   return(0);
 }

From 27518bf06f4e8f13aaf175b25666531d24d667eb Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Thu, 7 Jul 2016 09:14:28 +0300
Subject: [PATCH 20/23] [appsync] Fix condition for logging systemd service
 stop problems

The systemd_control_service() function returns zero on success. Due to
incorrect boolean check in appsync_stop() diagnostic failure messages are
emitted on success and failures are ignored silently.

Fix the condition so that failures are logged.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-appsync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/usb_moded-appsync.c b/src/usb_moded-appsync.c
index e72372b..2d1136f 100644
--- a/src/usb_moded-appsync.c
+++ b/src/usb_moded-appsync.c
@@ -456,7 +456,7 @@ int appsync_stop(void)
 
     if(data->systemd)
     {
-        if(!systemd_control_service(data->name, SYSTEMD_STOP))
+        if(systemd_control_service(data->name, SYSTEMD_STOP))
 		log_debug("Failed to stop %s\n", data->name);
     }
   }

From df105a6b09937a73ee2710593f144e4888443870 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Thu, 7 Jul 2016 09:32:24 +0300
Subject: [PATCH 21/23] [appsync] Stop post-enum apps before stopping pre-enum
 apps. JB#35683

When appsync is starting applications, pre-enum applications are started
before post-enum apps. However stopping applications is done in whatever
order they happen to be after configuration parsing - which could cause
problems if post apps have explicit dependencies on pre apps.

Do also application stopping in two phases: first post-enum apps followed
by pre-enum apps.

Also, apart from usb-moded startup/shutdown, attempt to skip stopping of
applications that have not been started.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-appsync.c     | 45 ++++++++++++++++++++++++++++---------
 src/usb_moded-appsync.h     |  2 +-
 src/usb_moded-modesetting.c |  3 ++-
 src/usb_moded.c             |  4 ++--
 4 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/src/usb_moded-appsync.c b/src/usb_moded-appsync.c
index 2d1136f..ff79dfb 100644
--- a/src/usb_moded-appsync.c
+++ b/src/usb_moded-appsync.c
@@ -253,10 +253,9 @@ int activate_sync(const char *mode)
       /* do not launch items marked as post, will be launched after usb is up */
       if(data->post)
       {
-	mark_active(data->name, data->post);
 	continue;
       }
-      log_debug("launching app %s\n", data->name);
+      log_debug("launching pre-enum-app %s\n", data->name);
       if(data->systemd)
       {
         if(!systemd_control_service(data->name, SYSTEMD_START))
@@ -318,11 +317,12 @@ int activate_sync_post(const char *mode)
       /* launch only items marked as post, others are already running */
       if(!data->post)
 	continue;
-      log_debug("launching app %s\n", data->name);
+      log_debug("launching post-enum-app %s\n", data->name);
       if(data->systemd)
       {
         if(systemd_control_service(data->name, SYSTEMD_START))
 		goto error;
+	mark_active(data->name, 1);
       }
       else if(data->launch)
       {
@@ -353,12 +353,12 @@ int mark_active(const gchar *name, int post)
 
   GList *iter;
 
-  if(post)
-    log_debug("App %s notified it is ready\n", name);
+  log_debug("%s-enum-app %s is started\n", post ? "post" : "pre", name);
 
   for( iter = sync_list; iter; iter = g_list_next(iter) )
   {
     struct list_elem *data = iter->data;
+
     if(!strcmp(data->name, name))
     {
       /* TODO: do we need to worry about duplicate names in the list? */
@@ -368,7 +368,7 @@ int mark_active(const gchar *name, int post)
       /* updated + missing -> not going to enumerate */
       if( missing ) break;
     }
-    else if( data->active == 0 )
+    else if( data->active == 0 && data->post == post )
     {
       missing = 1;
 
@@ -376,9 +376,9 @@ int mark_active(const gchar *name, int post)
       if( ret != -1 ) break;
     }
   }
-  if( !missing )
+  if( !post && !missing )
   {
-    log_debug("All apps active. Let's enumerate\n");
+    log_debug("All pre-enum-apps active. Let's enumerate\n");
     enumerate_usb();
   }
   
@@ -446,7 +446,7 @@ static void enumerate_usb(void)
   }
 }
 
-int appsync_stop(void)
+static void appsync_stop_apps(int post)
 {
   GList *iter = 0;
 
@@ -454,13 +454,38 @@ int appsync_stop(void)
   {
     struct list_elem *data = iter->data;
 
-    if(data->systemd)
+    if(data->systemd && data->active && data->post == post)
     {
+      log_debug("stopping %s-enum-app %s", post ? "post" : "pre", data->name);
         if(systemd_control_service(data->name, SYSTEMD_STOP))
 		log_debug("Failed to stop %s\n", data->name);
+      data->active = 0;
+    }
+  }
+}
+
+int appsync_stop(int force)
+{
+  /* If force arg is used, stop all applications that
+   * could have been started by usb-moded */
+  if(force)
+  {
+    GList *iter;
+    log_debug("assuming all applications are active");
+
+    for( iter = sync_list; iter; iter = g_list_next(iter) )
+    {
+      struct list_elem *data = iter->data;
+      data->active = 1;
     }
   }
 
+  /* Stop post-apps 1st */
+  appsync_stop_apps(1);
+
+  /* Then pre-apps */
+  appsync_stop_apps(0);
+
   /* Do not leave active timers behind */
   cancel_enumerate_usb_timer();
   return(0);
diff --git a/src/usb_moded-appsync.h b/src/usb_moded-appsync.h
index 14ae85c..0c33b93 100644
--- a/src/usb_moded-appsync.h
+++ b/src/usb_moded-appsync.h
@@ -48,6 +48,6 @@ void readlist(int diag);
 int activate_sync(const char *mode);
 int activate_sync_post(const char *mode);
 int mark_active(const gchar *name, int post);
-int appsync_stop(void);
+int appsync_stop(int force);
 void free_appsync_list(void);
 void usb_moded_appsync_cleanup(void);
diff --git a/src/usb_moded-modesetting.c b/src/usb_moded-modesetting.c
index 61e2eb1..6d01f26 100644
--- a/src/usb_moded-modesetting.c
+++ b/src/usb_moded-modesetting.c
@@ -566,7 +566,8 @@ int usb_moded_mode_cleanup(const char *module)
 	}
 
 #ifdef APP_SYNC
-	appsync_stop();
+	/* Stop applications started due to entering this mode */
+	appsync_stop(0);
 #endif /* APP_SYNC */
 
         if(!strcmp(module, MODULE_MASS_STORAGE)|| !strcmp(module, MODULE_FILE_STORAGE))
diff --git a/src/usb_moded.c b/src/usb_moded.c
index 44f85bb..382b6a0 100644
--- a/src/usb_moded.c
+++ b/src/usb_moded.c
@@ -623,7 +623,7 @@ static void usb_moded_init(void)
 #ifdef APP_SYNC
   readlist(diag_mode);
   /* make sure all services are down when starting */
-  appsync_stop();
+  appsync_stop(1);
   modelist = read_mode_list(diag_mode);
 #endif /* APP_SYNC */
 
@@ -670,7 +670,7 @@ static gboolean charging_fallback(gpointer data)
 static void handle_exit(void)
 {
   /* exiting and clean-up when mainloop ended */
-  appsync_stop();
+  appsync_stop(1);
   hwal_cleanup();
   usb_moded_dbus_cleanup();
 #ifdef MEEGOLOCK

From 206c9573862cf8eeff7bc3cfc1ff2ae7c1007d47 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Thu, 7 Jul 2016 09:41:08 +0300
Subject: [PATCH 22/23] [appsync] Start/stop only applications relevant to
 mode. JB#35683

The boolean state data for tracking active/inactive applications makes
it difficult to separate applications that are not relevant to the mode
and as a result an attempt to stop also those irrelevant apps that were
not started is made when exiting the mode.

Switch from boolean to inactive|active|dontcare state tracking.

When starting appsync, mark apps relevant to the mode as inactive and
the rest as dontcare.

When stopping appsync, stop only those applications that have been
activated by usb-moded.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-appsync.c | 32 +++++++++++++++++---------------
 src/usb_moded-appsync.h | 11 ++++++++++-
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/src/usb_moded-appsync.c b/src/usb_moded-appsync.c
index ff79dfb..74fe118 100644
--- a/src/usb_moded-appsync.c
+++ b/src/usb_moded-appsync.c
@@ -172,6 +172,7 @@ static struct list_elem *read_file(const gchar *filename, int diag)
   list_item->systemd = g_key_file_get_integer(settingsfile, APP_INFO_ENTRY, APP_INFO_SYSTEMD_KEY, NULL);
   log_debug("Systemd control = %d\n", list_item->systemd);
   list_item->post = g_key_file_get_integer(settingsfile, APP_INFO_ENTRY, APP_INFO_POST, NULL);
+  list_item->state = APP_STATE_DONTCARE;
 
 cleanup:
 
@@ -194,7 +195,7 @@ static struct list_elem *read_file(const gchar *filename, int diag)
 int activate_sync(const char *mode)
 {
   GList *iter;
-  int count = 0, count2 = 0;
+  int count = 0;
 
   log_debug("activate sync");
 
@@ -208,24 +209,25 @@ int activate_sync(const char *mode)
     return 0;
   }
 
-  /* set list to inactive, mark other modes as active already */
+  /* Count apps that need to be activated for this mode and
+   * mark them as currently inactive */
   for( iter = sync_list; iter; iter = g_list_next(iter) )
   {
     struct list_elem *data = iter->data;
 
-    count++;
     if(!strcmp(data->mode, mode))
-    	data->active = 0;
+    {
+      ++count;
+      data->state = APP_STATE_INACTIVE;
+    }
     else
     {
-	count2++;
-	data->active = 1;
+      data->state = APP_STATE_DONTCARE;
     }
   }
 
-  /* if the number of active modes is equal to the number of existing modes
-     we enumerate immediately */
-  if(count == count2)
+  /* If there is nothing to activate, enumerate immediately */
+  if(count <= 0)
   {
       log_debug("Nothing to launch.\n");
       enumerate_usb();
@@ -362,13 +364,13 @@ int mark_active(const gchar *name, int post)
     if(!strcmp(data->name, name))
     {
       /* TODO: do we need to worry about duplicate names in the list? */
-      ret = !data->active;
-      data->active = 1;
+      ret = (data->state != APP_STATE_ACTIVE);
+      data->state = APP_STATE_ACTIVE;
 
       /* updated + missing -> not going to enumerate */
       if( missing ) break;
     }
-    else if( data->active == 0 && data->post == post )
+    else if( data->state == APP_STATE_INACTIVE && data->post == post )
     {
       missing = 1;
 
@@ -454,12 +456,12 @@ static void appsync_stop_apps(int post)
   {
     struct list_elem *data = iter->data;
 
-    if(data->systemd && data->active && data->post == post)
+    if(data->systemd && data->state == APP_STATE_ACTIVE && data->post == post)
     {
       log_debug("stopping %s-enum-app %s", post ? "post" : "pre", data->name);
         if(systemd_control_service(data->name, SYSTEMD_STOP))
 		log_debug("Failed to stop %s\n", data->name);
-      data->active = 0;
+      data->state = APP_STATE_DONTCARE;
     }
   }
 }
@@ -476,7 +478,7 @@ int appsync_stop(int force)
     for( iter = sync_list; iter; iter = g_list_next(iter) )
     {
       struct list_elem *data = iter->data;
-      data->active = 1;
+      data->state = APP_STATE_ACTIVE;
     }
   }
 
diff --git a/src/usb_moded-appsync.h b/src/usb_moded-appsync.h
index 0c33b93..3cca417 100644
--- a/src/usb_moded-appsync.h
+++ b/src/usb_moded-appsync.h
@@ -29,6 +29,15 @@
 #define APP_INFO_SYSTEMD_KEY	"systemd"
 #define APP_INFO_POST		"post"
 
+typedef enum {
+  /** Application is not relevant for the current mode */
+  APP_STATE_DONTCARE = 0,
+  /** Application should be started */
+  APP_STATE_INACTIVE = 1,
+  /** Application should be stopped when exiting the mode  */
+  APP_STATE_ACTIVE   = 2,
+} app_state_t;
+
 /** 
  * keep all the needed info together for launching an app 
  */
@@ -38,7 +47,7 @@ typedef struct list_elem
   char *name;		/* name of the app to launch */
   char *mode; 		/* mode in which to launch the app */
   char *launch;		/* dbus launch command/address */ 
-  int active;		/* marker to check if the app has started sucessfully */
+  app_state_t state;	/* marker to check if the app has started sucessfully */
   int systemd;		/* marker to know if we start it with systemd or not */
   int post;		/* marker to indicate when to start the app */
   /*@}*/

From 931f01a26f5623153cdcb55e6132b88a88054f09 Mon Sep 17 00:00:00 2001
From: Simo Piiroinen <simo.piiroinen@jollamobile.com>
Date: Thu, 7 Jul 2016 09:45:04 +0300
Subject: [PATCH 23/23] [appsync] Remove unneeded tag based enumeration trigger
 filtering

Since enumeration timers are not left running anymore when enumeration
occurs / appsync is canceled, the increment at activate_sync() and
check at enumerate_usb() based tag code filtering is no longer needed.

Remove the tag variables and code related to them.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
---
 src/usb_moded-appsync.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/src/usb_moded-appsync.c b/src/usb_moded-appsync.c
index 74fe118..bfb10b9 100644
--- a/src/usb_moded-appsync.c
+++ b/src/usb_moded-appsync.c
@@ -49,8 +49,6 @@ static void cancel_enumerate_usb_timer(void);
 static GList *sync_list = NULL;
 
 static guint enumerate_usb_id = 0;
-static unsigned sync_tag = 0;
-static unsigned enum_tag = 0;
 static struct timeval sync_tv;
 #ifdef APP_SYNC_DBUS
 static  int no_dbus = 0;
@@ -199,8 +197,8 @@ int activate_sync(const char *mode)
 
   log_debug("activate sync");
 
-  /* Bump tag, see enumerate_usb() */
-  ++sync_tag; gettimeofday(&sync_tv, 0);
+  /* Get start of activation timestamp */
+  gettimeofday(&sync_tv, 0);
 
   if( sync_list == 0 )
   {
@@ -422,30 +420,15 @@ static void enumerate_usb(void)
   /* Stop the timer in case of explicit enumeration call */
   cancel_enumerate_usb_timer();
 
-  /* We arrive here twice: when app sync is done
-   * and when the app sync timeout gets triggered.
-   * The tags are used to filter out these repeats.
-   */
-
-  if( enum_tag == sync_tag )
-  {
-    log_debug("ignoring enumeration trigger");
-  }
-  else
-  {
-
-    enum_tag = sync_tag;
-
-    /* Debug: how long it took from sync start to get here */
-    gettimeofday(&tv, 0);
-    timersub(&tv, &sync_tv, &tv);
-    log_debug("sync to enum: %.3f seconds", tv.tv_sec + tv.tv_usec * 1e-6);
+  /* Debug: how long it took from sync start to get here */
+  gettimeofday(&tv, 0);
+  timersub(&tv, &sync_tv, &tv);
+  log_debug("sync to enum: %.3f seconds", tv.tv_sec + tv.tv_usec * 1e-6);
 
 #ifdef APP_SYNC_DBUS
-    /* remove dbus service */
-    usb_moded_appsync_cleanup();
+  /* remove dbus service */
+  usb_moded_appsync_cleanup();
 #endif /* APP_SYNC_DBUS */
-  }
 }
 
 static void appsync_stop_apps(int post)