Re: Intro and new extension: jsmin

Hi, On Mon, 2012-09-17 at 09:32 -0700, Michael Squires wrote: > I would like to release an extension via PECL for minifying javascript > using Douglas Crockford's jsmin.c. > > Current repo: https://github.com/sqmk/pecl-jsmin This might be useful. I had a quick look and some tiny(!) notes on the code: 1. the PHP Coding standard (see php-src/CODING_STANDARD) suggests to always use curly braces, even with a one line if. No big issue but, well ;-) 2. if (!javascript_len) RETURN_NULL(); -- i would consider an empty string to be "valid" input, and would expect the empty string to be the proper result then, not NULL 3. in jsmin() your creating a copy (minified = estrndup(jmo->buffer->c, jmo->buffer->len);) of a buffer and then in PHP_FUNCTION(jsmin) you are copying it, again while not freeing the intermediate copy. changing the 1 in the ZVAL_STRING() call to a 0 should avoid the copy (also build your PHP using --enable-debug, then this should emit a note) 4. using RETURN_STRING() instead of ZVAL_STRING(return_value) is a it nicer to read ;-) 5. in jsmin() you know the length (jmo->buffer->len),you should use that to fill the return_value inside the PHP_FUNCTION (either return the length as additional parameter via pointer, or pass the return_value as zval* from the PHP_FUNCTION to jsmin()) Looks like a relatively long list, but in fact are all minor things. No worries :-) johannes