On Tue, Oct 06, 2009 at 12:16:26PM -0600, tom fogal wrote:
> > Also, MANGLE was not and is not supported here on non-x86 platforms
> > because of glprocs.h.
> Hrm?  I might have recently broken our AIX support, then..
I must have been thinking something else.  It should not.  Sorry for the
confusion.
> > > > -        entry[i]->parameter_signature = str_dup(real_sig);
> > > > -        fill_in_entrypoint_offset(entry[i]->dispatch_stub, offset);
> > > > -        entry[i]->dispatch_offset = offset;
> > > > +         if (entry[i] == NULL) {
> > > > +            entry[i] = _glapi_add_dynamic_entry(function_names[i]);
> > > > +            if (entry[i] == NULL) {
> > > > +               /* FIXME: Possible memory leak here.
> > > > +               */
> > >
> > > What's leaked?  You've simply copied/maintained the existing
> > > comment; presumably you know what's going on here now and could
> > > expand on the comment.
> >
> > The error is that, there might already be some dynamic dispatches
> > generated when we bail out.  The generated dynamic dispatches are
> > useable.  It can be considered a leak or not.
> >
> > This patch is about refactoring.  I do not intend to introduce
> > functional changes if possible.  If I do, I will try to document it.
> 
> Sorry, I didn't mean to imply that you should fix the leak.  I did mean
> to imply that it would be nice if you could update the comment from
> `possible memory leak' to `we might be leaking any dispatches that were
> generated above by returning here w/o cleaning them up' (or similar).
I will update the comments.
> 
> > > Secondly, any chance of using stdlib's bsearch here?
> >
> > I just had a look at _mesa_bsearch.  It seems bsearch is not
> > universally available?  I think the inline version is also slightly
> > faster.
> I would bet a custom binary search would outperform any bsearch call,
> because there's always going to be a need for a function pointer in the
> latter.  My preference is still on reusing the existing infrastructure,
> until a demonstrated performance benefit justifies not doing so.
My concern was not true.  I am switching to bsearch().
> Anyway, regardless of which way some of these decisions go, this looks
> good to me / your responses have convinced me.  I've also applied it
> and got one of my dynamic loading test programs to work.
>   Signed-off-by: Tom Fogal <tfo...@alumni.unh.edu>
> FWIW.
> 
> -tom

-- 
Regards,
olv

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to