[alsa-devel] [PATCH] conf: don't check if config files are up to date

[Takashi Iwai]

On Sun, 01 May 2016 14:43:42 +0200,
Alexander E. Patrakov wrote:
> 
> Even if no files have been changed, the config may become outdated
> due to hot-plugged devices.
> 
> Note: this patch has a huge potential to crash badly-written applications
> that keep config pointers and strings for too long. But I see no other
> way to support hot-pluggable devices.

Can we opt in this feature, e.g. by enabling it via some new API
function?  Enabling this blindly appears too risky.


thanks,

Takashi

> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=54029
> Signed-off-by: Alexander E. Patrakov <patrakov at gmail.com>
> ---
>  src/conf.c     | 63 ++++++++++++++++++++++++----------------------------------
>  src/confmisc.c |  4 +++-
>  2 files changed, 29 insertions(+), 38 deletions(-)
> 
> diff --git a/src/conf.c b/src/conf.c
> index f8b7a66..34576aa 100644
> --- a/src/conf.c
> +++ b/src/conf.c
> @@ -3661,14 +3661,15 @@ SND_DLSYM_BUILD_VERSION(snd_config_hook_load_for_all_cards, SND_CONFIG_DLSYM_VER
>  #endif
>  
>  /** 
> - * \brief Updates a configuration tree by rereading the configuration files (if needed).
> + * \brief Updates a configuration tree by rereading the configuration files.
>   * \param[in,out] _top Address of the handle to the top-level node.
>   * \param[in,out] _update Address of a pointer to private update information.
>   * \param[in] cfgs A list of configuration file names, delimited with ':'.
>   *                 If \p cfgs is \c NULL, the default global
>   *                 configuration file is used.
> - * \return 0 if \a _top was up to date, 1 if the configuration files
> - *         have been reread, otherwise a negative error code.
> + * \return 0 if \a _top was up to date (this can happen only in previous
> + *                 ALSA-lib versions), 1 if the configuration files have been
> + *                 reread successfully, otherwise a negative error code.
>   *
>   * The variables pointed to by \a _top and \a _update can be initialized
>   * to \c NULL before the first call to this function.  The private
> @@ -3679,7 +3680,8 @@ SND_DLSYM_BUILD_VERSION(snd_config_hook_load_for_all_cards, SND_CONFIG_DLSYM_VER
>   * The global configuration files are specified in the environment variable
>   * \c ALSA_CONFIG_PATH.
>   *
> - * \warning If the configuration tree is reread, all string pointers and
> + * \warning In order to deal with hot-pluggable devices, the configuration
> + * tree is always reread, even if nothing changed. All string pointers and
>   * configuration node handles previously obtained from this tree become
>   * invalid.
>   *
> @@ -3754,35 +3756,6 @@ int snd_config_update_r(snd_config_t **_top, snd_config_update_t **_update, cons
>  			local->count--;
>  		}
>  	}
> -	if (!update)
> -		goto _reread;
> -	if (local->count != update->count)
> -		goto _reread;
> -	for (k = 0; k < local->count; ++k) {
> -		struct finfo *lf = &local->finfo[k];
> -		struct finfo *uf = &update->finfo[k];
> -		if (strcmp(lf->name, uf->name) != 0 ||
> -		    lf->dev != uf->dev ||
> -		    lf->ino != uf->ino ||
> -		    lf->mtime != uf->mtime)
> -			goto _reread;
> -	}
> -	err = 0;
> -
> - _end:
> -	if (err < 0) {
> -		if (top) {
> -			snd_config_delete(top);
> -			*_top = NULL;
> -		}
> -		if (update) {
> -			snd_config_update_free(update);
> -			*_update = NULL;
> -		}
> -	}
> -	if (local)
> -		snd_config_update_free(local);
> -	return err;
>  
>   _reread:
>   	*_top = NULL;
> @@ -3823,15 +3796,31 @@ int snd_config_update_r(snd_config_t **_top, snd_config_update_t **_update, cons
>  	*_top = top;
>  	*_update = local;
>  	return 1;
> +
> +_end:
> +	if (err < 0) {
> +		if (top) {
> +			snd_config_delete(top);
> +			*_top = NULL;
> +		}
> +		if (update) {
> +			snd_config_update_free(update);
> +			*_update = NULL;
> +		}
> +	}
> +	if (local)
> +		snd_config_update_free(local);
> +	return err;
>  }
>  
>  /** 
> - * \brief Updates #snd_config by rereading the global configuration files (if needed).
> - * \return 0 if #snd_config was up to date, 1 if #snd_config was
> - *         updated, otherwise a negative error code.
> + * \brief Updates #snd_config by rereading the global configuration files.
> + * \return 0 if #snd_config was up to date (this can happen only in previous
> + *         ALSA-lib versions), 1 if #snd_config was updated successfully,
> + *         otherwise a negative error code.
>   *
>   * \warning Whenever #snd_config is updated, all string pointers and
> - * configuration node handles previously obtained from it may become
> + * configuration node handles previously obtained from it become
>   * invalid.
>   *
>   * \par Errors:
> diff --git a/src/confmisc.c b/src/confmisc.c
> index ae0275f..8f97b72 100644
> --- a/src/confmisc.c
> +++ b/src/confmisc.c
> @@ -599,7 +599,9 @@ static int open_ctl(long card, snd_ctl_t **ctl)
>  	char name[16];
>  	snprintf(name, sizeof(name), "hw:%li", card);
>  	name[sizeof(name)-1] = '\0';
> -	return snd_ctl_open(ctl, name, 0);
> +
> +	/* Take care not to update the config - we may be running hooks on pcms right now */
> +	return snd_ctl_open_lconf(ctl, name, 0, snd_config);
>  }
>  
>  #if 0
> -- 
> 2.8.0
>