"Randall S. Becker" <rsbecker@xxxxxxxxxxxxx> writes:
> On June 16, 2020 4:02 AM, Peff wrote:
>> To: Randall S. Becker <rsbecker@xxxxxxxxxxxxx>
>> Cc: git@xxxxxxxxxxxxxxx
>> Subject: Re: [Possible Bug] Use of write on size-limited platforms
>>
>> On Mon, Jun 15, 2020 at 06:38:34PM -0400, Randall S. Becker wrote:
>>
>> > > 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?
>>
>> I could go either way on that. IMHO it isn't offering much over a bare
>> write_in_full() call. The "don't call write() if there are 0 bytes"
>> logic is part of write_in_full() already.
>
> Patch delivered.
I think these go in the right direction and would be good with minor
fixes. I do agree with the other reviewer that dumping all the
contents of the buffer to die() may not be what we want (do we even
need to see the contents at all?). Even though I do not mind too
much about the unneeded {braces}, I'd prefer to see it fixed if we
are rerolling the patch for other reason anyway. And I think 2 and
3 can and should be a single patch---if we are removing a function
that is no longer used, we should remove both its decl and defn at
the same time.
Thanks.
