[MPlayer-dev-eng] [PATCH] m_property.c - please give feedback

[Chris Welton]

Thank you for the feedback, I will try to take that
into account...

Chris

--- Reimar Döffinger
<Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:

> Hello,
> On Sat, Nov 24, 2007 at 01:25:09AM -0800, Chris
> Welton wrote:
> 
> > Index: m_property.c
> >
>
===================================================================
> > --- m_property.c	(revision 25155)
> > +++ m_property.c	(working copy)
> > @@ -20,7 +20,9 @@
> >  static int do_action(m_option_t* prop_list, const
> char* name,
> >                       int action, void* arg, void
> *ctx) {
> >      const char* sep;
> > -    m_option_t* prop;
> > +    const m_option_t* prop;
> > +    const m_option_t** arg_handle;
> > +    m_property_ctrl_f ctrl_f;
> >      m_property_action_t ka;
> >      int r;
> >      if((sep = strchr(name,'/')) && sep[1]) {
> > @@ -37,10 +39,12 @@
> >      } else
> >          prop = m_option_list_find(prop_list,
> name);
> >      if(!prop) return M_PROPERTY_UNKNOWN;
> > -    r =
> ((m_property_ctrl_f)prop->p)(prop,action,arg,ctx);
> > +    ctrl_f = prop->p;
> > +    r = ctrl_f((void *)prop,action,arg,ctx);
> 
> I do not see any advantages in using an extra ctrl_f
> variable.
> In addition that cast to void * is not good, since
> it casts a const
> away. There are only two ways to fix warnings: fix
> the code _properly_
> (and casting away const is never proper, changing
> m_property_ctrl_f
> instead probably would work fine here) or disable
> the warning.
> Otherwise changing m_option_t* prop; to const is
> correct.
> 
> >      if(action == M_PROPERTY_GET_TYPE && r < 0) {
> >         if(!arg) return M_PROPERTY_ERROR;
> > -        *(m_option_t**)arg = prop;
> > +        arg_handle = arg;
> > +        *arg_handle = prop;
> 
> That only obfuscates the code. Also there is no
> point in defining
> arg_handle at the top of the function if it is only
> used in this block
> 
> Greetings,
> Reimar Döffinger
> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
>
http://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
> 



      ____________________________________________________________________________________
Be a better sports nut!  Let your teams follow you 
with Yahoo Mobile. Try it now.  http://mobile.yahoo.com/sports;_ylt=At9_qDKvtAbMuh1G1SQtBI7ntAcJ