This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v1.1][BZ #15362] Correctly return bytes written out in _IO_new_file_write


> 
> Right, again I overlooked the fact that it ought to behave as if it
> called fputc on each object.
> 

I also was just looking at the installed stdio headers, and bits/stdio.h
contains a macro that replaces calls to fwrite_unlocked() of constant 8 bytes or
less with inline calls to _IO_putc_unlocked().  So the following program will
actually print out something different depending or whether the line that
undefines fwrite_unlocked is commented out or not, since when it's uncommented,
the actual function call is used, which has a slightly different behavior in
this edge case.

  #include <stdio.h>
  #include <unistd.h>
  #include <string.h>
  #include <assert.h>
  
  static char stdio_buf[1024];
  
  /* #undef fwrite_unlocked */
  
  int main()
  {
  	size_t i;
  	size_t bytes_written;
  
  	setvbuf(stdout, stdio_buf, _IOLBF, sizeof(stdio_buf));
  
  	for (i = 0; i < sizeof(stdio_buf) - 6; i++)
  		fputc(' ', stdout);
  
  	close(STDOUT_FILENO);
  
  	bytes_written = fwrite_unlocked("123\n123\n", 1, 8, stdout);
  
  	fprintf(stderr, "Wrote %zu bytes (expected 3)\n",
  		bytes_written);
  	return bytes_written < 4 ? 0 : -1;
  }

This is obviously a fairly minor point as evidently no one has ever fixed this,
but I'm surprised, since it looks like most of this code has been around for 15
years and I hadn't expected to be finding bugs in it just now.

> 
> When the write fails, it sets ERR_SEEN.  Here, a zero or partial write
> is implied to be either an error or EOF, so the application is
> expected to call ferror or feof to check which one it is.
> 

I thought that the EOF condition (as tested through feof()) was only meaningful
for reading.

> 
> I'm not sure.  Shouldn't it just be limited to the last newline that
> gets flushed and hence not include the buffer up to the next newline?
> Everything after that, even if buffered, should be regarded as not
> written to be consistent of our return value of 0 when a single line
> failed to flush.
> 

I was thinking that fwrite() is supposed to accept as much as the provided data
as it can without violating the requested buffering semantics.  So if a single
line of length n including the newline fails to flush, but it still fits in the
buffer, the first n - 1 bytes should still be kept and that would be the return
value.  This may sound like it's abusing the buffer, but this would match what
would happen if fputc() were to be called n times as the C standard suggests:
the first n - 1 calls would succeed but the nth would fail, so the return value
would be n - 1 bytes written.

> 
> Yes for wfileops.c but no for oldfileops.c.  oldfileops.c is there to
> preserve binary compatibility for applications linked to glibc
> versions.  I believe the user here is old versions of libstdc++.
> 

Hmm, I'm confused by _IO_wfile_xsputn() though.  Like _IO_new_file_xsputn(),
there's a comment that claims it is an optimized implementation that uses
sys_write() directly when possible.  But the actual code doesn't appear to do
that, as it just copies some of the data into the buffer and calls
_IO_wdefault_xsputn(), which appears to also handle copying data into the
buffer...


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]