[vlc-devel] [PATCH 1/8] Extensions: public include

[Jean-Philippe André]

Hi,

On Thursday 24 December 2009 10:04:39 Rémi Denis-Courmont wrote:
> Le mercredi 23 décembre 2009, Jean-Philippe André a écrit :
> assert() in header files has proven highly problematic.

I'll remove this.

> > +#define extension_UnsetDialogProvider(a) \
> > +        __extension_UnsetDialogProvider( VLC_OBJECT(a) )
> > +static inline void __extension_UnsetDialogProvider( vlc_object_t *p_this
> > ) +{
> > +    var_Destroy( p_this, "dialog-provider" );
> > +}
> 
> Seems unsafe. The caller does not know when the provider is
> unreferenced. Also, if the variable was created more than once,
> this won't even unregister the pointer.

Even if the "Set" function is meant to be called only once? Otherwise, we can 
set the variable to NULL, and then destroy it (looks strange).

By the way, an other idea would be to factorize this dialog provider with the 
dialog provider that calls "dialog_Register". We could maybe have a 
"dialog_GetProvider". 

> > +struct extension_dialog_t
> > +{
> > +    vlc_object_t *p_object;      ///< FIXME description
> > +
> > +    char *psz_title;             ///< Title for the Dialog (in
> > TitleBar)
> > +    int i_width;                 ///< Width hint in pixels
> > (may be discarded)
> > +    int i_height;                ///< Height hint
> > in pixels (may be discarded) +
> > +    int i_num_widgets;           ///< Number of widgets
> 
> Shouldn't those be unsigned?

Yes, they can be defined as unsigned, if that's better.

I'll be off next week, don't expect much answers from me before next year :)

Merry Xmas everybody!
Best regards,

-- 
Jean-Philippe André (jpeg)