[vlc-devel] Re: Podcast improvement

[Antoine Cellerier]

Hello, 

I'm currently at work so i can't test your patch but i'll try to comment
it (At least the C related aspects).

On Tue, Mar 20, 2007, Vincent Maury wrote:
> +    static int Subscribe ( char *psz_url , vlc_object_t *p_this );
> +    static int Unsubscribe ( input_item_t *p_input , vlc_object_t *p_this );

We usualy pass p_this as the first argument.

You also might want to add some locking when calling Subscribe /
Unsubscribe.

> +    /* test of psz_url value */
> +    /* I don't know if a test function already exist ? */
> +    if ( strstr( psz_url , "http://" ) !== psz_url )
> +    {
> +        msg_Warn( p_sd, "Podcast url format not valid, subscribe canceled");
> +        return VLC_EGENERIC;
> +    }

AFAIK, !== doesn't exist in C.
And we shouldn't limit ourselves to podcasts published using HTTP. (at
least i can't see why we would)

> +    /* Add psz_url to vlcrc */
> +    char *psz_vlcrc = var_CreateGetString( p_sd, "podcast-urls" );
> +    int len_vlcrc = strlen( psz_vlcrc );
> +    int len_url = strlen( psz_url );


Please declare variables at the top of a code block (else BeOS people
won't be able to compile :p)

> +    if ( len_vlcrc == 0 )

Current space conventions in VLC are to not put a space between
if/while/for/... and the parenthesis.

> +        if ( p_sys->ppsz_urls[i][0] != 0 ) /* Does this work ??? */

It checks if the first char is non null (which means that the string
isn't empty).



Concerning the rebuild of the "urls" pipe seperated list, it might be a
good idea to use a seperate function to create it from scratch everytime
p_sys->ppsz_urls is changed. If the list needs to be updated, you can
just delete it and create it again. IMO, this will simplify the code.

-- 
Antoine Cellerier
dionoea

-- 
This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
To unsubscribe, please read http://developers.videolan.org/lists.html