RE: [Patch v1 1/3] bugreport.c: replace strbuf_write_fd with write_in_full

From: "Randall S. Becker" <rsbecker@nexbridge.com>
To: "'Junio C Hamano'" <gitster@pobox.com>
Cc: "'Đoàn Trần Công Danh'" <congdanhqx@gmail.com>, git@vger.kernel.org
Subject: RE: [Patch v1 1/3] bugreport.c: replace strbuf_write_fd with write_in_full
Date: Fri, 19 Jun 2020 15:37:27 -0400	[thread overview]
Message-ID: <02c201d64671$13c9a840$3b5cf8c0$@nexbridge.com> (raw)
In-Reply-To: <xmqqimfmhmzs.fsf@gitster.c.googlers.com>

On June 19, 2020 3:31 PM, Junio C Hamano wrote:
> To: Randall S. Becker <rsbecker@nexbridge.com>
> Cc: 'Đoàn Trần Công Danh' <congdanhqx@gmail.com>;
> randall.s.becker@rogers.com; git@vger.kernel.org
> Subject: Re: [Patch v1 1/3] bugreport.c: replace strbuf_write_fd with
> write_in_full
> 
> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> 
> >> > +	if (write_in_full(report, buffer.buf, buffer.len) < 0) {
> >> > +		die(_("couldn't write report contents '%s' to file '%s'"),
> >> > +			buffer.buf, report_path.buf);
> >>
> >> Doesn't this dump the whole report to the stderr?
> >> If it's the case, the error would be very hard to grasp.
> >
> > Where else can we put the error? By this point, we're likely out of disk or
> virtual memory.
> >
> >> Nit: We wouldn't want the pair of {}.
> >>
> >> > +	}
> >> >  	close(report);
> >
> > I'm not sure what you mean in this nit? {} are balanced. You mean in the
> error message?
> 
> I think he means that a block that consists of a single statement (i.e. call to
> die()) does not have to be enclosed in {}.
> 
> (partial quote from Documentation/CodingGuidelines):
> 
>  - We avoid using braces unnecessarily.  I.e.
> 
> 	if (bla) {
> 		x = 1;
> 	}
> 
>    is frowned upon. But there are a few exceptions:

I get that. I was trying to maintain visual consistency with the rest of bugreport.c. Will redo it.