Marcus Boerger wrote:

I'd say the only thing that doesn't look ten times better than what we have 
right now is that you have exported
function prefixed with underscore "PHPAPI int _php_output" and static ones 
without. Imo it should be the other way
round.

This is because the TSRM macros don't have underscores, so one
uses php_output_flush() instead of _php_output_flush(TSRMLS_C)
of course.  So the public API except that used in main.c or
in SAPIs is TSRM macro free (like in the streams layer or
pecl/http and possibly else where).

The open question i have however is how do you deal with unicode. I saw some 
estr*() calls where i would have
expected a few unicode checks. So there is no unicode support right now. So i 
suggest you add that too.

I don't yet see why output control should know about or handle
unicode differently, it basically just shovels binary data around.
I didn't dive into the unicode stuff yet, so I might be quite off...

A think i am a bit curious yout is why some function have both *() and *_all() 
versions. Why didn't you choose to
pass an int in?

Because the *_all() functions don't honor if the handler is cleanable
and/or removable, those are ultimate functions like for shutting down
the output control layer.  I think it's better to have separate functions
to emphasize this difference.

Last but not least i wonder why i am seeing use of the strange 
TSRMLS_FETCH_FROM_CTX() instead of passing TSRM keys
using TSRMLS_* macros.

This is only used in zend_stack_apply callbacks which don't support
TSRM by signature.

Thanks,
--
Michael - <mike(@)php.net> http://dev.iworks.at/ext-http/http-functions.html.gz

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to