coding volunteers needed for msrpc server-side API conversion

Elrond Elrond at Wunder-Nett.org
Fri Jan 28 16:00:51 GMT 2000


On Fri, Jan 28, 2000 at 07:57:27AM +1100, Luke Kenneth Casson Leighton wrote:
> > > > Well, that sounds like spoolss isn't doing _any_ dynamic
> > > > memory, so there isn't really a conflict / problem here.
> > > 
> > > strictly speaking?   you're right.  the malloc / freeing is being done by
> > > the marshalling /. unmarshalling code.
> > > 
> > > however, jean-francois' implementation does (and ignores the unmarshalled
> > > BUFFER*)
> > 
> > So in the long run, that should be converted !?
> 
> yes.  probably to type-cast the already-malloc'd data to the appropriate
> structure, rather than internally malloc it and then copy it in!
> 
>  
> > > > All my arguments are for dynamic memory only. For the rest,
> > > > there's no need to dup() things.
> > > 
> > > the static-sized structures? correct.
> > > 
> > > even for the dynamic memory ones, it's not _really_ ok to dup() an list or
> > > array structure, when it could be a megabyte of contiguous memory.
> > 
> > Okay, that sounds right. Let me see, if I can modify my
> > "model" to do this:
> > 
> > In cli_*.c we do:
> > xxx_xxx(const foo in, foo **out)
> > 	xxx_make_q_xxx(in) /* this one "remembers" the pointers */
> > 	xxx_io_q_xxx() /* this one USEs the pointers */
> > 	...
> > 	xxx_io_r_xxx(&x) /* mallocs appropiate memory */
> > 	...
> > 	*out = x
> 
> yep.  looks good, even for dynamic memory and static memory cases.
> the CALLER of xxx_xxx(foo in , foo **out) is responsible for freeing *out
> (x).  see NetApiBufferFree() in ms code.  we really should be implementing
> this type of system.
> 
> > In srv_*.c we have:
> > dispatch_xxx_xxx()
> > 	xxx_io_q_xxx(&x) /* will malloc memory for x, as we're receiving */
> > 	...
> > 	_xxx_xxx(x, &y)  /* will USE x, malloc y */
> > 	free(x); /* we don't need it any more */
> > 	...
> > 		/* y is the answer */
> > 	...
> > 	xxx_make_r_xxx(y) /* will "remember" y */
> > 	xxx_io_r_xxx() /* will USE the y */
> > 	...
> > 	free(y);
> 
> > And in daemon/*_real.c
> > _xxx_xxx(const foo in, foo **out)
> > 	/* this function does real work */
> > 	/* will only USE in */
> > 	/* will malloc memory for out */
> > 	
> > 	*out = malloc();
> 
> yep!!!  you notice that if you cut out the marshalling  and unmarshalling
> bits, the xxx_xxx() memory alliocation and usage and the _xxx_xxx() memory
> allcation and usage are EXACTLY the same?

Yes, This was one of the goals, when I made this thing up.

> that's the way it _should_ be.

So, we do it _this_ way ?

It's just, I would like to have some _nice_ rules, and have
some. So I can start and rewrite things correctly.

I don't have problems, if some things don't follow the
rules, but new (or rewritten) things should follow them.

> > Since I haven't looked at the whole spoolss-code, I currently can't
> > tell, if it could fit in this model.
> 
> it should.
>  
> > But the main idea is now:
> > 
> > - xxx_io_xxx doesn't do _any_ free itself.
> 
> umm, well, i actually have freeing() in some of the code when it's used in
> server-side, so the logic obeys the Rules above, but it just doesn't
> _look_ like it obeys the rules.

So it eventually should be rewritten at some point to
completely obey the rules.

> the reason for this is that some of the free_xxx() routines are a little
> complex.

Hmm... Example?


> > - xxx_io_xxx mallocs, when it receives.
> > - make_xxx doesn't dup() anything.
> 
> correct.  these rules are currently broken by some of the code to
> make_q_xxx() being combined with mallocs() as outlined in Rules above.
> 
> what is worse is that make_q_xxx() sometimes takes char* or char** and
> consttcucts UNISTR* or UNISTR**, which is _Really_ bad!

Yes, I will try to take a look at these in lsa...
(hopefully, I find the time...)

> > - so xxx_io_xxx may not realloc when it sends (this is
> >   anyway good)
> 
> um... except for some of the [in out] parameters.

Hmm... They're realy in_out "on the wire" ??
How's that possible?

I think, we should get them to be real in or out
parameters. If that means, we have to pass around more
parameters, we should do it. Just to get things clean.

Until then, we can say "These are exceptions to our general
rules".


    Elrond


(Okay, I'm now on samba-technical)


More information about the samba-technical mailing list