Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:26639
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.162 as permitted sender)
Message-ID: <36.B6.01084.78D4C554@pb1.pair.com>
Date: Thu, 16 Nov 2006 13:37:44 +0200
To: Sara Golemon <pollita@php.net>
Cc: stas@zend.com,internals@lists.php.net
In-Reply-To: <455BEDA4.4090608@php.net>
References: <455BEDA4.4090608@php.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
Subject: Re: [PHP-DEV] fgets()/fgetss() BC break in HEAD
From: zeev@zend.com (Zeev Suraski)

At 06:48 16/11/2006, Sara Golemon wrote:
>Zeev-
>
>>My IQ is higher than 12, and I don't see how defensive coding could 
>>have defended against this BC break.  This code is missing error 
>>checking, but that could be quite reasonable (e.g. if you check 
>>ahead of time that the file is big enough to match the format 
>>you're expecting - so it's not perfect, but it's quite 
>>reasonable).  But even if it did have error checking, it would look 
>>something like this:
>You're ignoring the fact that fgets() is the wrong tool for this 
>job. This is something for fread().  "I want to read precisely X 
>bytes.  I know! I'll use a function which halts at a newline 
>character."  This isn't accidental reasoning of someone who "doesn't 
>have the time to read every page of the manual",  This is taking 
>antibiotics for a flu.

I'm not ignoring that fact at all (well I did in this particular 
email, but not beforehand).  I know it's designed to read full 
lines.  But do you really think it's not being used for reading a 
specific number of bytes as well?  Especially considering the fact 
that if you go to the documentation, fputs() is documented as an 
alias of fwrite() (IIRC).
Whether this is or isn't the right tool for the job is beside the 
point IMHO.  Fact is it *is* being used that way, and it will break 
apps if changed.

>>So, after the BC break, it'd barf.  There's really no way to 
>>protect against this BC break, and it's pretty clear this behavior 
>>is being relied upon.
>By one project. Which is very clearly using fgets() to do what 
>fgetc() should be doing.

That's just one project on Google Code, and Ilia's search was only 
for reading of two bytes IIRC.
Anyway, if there's one, there's plenty.  That's an axiom :)

>>>>Or change the docs and the variable name to something other than
>>>>maxchars is a perfect solution. :-)
>>>Yes, and that's what I said both in this thread and on IRC before 
>>>the thread started.
>>That's fine by me, but I think it's a different issue.  The BC 
>>break should be reverted irregardless...
>Just to be clear, noone's arguing against reversion.  I accepted 
>before this thread ever started that while the current logic is 
>broken, there's no driving force to fix it.

Yep I know.   We're just having fun discussing it aren't we :)

Zeev