[vlc-devel] [PATCH] vlc_input: fix memcpy to NULL or uninitialized memory and clean up code

[Rémi Denis-Courmont]

On Sunday 13 December 2015 00:38:05 Zhao Zhili wrote:
> From: Zhao Zhili <wantlamy at gmail.com>
> 
> ---
>  include/vlc_input.h | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/include/vlc_input.h b/include/vlc_input.h
> index f6209e3..bf890cc 100644
> --- a/include/vlc_input.h
> +++ b/include/vlc_input.h
> @@ -102,16 +102,7 @@ typedef struct input_title_t
> 
>  static inline input_title_t *vlc_input_title_New(void)
>  {
> -    input_title_t *t = (input_title_t*)malloc( sizeof( input_title_t ) );
> -    if( !t )
> -        return NULL;
> -
> -    t->psz_name = NULL;
> -    t->i_flags = 0;
> -    t->i_length = 0;
> -    t->i_seekpoint = 0;
> -    t->seekpoint = NULL;

I fail to see an improvement here. calloc()/memset() is not strictly warranted 
to set pointer to NULL. So this is just an uglier way to get the same result.

> -
> +    input_title_t *t = (input_title_t*)calloc( 1, sizeof( input_title_t )
> );
>      return t;
>  }
> 
> @@ -189,9 +180,13 @@ static inline input_attachment_t
> *vlc_input_attachment_New( const char *psz_name
>      a->psz_mime = strdup( psz_mime ? psz_mime : "" );
>      a->psz_description = strdup( psz_description ? psz_description : "" );
>      a->i_data = i_data;
> -    a->p_data = malloc( i_data );
> -    if( i_data > 0 && likely(p_data != NULL) )

Indeed, this was wrong as it checks p_data instead of a->p_data.
But you are not fixing it properly.

> -        memcpy( a->p_data, p_data, i_data );
> +    a->p_data = NULL;
> +    if( i_data > 0 && likely(p_data) )

Please don´t remove explicit NULL comparisons.

The compiler can optimize it if it wants to, so there is no point.

> +    {
> +        a->p_data = malloc( i_data );
> +        if( likely(a->p_data) )
> +            memcpy( a->p_data, p_data, i_data );
> +    }
> 
>      if( unlikely(a->psz_name == NULL || a->psz_mime == NULL
> 
>                || a->psz_description == NULL || (i_data > 0 && a->p_data ==
> 
> NULL)) )
> @@ -233,7 +228,7 @@ struct input_thread_t
>      bool b_preparsing;
>      bool b_dead VLC_DEPRECATED;
> 
> -    /* All other data is input_thread is PRIVATE. You can't access it
> +    /* All other data in input_thread_t is PRIVATE. You can't access it
>       * outside of src/input */
>      input_thread_private_t *p;
>  };

-- 
Rémi Denis-Courmont
http://www.remlab.net/