[MPlayer-G2-dev] Re: g2 config - restart...

[Alban Bedel]

Hi D Richard Felker III,

on Thu, 15 May 2003 04:22:09 -0400 you wrote:

> On Wed, May 14, 2003 at 10:52:14PM +0200, Alban Bedel wrote:
> > > btw i defined in my layer 0 draft, that you should make a static
> > > stuct, filled with the defaults. it also serevs as target of
> > > pointers in config_t, but that static instance is read-only, must
> > > not be modified.
> > 
> > That's why when discribing struct fields config_t->p should store the
> > offset of the field in the struct and not a normal pointer. The offset
> > is easy to get with such macro :
> > 
> > // From glib.h (modified ;-)
                    ^^^^^^^^
> > #define M_ST_OFF(struct_type, member)    \
> >     ((void*) &((struct_type*) 0)->member)
> > #define M_ST_MB_P(struct_p, struct_offset)   \
> >     ((void*) (struct_p) + (unsigned long) (struct_offset))
> > #define M_ST_MB(member_type, struct_p, struct_offset)   \
> >     (*(member_type*) M_ST_MB_P ((struct_p), (struct_offset)))
> > 
> > Then it's no prob. to set a specific field of the struct after having
> > allocated some mem for it.
> 
> LOL!!! These are from glib?? I figured someone who could write such a
> horrible library must not know C, but this is beyond even my
> expectations of their brain damage.... (Hint: learn the difference
> between pointer types and integer types!!)
Well these are not the original from glib. Sure that it would have been
better to cast the result of M_ST_OFF to some integer type. But as this
macro is used to set the field of config_t->p wich is void* i instead
casted it void* to avoid pages of warning. Or would it really different
to have ((void*)((unsigned long) &((struct_type*) 0)->member)) ?
I know that on some arch you can't store integer in pointer but here
it's a struct offset so uselly only a few bytes so it will fit in
both an integer and a void* in 99% of the case.
I saw your other post and i agree that your version is better.

> > > but you're maybe right, if defaulting everything to zero is ok, then
> > > we could left this out (optinally).
> > 
> > Imho the 'default struct' is needed bcs in many case you need default
> > values wich are not zero and there is also cases where zero is a fully
> > valid value but it shouldn't be the default (the evil case ;)
> > 
> > Until now imho there is no big problem with implementing such stuff.
> > The only problematic thing that i can see is the current main problem
> > of the 'new' config code in G1 : dynamicly allocated stuff vs. mem
> > leak vs. static values.
> > If we strictly deal with struct that are create by (properly) copying
> > the default stuct it's not a problem as we will only have dynamiclly
> > allocted stuff.
> > But if we also want to operate on 'normal' vars (we probably want)
> > then it's a problem.
> > In current G1 it's solved with a hack (when options are registered
> > in the context saving system those who use dynamicly allocted
> > mem (like string) have their initial value replaced by a copy of it
> > so there is then only dynamic stuff). If somebody know how one can
> > tell if a mem area was dynamiclly allocated or not then it's easy
> > to solve. Othewise i really can't think of smthg clean.
> 
> I say get rid of the hacks, and do it right. Since most of the things
> we'll be exporting config vars for can have multiple instances, it's
> nonsense to directly operate on variables that might be initialized
> with a constant string. Instead we should use a template for the
> defaults, and copy it into the dynamically allocated per-instance
> config structure.

Well this lead us to the question: do we allow options to operate on
vars or not ? For G2 libs it's not needed at all. But if we want
to make this useful for apps too, then operating on really vars make
sense.

	Albeu