[vlc-devel] [PATCH] Improve FTP servers compatibility

[Rémi Denis-Courmont]

On Saturday 28 November 2015 11:47:29 Sylver Bruneau wrote:
> +    /* count number of elements in path */
> +    p_temp_path = (char *)calloc( 1, strlen( p_sys->url.psz_path ) + 1 );

Useless cast. Useless memset(). Missing error case.

> +    strcpy(p_temp_path, p_sys->url.psz_path);

strdup? But I don´t see why you need to copy the string at all (e.g. "%.*s").

> +    for (p_path_element = strtok(p_temp_path, "/");
> p_path_element;
> p_path_element = strtok(NULL, "/")) +    {
> +        i_element_count++;
> +    }

Not thread-safe.

> +
> +    /* go to root path (this is needed on some FTP servers) */
> +    if( ftp_SendCommand( p_this, p_sys, "CWD /") < 0 )
> +        goto error;
> +    else
> +    if( ftp_RecvCommand( p_this, p_sys, NULL, NULL ) != 2 )
> +    {
> +        msg_Err( p_this, "file or directory does not exist" );
> +        goto error;
> +    }
> +
> +    /* Perform a CWD for each path element and save file name for future
> use */
> +    strcpy(p_temp_path, p_sys->url.psz_path);
> +    p_path_element = strtok(p_temp_path, "/");

Ditto.

> +
> +    while (i_element_count > 0)
> +    {
> +        if (i_element_count == 1)
> +        {
> +            p_sys->p_filename = (char *)calloc( 1, strlen( (const char
> *)p_path_element ) + 1 );
> +            strcpy(p_sys->p_filename,
> p_path_element);

Same as above.

> +
> +        }
> +        else
> +        {
> +            if( ftp_SendCommand( p_this, p_sys, "CWD %s", p_path_element )
> < 0 ) +                goto error;
> +            else
> +            if( ftp_RecvCommand( p_this, p_sys, NULL, NULL ) != 2 )
> +            {
> +                msg_Err( p_this, "file or directory does not exist" );
> +                goto error;
> +            }
> +        }
> +        p_path_element = strtok(NULL, "/");

Same as above.

> +        i_element_count --;
> +    }
> +
>      /* get size */
>      if( p_sys->url.psz_path == NULL )
>          b_directory = true;
>      else
> -    if( ftp_SendCommand( p_this, p_sys, "SIZE %s", p_sys->url.psz_path ) <
> 0 ) +    if( ftp_SendCommand( p_this, p_sys, "SIZE %s", p_sys->p_filename )
> < 0 ) goto error;
>      else
>      if ( ftp_RecvCommand( p_this, p_sys, NULL, &psz_arg ) == 2 )
> @@ -658,15 +706,6 @@ static int InOpen( vlc_object_t *p_this )
>          msg_Dbg( p_access, "file size: %"PRIu64, p_sys->size );
>      }
>      else
> -    if( ftp_SendCommand( p_this, p_sys, "CWD %s", p_sys->url.psz_path ) < 0
> ) -        goto error;
> -    else
> -    if( ftp_RecvCommand( p_this, p_sys, NULL, NULL ) != 2 )
> -    {
> -        msg_Err( p_this, "file or directory does not exist" );
> -        goto error;
> -    }
> -    else
>          b_directory = true;
> 
>      if( b_directory )
> @@ -693,7 +732,9 @@ error:
> 
>  exit_error:
>      vlc_UrlClean( &p_sys->url );
> +    free( p_sys->p_filename );
>      free( p_sys );
> +    free( p_temp_path );
>      return VLC_EGENERIC;
>  }
> 
> @@ -740,6 +781,7 @@ static int OutOpen( vlc_object_t *p_this )
> 
>  exit_error:
>      vlc_UrlClean( &p_sys->url );
> +    free( p_sys->p_filename );
>      free( p_sys );
>      return VLC_EGENERIC;
>  }
> @@ -767,6 +809,7 @@ static void Close( vlc_object_t *p_access, access_sys_t
> *p_sys )
> 
>      /* free memory */
>      vlc_UrlClean( &p_sys->url );
> +    free( p_sys->p_filename );
>      free( p_sys );
>  }
> 
> @@ -1080,10 +1123,10 @@ static int ftp_StartStream( vlc_object_t *p_access,
> access_sys_t *p_sys, else
>      {
>          /* "1xx" message */
> -        assert( p_sys->url.psz_path );
> +        assert( p_sys->p_filename );
>          if( ftp_SendCommand( p_access, p_sys, "%s %s",
>                               p_sys->out ? "STOR" : "RETR",
> -                             p_sys->url.psz_path ) < 0
> +                             p_sys->p_filename ) < 0
> 
>           || ftp_RecvCommand( p_access, p_sys, &i_answer, NULL ) > 2 )
> 
>          {
>              msg_Err( p_access, "cannot retrieve file" );

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