On June 19, 2020 3:31 PM, Junio C Hamano wrote: > To: Randall S. Becker <rsbecker@xxxxxxxxxxxxx> > Cc: 'Đoàn Trần Công Danh' <congdanhqx@xxxxxxxxx>; > randall.s.becker@xxxxxxxxxx; git@xxxxxxxxxxxxxxx > Subject: Re: [Patch v1 1/3] bugreport.c: replace strbuf_write_fd with > write_in_full > > "Randall S. Becker" <rsbecker@xxxxxxxxxxxxx> 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.