Bug 164674 - [patch] [libc] vfprintf/vfwprintf return error (EOF) on success if __SERR flag is already set on file
Summary: [patch] [libc] vfprintf/vfwprintf return error (EOF) on success if __SERR fla...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-standards (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-01-31 23:10 UTC by Matthew Story
Modified: 2018-05-23 10:27 UTC (History)
0 users

See Also:


Attachments
file.diff (6.37 KB, patch)
2012-01-31 23:10 UTC, Matthew Story
no flags Details | Diff
freebsd.PR164674-better.patch.txt (4.77 KB, patch)
2012-04-02 21:05 UTC, Matthew Story
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Story 2012-01-31 23:10:09 UTC
The printf family of functions behaves unpredictably if the file passed to the function has the __SERR flag set in _flags.  The one consistency across all of the cases detailed below is that regardless of the number of bytes written to the file, and the success/failure of that operation, the printf functions (printf, fprintf, wprintf, fwprintf) will always return -1 (EOF).

 * For the case of an unbuffered file,  the operation currently fails transmitting no bytes, and returns -1.
 * For the case of a buffered file, the operation transmits all bytes and the function returns -1.  

The problem is that the behavior here is inconsistent, and should not be.   The question is wether all should be made to fail consistently and transmit no bytes if __SERR is set on _flags, or if failure should be determined based on the success of byte transmission, discounting file state.

Per the ISO/IEC 9899:201x (C11) Section 7.21.6.1, 14:

The fprintf function returns the number of characters transmitted, or a negative value if an output or encoding error occurred.

My reading of this specification is that success should be determined based on byte-transmission, and should not factor-in file state.  In addition to the ISO standard, the glibc implementation will reliably succeed with any error flag set if bytes are successfully transmitted (although it will transmit partial messages prior to successful conversion, which is unfortunate).

The attached patch makes the operation on buffered and unbuffered files consistent across the affected printf/wprintf functions, determines success/failure based on successful byte-transmission alone, and preserves _flags state for __SERR as passed in.

I originally discovered this behavior through black-box testing ``%<n>'' behavior against data-types through programmatic exhaustion, although a more likely cause of file error state is reading from a file open in write mode, prior to writing to that file (which is the example used in the repeat steps below).

Fix: See patch, my fix is to store and then unset the error flag inside of the exclusive lock, and then to reset the error flag to the original (or new) state after the vs(w)printf prior to releasing the exclusive lock.

this change maintains the ferror state of the file as passed in or as set by __vfprintf, but allows for vs(w)printf to determine success of the print operation independent of the state of the file as passed in.  there may be other ways to handle this more gracefully, but this change seems to me to be the safest and most minimally impacting solution.

The patch applies cleanly on both 8.2/9.0, and includes an additional regression test for stdio pertaining to correctness of return codes.

Patch attached with submission follows:
How-To-Repeat: /* this simple program demonstrates behavior on buffered (stdout) and unbuffered (stderr) files */
#include <stdio.h>
#define HELLO   "Hello, world\n"

int
main() {
        /* make sure everything is nice and clean */
        clearerr(stdout);
        /* read from write-only file, should cause an error */
        fgetc(stdout);
        printf("stdout is buffered, and %s an error\n",
                ferror(stdout) ? "has":"doesn't have");

        printf("bytes transmitted without replace: %d\n",
                printf(HELLO));
        printf("bytes transmitted with replace: %d\n",
                printf("%s", HELLO));

        clearerr(stderr);
        /* read from write-only file, should cause an error */
        fgetc(stderr);
        /*
         * print diagnostics to stdout, because some will not
         * print to stderr correctly if stderr has an error
         */
        printf("stderr is unbuffered, and %s an error\n",
                ferror(stderr) ? "has":"doesn't have");
        printf("bytes transmitted without replace: %d\n",
                fprintf(stderr, HELLO));
        printf("bytes transmitted with replace: %d\n",
                fprintf(stderr, "%s", HELLO));

        return 0;
}
Comment 1 Matthew Story 2012-01-31 23:16:22 UTC
On Tue, Jan 31, 2012 at 6:06 PM, Matthew Story <matthewstory@gmail.com>wrote:

> >Synopsis:       vsprintf/vswprintf return error (EOF) on success if
> __SERR flag is already set on file
>

this synopsis should read "vfprintf/vfwprintf return ...", apologies for
the typo, can someone with appropriate edit privileges change this?

thanks
Comment 2 Mark Linimon 2012-02-01 00:47:13 UTC
On Tue, Jan 31, 2012 at 06:16:22PM -0500, Matthew Story wrote:
> this synopsis should read "vfprintf/vfwprintf return ...", apologies for
> the typo, can someone with appropriate edit privileges change this?

done.

mcl
Comment 3 Matthew Story 2012-02-01 01:52:20 UTC
thanks!

On Tue, Jan 31, 2012 at 7:47 PM, Mark Linimon <linimon@lonesome.com> wrote:

> On Tue, Jan 31, 2012 at 06:16:22PM -0500, Matthew Story wrote:
> > this synopsis should read "vfprintf/vfwprintf return ...", apologies for
> > the typo, can someone with appropriate edit privileges change this?
>
> done.
>
> mcl
>



-- 
regards,
matt
Comment 4 Matthew Story 2012-02-16 15:55:11 UTC
On Tue, Jan 31, 2012 at 6:06 PM, Matthew Story <matthewstory@gmail.com>wrote:

>
> >Number:         164674
> >Category:       kern
> >Synopsis:       vfprintf/vfwprintf return error (EOF) on success if
> __SERR flag is already set on file
>

Apologies for cross-posting, but I think that standards might be a more
appropriate responsible party for this than -bugs or kern.  See description
for more info, but the basic issue is that C99 and C11 stipulate that
fprintf should return -1 "if an output or encoding error occurred."
Currently, printf is encoding and outputting successfully (on line or fully
buffered FILEs), but returning -1 if the FILE has an error.  The C99/C11
specifications make no mention of FILE state in fprintf return conditions,
so this functionality seems to not conform to the specification, attached
patch resolves that issue.


> >Confidential:   no
> >Severity:       non-critical
> >Priority:       low
> >Responsible:    freebsd-bugs
> >State:          open
> [...snip]
> >Description:
> The printf family of functions behaves unpredictably if the file passed to
> the function has the __SERR flag set in _flags.  The one consistency across
> all of the cases detailed below is that regardless of the number of bytes
> written to the file, and the success/failure of that operation, the printf
> functions (printf, fprintf, wprintf, fwprintf) will always return -1 (EOF).
>
>  * For the case of an unbuffered file,  the operation currently fails
> transmitting no bytes, and returns -1.
>  * For the case of a buffered file, the operation transmits all bytes and
> the function returns -1.
>
> The problem is that the behavior here is inconsistent, and should not be.
>   The question is wether all should be made to fail consistently and
> transmit no bytes if __SERR is set on _flags, or if failure should be
> determined based on the success of byte transmission, discounting file
> state.
>
> Per the ISO/IEC 9899:201x (C11) Section 7.21.6.1, 14:
>
> The fprintf function returns the number of characters transmitted, or a
> negative value if an output or encoding error occurred.
>
> My reading of this specification is that success should be determined
> based on byte-transmission, and should not factor-in file state.  In
> addition to the ISO standard, the glibc implementation will reliably
> succeed with any error flag set if bytes are successfully transmitted
> (although it will transmit partial messages prior to successful conversion,
> which is unfortunate).
>
> The attached patch makes the operation on buffered and unbuffered files
> consistent across the affected printf/wprintf functions, determines
> success/failure based on successful byte-transmission alone, and preserves
> _flags state for __SERR as passed in.
>
> [...snip]


-- 
regards,
matt
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2012-03-05 05:25:27 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-standards

over to responsible
Comment 6 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:03 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.