[vlc-devel] [PATCH] playlist service discovery NULL pointer dereference

Pierre d'Herbemont pdherbemont at free.fr
Mon Aug 20 19:09:30 CEST 2007


Hello Alexander,

You are right, psz_cat can be NULL (and in fact is most of the time),  
however a good implementation of xxprintf usually check for NULL  
string pointers, and print "(null)" accordingly. That's why I wonder  
on what system are you?

I'll apply more or less the same patch, but we may want to catch the  
problem at a lower level, and provide our own safe implementation of  
vaprintf.

Could you be more specific about the other NULL pointers bad  
handling? Usually such crash are not hard to resolve, unless they  
hide a much bigger trouble.

Thanks for your patch and your interest,

Pierre.

On 20 août 07, at 17:48, Alexander Gall wrote:

>
> This code assumes that the "name" and "category" fields of a sd item
> are always set.  VLC crashes immediately if SAP is turned on in a
> network that has global multicast connectivity.  Apparently, most
> session descriptions sent to the global SAP group are missing the
> field that is interpreted as "category" (I didn't bother to track down
> which field that is) and some are also missing the one from which the
> "name" is derived.  I don't know whether those fields are mandatory or
> not in the session descriptions and it doesn't really matter.  VLC
> should just do something sensible instead of crashing.
>
> I'd like to make a general comment on the handling of strings in VLC.
> Many of the bugs I stumble upon when building VLC from the svn trunk
> are of this kind.  There must be loads and loads more of them.  Maybe
> it would be a good idea to practice some "defensive programming" and
> check those pointers before using them.  That would avoid many of the
> crashes (when a NULL pointer is not really a problem like in this
> case) or would at least lead to a crash with better diagnostics when
> an inconsistency is fatal.
>
> -- 
> Alex
>
> --- src/playlist/services_discovery.c.orig      2007-08-20  
> 17:22:38.342472000 +0200
> +++ src/playlist/services_discovery.c   2007-08-20  
> 17:15:18.885875000 +0200
> @@ -183,7 +183,9 @@
>      const char * psz_cat = p_event- 
> >u.services_discovery_item_added.psz_category;
>      playlist_item_t *p_new_item, * p_parent = user_data;
>
> -    msg_Dbg( p_parent->p_playlist, "Adding %s in %s", p_input- 
> >psz_name, psz_cat );
> +    msg_Dbg( p_parent->p_playlist, "Adding %s in %s",
> +            p_input->psz_name ? p_input->psz_name : "<UNKNOWN>",
> +            psz_cat ? psz_cat : "<UNKNOWN>");
>
>      /* If p_parent is in root category (this is clearly a hack)  
> and we have a cat */
>      if( !EMPTY_STR(psz_cat) &&
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel

_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
http://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list