Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:47430
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain beccati.com designates 213.92.90.60 as permitted sender)
Message-ID: <4BA3A13F.8030002@beccati.com>
Date: Fri, 19 Mar 2010 17:07:27 +0100
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; it; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3
MIME-Version: 1.0
To: Stanley Sufficool <ssufficool@gmail.com>
CC: Lukas Kahwe Smith <mls@pooteeweet.org>, 
 Christopher Jones <christopher.jones@oracle.com>,
 PHP Developers Mailing List <internals@lists.php.net>
References: <ca02946e1003141307qbca014frb7f9da2f7a23dda5@mail.gmail.com> <4B9E5634.1040508@oracle.com> <ca02946e1003171947r2ff25aa9lf398ff8a21d7b56c@mail.gmail.com> <2A17C50C-E2E6-438F-A47B-25F5E83DE6AA@pooteeweet.org>
In-Reply-To: <2A17C50C-E2E6-438F-A47B-25F5E83DE6AA@pooteeweet.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Bug # 50755
From: php@beccati.com (Matteo Beccati)

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.jones@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/