Re: Resource logging PHP extension

Hi, On Tue, 2010-09-07 at 02:10 +0200, Bostjan Skufca wrote: > Hi all! > > I have created a PHP extension for system resources usage logging. I find it > very useful for virtual hosting environment (gives insight in CPU/time usage > per vhost etc). > > I would like to share it with the world by including it in PECL. It is > tested with PHP versions 5.2.14 and 5.5.3. I use it in production without > any problems whatsoever. The code can be found here: > http://sourceforge.net/projects/rsclog/ I had a quick look over the code, some comments: 1) License headers missing By that it is not clear which license applies for every file. Please add them, as BSD/PHP License work file based a single Copying file is not enough. 2) strncpy You are using strncpy, we prefer usage of PHP's strlcpy which is safer, like adding checks to make sure the string is really NULL terminated. 3) /proc usage The README says this would work on Unix, I can find neither /proc/self/stat not /proc/$pid/stat on Solaris (but can find /proc/$pid/status, no idea if that helps) so that assumption in the README should be fixed, might also be good to add a test in the config.m4 for this. 4) C99/C++ comments Don't use // style comments in PHP, but always /* */ or comment out using #ifdef. 5) thread safety The extension depends on true globals to transport data, please look into TSRM to add isolation (... or at least prevent compiling this extension in thread-safe mode and document as such) 6) Tests Tests hould be written using PHP's phpt format so one can run them via make test, see http://qa.php.net/write-test.php > The "Requirements" website (http://pecl.php.net/account-request.php) states > I should discuss aforementioned inclusion on this list first, so here I am. > :) > What is the next step? Would you be willing to move over to PHP's svn server or do you want to keep on sourceforge's git? In general the extension might be acceptable for PECL. johannes