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-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-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-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-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-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-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-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-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-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"