Re: Extension proposition: proctitle

Hi, On Fri, 2009-01-23 at 07:25 +0100, Mark Karpeles wrote: > I've merged the initial code and the code posted on the bug report, > while trying to keep BSD compatibility (I'll need to change config.m4 to > make it compatible, that's planned before any release) and posted the > whole thing to a public svn: > > http://ookoo.org/svn/proctitle/ A few minor comments, but in general a +1: a) instead of symbol = (sapi_module_struct *)dlsym(NULL, "sapi_module"); won't a simple symbol = &sapi_module; do? sapi_module is exported as a global from SAPI.h. b) Please add a "proto" line and a short decription to the function, this helps when looking around and I think the docteam has some scripts looking for them for documentation purpose c) if(zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s",&title, &tlen) == FAILURE) RETURN_FALSE; two minor coding style things there: - We prefer to alway use braces {} - Usually we return NULL if parameter parsing fails. As said all minor cosmetic stuff. johannes