Date: Wed, 3 May 2017 09:56:49 -0500 From: Pedro Giffuni <pfg@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au>, Jilles Tjoelker <jilles@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r317709 - head/usr.bin/csplit Message-ID: <2c7bd200-957d-2c36-076e-41d4d04a2956@FreeBSD.org> In-Reply-To: <20170503162918.U1062@besplex.bde.org> References: <201705022156.v42LuKbp088899@repo.freebsd.org> <20170503162918.U1062@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 3/5/2017 01:55, Bruce Evans wrote:
> On Tue, 2 May 2017, Jilles Tjoelker wrote:
>
>> Log:
>> csplit: Fix check of fputs() return value, making csplit work again.
>>
>> As of r295638, fputs() returns the number of bytes written (if not
>> more than
>> INT_MAX). This broke csplit completely, since csplit assumed only
>> success
>> only for the return value 0.
>>
>> PR: 213510
>> Submitted by: J.R. Oldroyd
>> MFC after: 1 week
>> Relnotes: yes
>>
>> Modified:
>> head/usr.bin/csplit/csplit.c
>>
>> Modified: head/usr.bin/csplit/csplit.c
>> ==============================================================================
>>
>> --- head/usr.bin/csplit/csplit.c Tue May 2 21:33:27 2017 (r317708)
>> +++ head/usr.bin/csplit/csplit.c Tue May 2 21:56:20 2017 (r317709)
>> @@ -195,7 +195,7 @@ main(int argc, char *argv[])
>> /* Copy the rest into a new file. */
>> if (!feof(infile)) {
>> ofp = newfile();
>> - while ((p = get_line()) != NULL && fputs(p, ofp) == 0)
>> + while ((p = get_line()) != NULL && fputs(p, ofp) != EOF)
>> ;
>> if (!sflag)
>> printf("%jd\n", (intmax_t)ftello(ofp));
>
> I don't like checking for the specific value EOF instead of any negative
> value, though the EOF is Standard and I like checking for specific -1
> for sysctls. stdio is not very consistent, and this bug is due to old
> versions of FreeBSD documenting and returning the specific value 0 on
> non-error, which was also Standard.
>
The standard says non-negative, expecting zero to be the only
non-negative value is a bug.
The idea was mostly to match the MacOS behavior. MacOS buildbots are
expensive and
some projects find it's useful to have a FreeBSD builbot to have some
idea when non-portable
behavior is introduced.
> Grepping for fputs in /usr/src shows too many instances to check (mostly
> without any error handling). The simplest filter 'if (fputs' found the
> dependency on the old FreeBSD behaviour in csplit and 2 other places:
>
> contrib/mdocml/main.c: if (fputs(cp, stdout)) {
> contrib/mdocml/main.c- fclose(stream);
> contrib/libreadline/examples/rlcat.c: if (fputs (x, stdout) != 0)
> contrib/libreadline/examples/rlcat.c- return 1;
>
> More complicated filters like 'if ([^(]]*[^a-z_]fputs' failed to find
> any problems since I messed up the regexp.
>
I admittedly ignored contrib, plus I only skimmed for comparisons to zero.
Now I am worried: the classic BSD implementation is ubiquitous and this bug
is not easy to find, particularly in ports.
Do you guys think it we should revert to the previous behavior, at least
for 11.1?
The bugs should be reported upstream but it is not really our battle to
fix the world
for Apple is it?
Pedro.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2c7bd200-957d-2c36-076e-41d4d04a2956>