RE: [Possible Bug] Use of write on size-limited platforms

["Randall S. Becker" <rsbecker@nexbridge.com>]

On June 15, 2020 6:00 PM, Jeff King wrote:
> On Mon, Jun 08, 2020 at 05:41:34PM -0400, Randall S. Becker wrote:
> > I just wanted to check the following calls to make sure that it does
> > not fwrite or write should be xread/xwrite or are guaranteed not to
> > exceed
> > MAX_IO_SIZE:
> >
> > strbuf.c: strbuf_write, strbuf_write_fd, the size is not specified.
> >
> > The other uses of read/write appear to be safe.
> 
> strbuf_write() is using fwrite(), and we don't enforce MAX_IO_SIZE with stdio
> anywhere else. And I'd expect in general that if there are any platform
> limitations, the system libc would choose a sane value anyway.
> So that one is probably fine.
> 
> I think strbuf_write_fd() is wrong to use a raw write(), but for several
> reasons:
> 
>  - it won't enforce MAX_IO_SIZE, as you note
> 
>  - it won't handle EINTR, etc; callers need to be prepared to restart
>    such a write
> 
>  - it won't handle a partial write by looping until all output is sent
> 
> For the latter two, there are cases where some callers want the flexibility to
> stop when seeing a signal or a partial write. But I don't think that makes any
> sense for strbuf_write_fd(). If I pass in a strbuf with 8kb of data and I get a
> return value that indicates we only wrote 4kb, what do I do next? I certainly
> can't call strbuf_write_fd() again, since it would write from the beginning of
> the strbuf again. I'd have to call xwrite() myself after that. At which point I
> may as well have done so for the first call. :)
> 
> So I think this really ought to be using write_in_full(). There's only one caller,
> and I think it would be improved by the switch. Do you want to write a
> patch?
> 
> You could make an argument that the fwrite() version ought to also loop,
> since it's possible to get a partial write there, too. But we don't do that in
> general. I suspect in practice most stdio implementations will keep writing
> until they see an error, and most callers don't bother checking stdio errors at
> all, or use ferror().

I'll give the patch a go. It is very simple. Would you suggest removing the strbuf_write_fd() as part of this patch since it only impacts bugreport.c?

Regards,
Randall