Hi,
On Mon, 2009-11-30 at 19:02 +0530, Arvind Srinivasan wrote:
> (1) If the number of occurrences of the API to be modified are
> relatively small, then make the change directly.
> e.g.
> -int fcgi_accept_request(fcgi_request *req);
> -int fcgi_finish_request(fcgi_request *req, int force_close);
> +int fcgi_accept_request(fcgi_request *req TSRMLS_DC);
> +int fcgi_finish_request(fcgi_request *req, int force_close TSRMLS_DC);
right
> (2) If the number of occurrences is large, then rename the function
> (prefix with _) and add a macro that passes TSRMLS_C
> e.g.
> -ZEND_API void zend_hash_destroy(HashTable *ht);
> +ZEND_API void _zend_hash_destroy(HashTable *ht TSRMLS_DC);
> +#define zend_hash_destroy(ht) _zend_hash_destroy((ht) TSRMLS_CC)
I'm not too happy about hiding the TSRM macro with another macro as it
makes reading the code harder. When writing code I want to spot easily
whether my function needs TSRM or not.
When adding such a macro that should be done for making the code better
readable, not worse.
> (3) For functions that have varargs, pass TSRMLS_C as the last but one
> parameter
> e.g.
> -ZEND_API int zend_get_parameters(int ht, int param_count, ...);
> +ZEND_API int zend_get_parameters(int ht TSRMLS_DC, int param_count, ...);
right.
> (4) For single argument functions that have varags, pass TSRMLS_C as
> the first parameter
> e.g.
> -ZEND_API int zend_get_parameters_ex(int param_count, ...) /* {{{ */
> +ZEND_API int zend_get_parameters_ex(TSRMLS_DC1 int param_count, ...) /* {{{
> */
>
> where
> +#define TSRMLS_DC1 TSRMLS_D,
> +#define TSRMLS_CC1 TSRMLS_C,
Are there more cases of this? - If zend_get_parameters is the only case
I'd keep the TSRMLS_FETCH in it. zend_get_parameters is deprecated and
shouldn't be used anymore. But adding a new macro makes it harder for
new people to get started with the code.
johannes
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
