[vlc-devel] GSoC qualification tast WPL&ZPL playlist support

[Rémi Duraffort]

Le mardi 14 avril 2009 à 08:48:14, suheaven a écrit :
> Hi
> This patch removed dead functions and changed the copyright date of my file

Some small remarks.


=================
in modules/demux/playlist/wpl.c

+            psz_uri = ParseUriValue( psz_parse );
+            if( psz_uri && *psz_uri )
You can use EMPTY_STR if you want (exactly the same)


+            {
+                psz_uri = ProcessMRL( psz_uri, p_demux->p_sys->psz_prefix );
+                MaybeFromLocaleRep( &psz_uri );
+                p_input = input_item_NewExt( p_demux, psz_uri, psz_uri,
+                                        0, NULL, 0, -1 );
+                input_item_AddSubItem( p_current_input, p_input );
+                p_input = NULL;
+                free( psz_uri );
+            }
If psz_uri == "" you will leak memory.


+        }
+
+        free( psz_line );
+        psz_line = NULL;
Why psz_line = NULL ?

+
+        /* Fetch another line */
+        psz_line = stream_ReadLine( p_demux->s );


+static char* ParseUriValue(char* psz_string)
+{
+    int i_len = strlen( psz_string );
+    if(i_len <= 3 )
+        return NULL;
+    char* psz_value = (char *)malloc( i_len );
+    if( ! psz_value )
+        return NULL;
+    memset( psz_value, 0, i_len );
char* psz_value = calloc( i_len ); is equivalent.


=================
in modules/demux/playlist/zpl.c

+        /* One line starting with an URL is enough */
+        if( !strncasecmp( (const char *)p_peek, "http://", 7 ) ||
+            !strncasecmp( (const char *)p_peek, "mms://", 6 ) ||
+            !strncasecmp( (const char *)p_peek, "rtsp://", 7 ) ||
+            !strncasecmp( (const char *)p_peek, "https://", 8 ) ||
+            !strncasecmp( (const char *)p_peek, "ftp://", 6 ) )
+        {
And what about the other URL that vlc support ?

+        if( !strncasecmp( psz_parse, "NM", sizeof( "NM" ) - 1 ) )/*if the  line is "NM"*/
+        {
sizeof( "NM" ) -1 == strlen( "NM" )


+            psz_tabvalue = ParseTabValue( psz_parse );
+            if( psz_tabvalue && *psz_tabvalue )
again can use EMPTY_STR (really as you want)


+        if( psz_tabvalue )
+            free( psz_tabvalue );
No need to check for NULL before a free.

+        psz_tabvalue = NULL;
+        free( psz_line );
+        psz_line = NULL;
+
+        /* Fetch another line */
+        psz_line = stream_ReadLine( p_demux->s );
No need to put psz_line to NULL.

+
+        i_parsed_duration = 0;
no need to put it to 0.


+    char* psz_value = (char *)malloc( i_len );
+    if( ! psz_value )
+        return NULL;
+    memset( psz_value, 0, i_len );

calloc again.



Best regards.

-- 
Rémi Duraffort | ivoire