Re: Lowercase or camelcase function names?

From: Date: Mon, 07 Nov 2011 21:00:06 +0000
Subject: Re: Lowercase or camelcase function names?
References: 1 2 3 4  Groups: php.pecl.dev 
Request: Send a blank email to pecl-dev+get-8819@lists.php.net to get a copy of this message
Hello Johannes, I have done some work on the extension today. It works and is cleaned up a lot. Your hints where evry helpful to me. I have commented your suggestions below. On Mon, 07 Nov 2011 01:16:24 +0100 Johannes Schlüter <johannes@schlueters.de> wrote: [...] > Some small comments, without really deep into it: > * pval isn't used anymore, I think it was even dropped with 5.3. > zval is the exact same. Fixed. No use of both of them anymore. > * The PARSE_ARGS and ARG macros use an old dangerous, bad API. > Look into using zend_parse_parameters (see > README.PARAMETER_PARSING in php src) > * The current code does ARG(0)->value.lval, this should be > Z_LVAL_P(ARG(0)), while that call is, at least in > scard_release_context, unsafe, zend_parse_parameters fixes that, > though) Both items fixed. I have changed the parameter handling code to use "zend_parse_parameters". Macros removed. > * Our CODING_STYLE asks for {} for every if clause etc. even when > it's just a single line, the opening { should be on the same > line as the "if" (or "while", "do", ..) Fixed. > * I assume there are some leaks, make sure to build PHP with > --enable-debug Done... 4 Leaks are still open... ;) > * The CREDITS file should be two lines, first line extension name, > second line author names. This is afaik used by the Windows > build. It would also be used to create phpcredits() if this > extension was to be bundled with PHP (which I don't expect to > happen) Done. > * build.shlooks weird, phpize && ./configure && make should do all > the things needed, else config.m4 is wrong. Will check that. Just "phpize && ./configure && make" fails... > * php_pcsc.h should be as sparse as possible, as for a static > build it is included from main/builtin_functions.h which causes > conflicts. Cleaned it up... Am I missing something? > * TODO: "Make the PCSC context a global var" - this sounds weird. > Global variables often cause issues. Don't know why I wanted it as a global variable when written this code last year... :/ Line removed... ;) > This long list might look worse than it is, the only first 3 items > "matter" :-) Yeah... I am on the right way... ;) Should I do an account request in such a early state of the extension? I really would like to provide it ASAP but I don't know if you like 0.1.0 versions at pecl.php.net. A good thing would be the use of PHP's version control to keep changes at a central place... I think the code is very stable and ready for use in non-production environments. The PC/SC API is not implented completely. I removed a lot of Johann Dantant's code to make it a little bit simpler. I am willing to implement the complete API but at the moment the current code can be used for server side smartcard development. I don't need more function at the moment... ;) I have needed this extension last year because many smart card terminals are using Secure Access Modules (SAM) for doing communication to the card on the client side. I have developed terminals for writing smartcards online an needed this extension for communication with PHP based web applications. Since JAVA, Python etc. does have well written implementations I thought, since PHP is widespread, it would be nice to do it for PHP as well... The updated code is available at: http://code.google.com/p/pcsc-for-php/ Regards, Johannes P.S.: I need to replace all my whitespaces with tabs... :/

« previous php.pecl.dev (#8819) next »