svn commit: r317709 - head/usr.bin/csplit

[Pedro Giffuni]

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.