2010/4/12 Johannes Schlüter <johan...@php.net>:
> Hi,
>
> On Mon, 2010-04-12 at 08:25 +0000, Pierre Joye wrote:
>> -     switch (ZEND_NUM_ARGS()) {
>> -     case 2:
>> -             if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rl", 
>> &arg1, &arg2) == FAILURE) {
>> -                     RETURN_FALSE;
>> -             }
>> -             break;
>> -     default:
>> -             WRONG_PARAM_COUNT;
>> -             /* NOTREACHED */
>> -             break;
>> +     if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rl", &arg1, 
>> &arg2) == FAILURE) {
>> +             RETURN_FALSE;
>>       }
>
> This is a small change to the behavior: With the old code it would have
> returned NULL on wrong param count and FALSE in case the parsing failed,
> with the new code it returns FALSE in both cases.
>
> I'd prefer NULL in both cases as that's how we react usually (see
> <http://php.net/functions.internal>)

Ah right, it must be NULL not FALSE. Bu the portion of the code I
removed is not necessary (old parsing api).

> Doubt it makes any difference in real life though.
>
>
> Other than that: No test case possible for this new function?

We have tests already for that. But to actually see that only the
required bytes are read using system's read would require to use
strace (or other tools) in the test. You can see that by reading a
couple of bytes from a file and run php through strace, 8k will be
read with read, while the script will still gets the requested bytes
only.

Can I merge it?

Cheers,
-- 
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Reply via email to