Always take the item lock when reading/writing configuration values

This was a bit sloppy. In some cases, we had the config fiel lock,
but that that does not protect against config_Put*(). In some cases,
the lock was only taken for strings but not float/integers.

diff --git a/src/config/core.c b/src/config/core.c
index 037b1c5baf5fd081aa29995e40157ee75ea03262..6e88b99322ade19aa8d40558c2ea556e32da2cfe 100644 (file)
--- a/src/config/core.c
+++ b/src/config/core.c
@@ -548,18 +548,22 @@ void __config_ResetAll( vlc_object_t *p_this )
 
         for (size_t i = 0; i < p_module->confsize; i++ )
         {
-            if (IsConfigIntegerType (p_module->p_config[i].i_type))
-                p_module->p_config[i].value.i = p_module->p_config[i].orig.i;
+            module_config_t *p_config = p_module->p_config + i;
+
+            vlc_mutex_lock (p_config->p_lock);
+            if (IsConfigIntegerType (p_config->i_type))
+                p_config->value.i = p_config->orig.i;
             else
-            if (IsConfigFloatType (p_module->p_config[i].i_type))
-                p_module->p_config[i].value.f = p_module->p_config[i].orig.f;
+            if (IsConfigFloatType (p_config->i_type))
+                p_config->value.f = p_config->orig.f;
             else
-            if (IsConfigStringType (p_module->p_config[i].i_type))
+            if (IsConfigStringType (p_config->i_type))
             {
-                free ((char *)p_module->p_config[i].value.psz);
-                p_module->p_config[i].value.psz =
-                        strdupnull (p_module->p_config[i].orig.psz);
+                free ((char *)p_config->value.psz);
+                p_config->value.psz =
+                        strdupnull (p_config->orig.psz);
             }
+            vlc_mutex_unlock (p_config->p_lock);
         }
     }
 

diff --git a/src/config/file.c b/src/config/file.c
index caf8a09dd2a1b46f08e9b9875f6d3c0a59592f48..a01dc3c6953b1d5139d5e48bfebad7ffa02ba5f8 100644 (file)
--- a/src/config/file.c
+++ b/src/config/file.c
@@ -258,6 +258,7 @@ int __config_LoadConfigFile( vlc_object_t *p_this, const char *psz_module_name )
             /* We found it */
             errno = 0;
 
+            vlc_mutex_lock( p_item->p_lock );
             switch( p_item->i_type )
             {
                 case CONFIG_ITEM_BOOL:
@@ -287,19 +288,15 @@ int __config_LoadConfigFile( vlc_object_t *p_this, const char *psz_module_name )
                     break;
 
                 default:
-                    vlc_mutex_lock( p_item->p_lock );
-
                     /* free old string */
                     free( (char*) p_item->value.psz );
                     free( (char*) p_item->saved.psz );
 
                     p_item->value.psz = convert (psz_option_value);
                     p_item->saved.psz = strdupnull (p_item->value.psz);
-
-                    vlc_mutex_unlock( p_item->p_lock );
                     break;
             }
-
+            vlc_mutex_unlock( p_item->p_lock );
             break;
         }
     }
@@ -577,15 +574,17 @@ static int SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name,
              p_item < p_end;
              p_item++ )
         {
-            /* Do not save the new value in the configuration file
-             * if doing an autosave, and the item is not an "autosaved" one. */
-            bool b_retain = b_autosave && !p_item->b_autosave;
-
             if ((p_item->i_type & CONFIG_HINT) /* ignore hint */
              || p_item->b_removed              /* ignore deprecated option */
              || p_item->b_unsaveable)          /* ignore volatile option */
                 continue;
 
+            vlc_mutex_lock (p_item->p_lock);
+
+            /* Do not save the new value in the configuration file
+             * if doing an autosave, and the item is not an "autosaved" one. */
+            bool b_retain = b_autosave && !p_item->b_autosave;
+
             if (IsConfigIntegerType (p_item->i_type))
             {
                 int val = b_retain ? p_item->saved.i : p_item->value.i;
@@ -652,6 +651,7 @@ static int SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name,
 
             if (!b_retain)
                 p_item->b_dirty = false;
+            vlc_mutex_unlock (p_item->p_lock);
         }
     }
 
@@ -705,14 +705,14 @@ int config_AutoSaveConfigFile( vlc_object_t *p_this )
 {
     libvlc_priv_t *priv = libvlc_priv (p_this->p_libvlc);
     size_t i_index;
-    bool done;
+    bool save = false;
 
     assert( p_this );
 
     /* Check if there's anything to save */
     vlc_mutex_lock( &priv->config_lock );
     module_t **list = module_list_get (NULL);
-    for( i_index = 0; list[i_index]; i_index++ )
+    for( i_index = 0; list[i_index] && !save; i_index++ )
     {
         module_t *p_parser = list[i_index];
         module_config_t *p_item, *p_end;
@@ -720,18 +720,18 @@ int config_AutoSaveConfigFile( vlc_object_t *p_this )
         if( !p_parser->i_config_items ) continue;
 
         for( p_item = p_parser->p_config, p_end = p_item + p_parser->confsize;
-             p_item < p_end;
+             p_item < p_end && !save;
              p_item++ )
         {
-            if( p_item->b_autosave && p_item->b_dirty ) break;
+            vlc_mutex_lock (p_item->p_lock);
+            save = p_item->b_autosave && p_item->b_dirty;
+            vlc_mutex_unlock (p_item->p_lock);
         }
-        if( p_item < p_end ) break;
     }
-    done = list[i_index] == NULL;
     module_list_free (list);
     vlc_mutex_unlock( &priv->config_lock );
 
-    return done ? VLC_SUCCESS : SaveConfigFile( p_this, NULL, true );
+    return save ? VLC_SUCCESS : SaveConfigFile( p_this, NULL, true );
 }
 
 int __config_SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name )