From: Jeff Hostetler Date: Fri, 05 Feb 2021 18:30:53 GMT Subject: Re: [PATCH v2 04/14] pkt-line: optionally skip the flush packet in write_packetized_from_buf() Message-ID: <352af03f-916f-9104-d2d7-ed7457d37911@jeffhostetler.com> In-Reply-To: On 2/2/21 4:48 AM, Jeff King wrote: > On Mon, Feb 01, 2021 at 07:45:37PM +0000, Johannes Schindelin via GitGitGadget wrote: > >> From: Johannes Schindelin >> >> This function currently has only one caller: `apply_multi_file_filter()` >> in `convert.c`. That caller wants a flush packet to be written after >> writing the payload. >> >> However, we are about to introduce a user that wants to write many >> packets before a final flush packet, so let's extend this function to >> prepare for that scenario. > > I think this is a sign that the function is not very well-designed in > the first place. It seems like the code would be easier to understand > overall if that caller just explicitly did the flush itself. It even > already does so in other cases! > I agree. I'll move flush to the caller and rename the write packetized function slightly to guard against new callers assuming the old behavior during the transition. Jeff > Something like (untested): > > convert.c | 4 ++++ > pkt-line.c | 4 ---- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/convert.c b/convert.c > index ee360c2f07..3968ac37b9 100644 > --- a/convert.c > +++ b/convert.c > @@ -890,6 +890,10 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len > if (err) > goto done; > > + err = packet_flush_gently(process->in); > + if (err) > + goto done; > + > err = subprocess_read_status(process->out, &filter_status); > if (err) > goto done; > diff --git a/pkt-line.c b/pkt-line.c > index d633005ef7..014520a9c2 100644 > --- a/pkt-line.c > +++ b/pkt-line.c > @@ -256,8 +256,6 @@ int write_packetized_from_fd(int fd_in, int fd_out) > break; > err = packet_write_gently(fd_out, buf, bytes_to_write); > } > - if (!err) > - err = packet_flush_gently(fd_out); > return err; > } > > @@ -277,8 +275,6 @@ int write_packetized_from_buf(const char *src_in, size_t len, int fd_out) > err = packet_write_gently(fd_out, src_in + bytes_written, bytes_to_write); > bytes_written += bytes_to_write; > } > - if (!err) > - err = packet_flush_gently(fd_out); > return err; > } > > > -Peff >