On 18/03/2010 10:16, Lukas Kahwe Smith wrote:

On 18.03.2010, at 03:47, Stanley Sufficool wrote:

On Mon, Mar 15, 2010 at 8:45 AM, Christopher Jones
<christopher.jo...@oracle.com>  wrote:


Stanley Sufficool wrote:
I have attached patches for bug # 50755 on bugs.php.net. These also
cleanup to PDO DBLIB code to have less of a memory footprint and to
prepare for other feature additions such as multiple rowset support.

I have compiled and tested on x86.

Can someone review and provide feedback. Thank you.


Hi Stanley,

Persistence is good, but you might need to use another way to find
someone who has the skills, interest and time to review it.  Maybe ask
around on IRC - #php-dev-win on Freenode?

Tried IRC, no response. BTW, this is a Linux&&  Windows extension.

IMO, PDO should be a big focus to get stabilized. People are jumping
ship for poorer performing abstractions in PHP (ADO, MDB2, DBA,
etc...) Hell, even the PHP bug tracker uses MDB2. That's a bit of a
disgrace to a blessed and built in database abstraction extension.

I really want to contribute and have no problem with getting reigned
in with patch karma, but that will never happen if nobody cares to
review.


Yeah its a horrible chicken and egg situation we have with PDO. Too few people 
around, actually it seems too few to even review your patch when it comes to 
SQL server. Maybe Matteo has some time to take a peek. Especially in the 
beginning its very helpful to get some feedback on patches before getting 
direct commit access. Then again for PDO we might be at the stage where we 
might not even have the resources to do that :(

So, I tried to make some time and looked at the patch for 5.3. I have to say that I haven't tested it nor I'm an expert by any means of the sybase/mysql/freetds api.

Here are my findings:

1. dblib_driver.c and the README were changed so that the driver will always register as "dblib"

Seems like a pretty big behaviour change, moreover completely unrelated to the bug fix.


2. dblib_driver.c also contains whitespace changes, comments and dbsetopt vs DBSETOPT

Not sure how these are related to the bug fix.


3. lots of constants removed from php_pdo_dblib_int.h

Can you explain why? I guess they were used for compatibility purposes.


4. pdo_dblib_stmt_describe

this one looks suspicious:

+       col->name = (char*)dbcolname(H->link, colno+1);
+
+       if (!strlen(col->name)) {
+               spprintf(&tmp, 0, "computed%d", colno);
+               strlcpy(col->name, tmp, strlen(tmp)+1);
+               efree(tmp);

Aren't you overwriting memory that belongs to the library?


For now I couldn't find anything else to comment about, but I've had very little time.


Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to