[vlc-devel] [vlc-commits] config: assert that option exists
Thomas Guillem
thomas at gllm.fr
Fri Mar 2 13:17:46 CET 2018
Hello,
This commit causes a regression. It breaks var_Inherit() on a var that is not on config.
Example: "the chain-level" variable from the chain filter.
Several ways to fix it:
1/ Check var_Type() from var_Inherit() when doing a fallback on config.
2/ var_Inherit() must be called with a variable that is on the config, then we fix the chain.c filter to do a var_Get() on the parent.
I really prefer the 1st solution.
Regards,
On Wed, Feb 28, 2018, at 19:12, Rémi Denis-Courmont wrote:
> vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Wed Feb
> 28 19:04:17 2018 +0200| [0f69e188d4c7c130db30707c9fd3b70c0bc10fa2] |
> committer: Rémi Denis-Courmont
>
> config: assert that option exists
>
> We already assert that the type is correct. Only one call site actually
> checks for the error return value, and even this, it is debatable
> (that is var_Create(VLC_VAR_BOOL)). So an assertion makes more sense
> that printing an error.
>
> In some cases (e.g. Lua), we use config_GetType() ahead to check for
> errors though - that still works fine.
>
> > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=0f69e188d4c7c130db30707c9fd3b70c0bc10fa2
> ---
>
> src/config/core.c | 42 ++++++------------------------------------
> 1 file changed, 6 insertions(+), 36 deletions(-)
>
> diff --git a/src/config/core.c b/src/config/core.c
> index 6b4f6d97dc..255f3c7ab8 100644
> --- a/src/config/core.c
> +++ b/src/config/core.c
> @@ -83,12 +83,7 @@ int64_t config_GetInt( vlc_object_t *p_this, const
> char *psz_name )
> module_config_t *p_config = config_FindConfig( psz_name );
>
> /* sanity checks */
> - if( !p_config )
> - {
> - msg_Err( p_this, "option %s does not exist", psz_name );
> - return -1;
> - }
> -
> + assert(p_config != NULL);
> assert(IsConfigIntegerType(p_config->i_type));
>
> int64_t val;
> @@ -107,12 +102,7 @@ float config_GetFloat( vlc_object_t *p_this, const
> char *psz_name )
> p_config = config_FindConfig( psz_name );
>
> /* sanity checks */
> - if( !p_config )
> - {
> - msg_Err( p_this, "option %s does not exist", psz_name );
> - return -1;
> - }
> -
> + assert(p_config != NULL);
> assert(IsConfigFloatType(p_config->i_type));
>
> float val;
> @@ -131,12 +121,7 @@ char * config_GetPsz( vlc_object_t *p_this, const
> char *psz_name )
> p_config = config_FindConfig( psz_name );
>
> /* sanity checks */
> - if( !p_config )
> - {
> - msg_Err( p_this, "option %s does not exist", psz_name );
> - return NULL;
> - }
> -
> + assert(p_config != NULL);
> assert(IsConfigStringType (p_config->i_type));
>
> /* return a copy of the string */
> @@ -155,12 +140,7 @@ void config_PutPsz( vlc_object_t *p_this,
>
>
> /* sanity checks */
> - if( !p_config )
> - {
> - msg_Warn( p_this, "option %s does not exist", psz_name );
> - return;
> - }
> -
> + assert(p_config != NULL);
> assert(IsConfigStringType(p_config->i_type));
>
> char *str, *oldstr;
> @@ -185,12 +165,7 @@ void config_PutInt( vlc_object_t *p_this, const
> char *psz_name,
> module_config_t *p_config = config_FindConfig( psz_name );
>
> /* sanity checks */
> - if( !p_config )
> - {
> - msg_Warn( p_this, "option %s does not exist", psz_name );
> - return;
> - }
> -
> + assert(p_config != NULL);
> assert(IsConfigIntegerType(p_config->i_type));
>
> if (i_value < p_config->min.i)
> @@ -211,12 +186,7 @@ void config_PutFloat( vlc_object_t *p_this,
> module_config_t *p_config = config_FindConfig( psz_name );
>
> /* sanity checks */
> - if( !p_config )
> - {
> - msg_Warn( p_this, "option %s does not exist", psz_name );
> - return;
> - }
> -
> + assert(p_config != NULL);
> assert(IsConfigFloatType(p_config->i_type));
>
> /* if f_min == f_max == 0, then do not use them */
>
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits
More information about the vlc-devel
mailing list