[vlc-devel] [vlc-commits] config: assert that option exists

[Thomas Guillem]

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