Re: svn commit: r317709 - head/usr.bin/csplit
On 05/03/17 09:56, Pedro Giffuni wrote:
...
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);
I can't find the above on the version in FreeBSD-current (or 11-stable).
contrib/libreadline/examples/rlcat.c: if (fputs (x, stdout) != 0)
contrib/libreadline/examples/rlcat.c-return 1;
This is an example so it's luckily not under use. Hopefully libreadline is
going away from base but it's still important to report this upstream
(will do).
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.
I went though the OpenOffice code, and it's dependencies, and I couldn't
find any
place where a similar issue happens. I did find several places where
there was a
correct comparison to EOF or >=0, but the vast majority of code just
call fputs()
without checking the return value.
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?
It seems like most code may have already adapted to the Apple variant.
Pedro.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r317709 - head/usr.bin/csplit
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.cTue May 2 21:33:27 2017 (r317708)
+++ head/usr.bin/csplit/csplit.cTue 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.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r317709 - head/usr.bin/csplit
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.cTue May 2 21:33:27 2017
(r317708)
+++ head/usr.bin/csplit/csplit.cTue 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.
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.
mdocml is undocumented in its on man page, since that man page is a link
to mandoc(1) nad doesn't contain the word mdocml.
Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r317709 - head/usr.bin/csplit
Very interesting ... On 2/5/2017 16:56, Jilles Tjoelker wrote: Author: jilles Date: Tue May 2 21:56:20 2017 New Revision: 317709 URL: https://svnweb.freebsd.org/changeset/base/317709 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. Actually r295631 explains better why the change was made. I now checked with opengrok and it appears only csplit was hit. Thanks! Pedro. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r317709 - head/usr.bin/csplit
Author: jilles
Date: Tue May 2 21:56:20 2017
New Revision: 317709
URL: https://svnweb.freebsd.org/changeset/base/317709
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.cTue May 2 21:33:27 2017
(r317708)
+++ head/usr.bin/csplit/csplit.cTue 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));
@@ -392,7 +392,7 @@ do_rexp(const char *expr)
/* Read and output lines until we get a match. */
first = 1;
while ((p = get_line()) != NULL) {
- if (fputs(p, ofp) != 0)
+ if (fputs(p, ofp) == EOF)
break;
if (!first && regexec(, p, 0, NULL, 0) == 0)
break;
@@ -453,7 +453,7 @@ do_lineno(const char *expr)
while (lineno + 1 != lastline) {
if ((p = get_line()) == NULL)
errx(1, "%ld: out of range", lastline);
- if (fputs(p, ofp) != 0)
+ if (fputs(p, ofp) == EOF)
break;
}
if (!sflag)
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
