perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philippe M. Chiasson" <go...@ectoplasm.org>
Subject Re: Please help adding ModPerl::Interpreter
Date Thu, 25 Oct 2007 07:57:12 GMT
Torsten Foertsch wrote:
> On Thursday 18 October 2007, Philippe M. Chiasson wrote:
>>> The patch contains all my findings so far including the pnotes refcount
>>> problem. Pnotes now lock the interpreter like pools do.
>> Any chance you can break the patch into multiple patches, one for each
>> feature/fix? Ideally with an accompanying entry in Changes ? It'll be
>> simpler to merge these one at a time back to the trunk/
> 
> Is the test suite expected to succeed after each patch?

In general, yes, that's the idea.

 I can think of a few
> minor patches like pnotes, cleanuphandler, logging the pid with modperl_trace 
> plus one big chunk with the basic interpreter management. Otherwise it 
> doesn't make sense for me.

The current stream of patches is piling up (my bad), but they are much
easier to digest now (your good).

>>> There is a new ${r|c}->pnotes_kill function that can be used to
>>> prematurely delete pnotes.
>> Not sure about kill, how aobut:
>>
>> ->pnotes_reset() ?
>> ->pnotes_destroy() ?
> 
> It was named after apr_pool_cleanup_kill(). If you don't like it then what do 
> you prefer _destroy or _reset? To me it's all the same.

In that case, yes, pnotes_kill() probably is a bit more consistent.

Of course, after thinking about it, the more Perl-ish thing to do would
be to make this work:

undef $r->pnotes

Right now, you get a very nice error if you try that:

 "Can't modify non-lvalue subroutine call"

Oh, well ;-)

------------------------------------------------------------------------
Philippe M. Chiasson     GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/       m/gozer\@(apache|cpan|ectoplasm)\.org/


Mime
View raw message