Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:68002
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.170 as permitted sender)
Message-ID: <51CD5580.50103@gmail.com>
Date: Fri, 28 Jun 2013 11:21:04 +0200
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:17.0) Gecko/20130509 Thunderbird/17.0.6
MIME-Version: 1.0
To: internals@lists.php.net
References: <CAGa2bXYNgjhUWTqzhorcYhd7SEyEvxdhCVWyXr8O-hsZ103E6g@mail.gmail.com> <CAGa2bXYBR-=rrifyhWSwB7M1u5J5EL_0o0DcUuFZ9DEuMCrjjQ@mail.gmail.com> <CAL+t6cGUJmfhVdSCf3XNn4Mri_fvq-Fuad40yrrKOd8c1uwxXw@mail.gmail.com>
In-Reply-To: <CAL+t6cGUJmfhVdSCf3XNn4Mri_fvq-Fuad40yrrKOd8c1uwxXw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Re: hex2bin: E_WARNING is too much for odd string?
From: leafnode@gmail.com (=?UTF-8?B?TGVzemVrIEtydXBpxYRza2k=?=)

On 2013-06-27 03:33, Sherif Ramadan wrote:
> On Wed, Jun 26, 2013 at 9:05 PM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>
>> Hi all,
>>
>> I've sent pull request for PHP-5.5 branch.
>> https://github.com/php/php-src/pull/369
>>
>> It's simple 1 liner removes E_WARNING for invalid length.
>>
>> Are there any objections?
>>
> Yes, I object to removing the error in the same breath we're arguing for
> consistency. 5.4.0 through 5.4.3 (no error), 5.4.4 through 5.4.16 (error),
> 5.5.0 (error), 5.5.1 (no error)??? What kind of consistency is this?
>
> I thought you wanted to add an extra error for malformed hex, which I would
> have been fine with, but removing the error entirely? The error is useful.
> It informs the user that they may have buggy code since the function is
> clearly documented to expect even length hex encoded strings.

In my opinion generating E_WARNING is too severe. Passing improper 
string to hex2bin does not necessary mean "buggy code". If the string is 
eg. from external input, why should I check if it's a well formed hex 
string if the function does this anyway? I think E_NOTICE + returning 
false is enough.

And about consistency... I don't know the rationale behind adding 
E_ERROR in recent versions, but if the situation is bad, it should be 
fixed, not proliferated to next versions. Yes, it'd better if the change 
was better thought, but the milk has spilled.

--Leszek