Re: [PATCH v4 01/12] pkt-line: eliminate the need for static buffer in packet_write_gently()
- From
Jeff King <peff@peff.net>
- Date
- Feb 26, 2021, 07:21 UTC
- Message-ID
- <YDihb2Kspbh4FIlW@coredump.intra.peff.net>
- In-Reply-To
- <2d6858b1625aa3c96688c6c6a9157c2d2b16f43e.1613598529.git.gitgitgadget@gmail.com>
On Wed, Feb 17, 2021 at 09:48:37PM +0000, Jeff Hostetler via GitGitGadget wrote:
> Change the API of `write_packetized_from_fd()` to accept a scratch space > argument from its caller to avoid similar issues here.
OK, but...
Show 16 quoted lines
> diff --git a/convert.c b/convert.c
> index ee360c2f07ce..41012c2d301c 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -883,9 +883,10 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
> if (err)
> goto done;
>
> - if (fd >= 0)
> - err = write_packetized_from_fd(fd, process->in);
> - else
> + if (fd >= 0) {
> + struct packet_scratch_space scratch;
> + err = write_packetized_from_fd(fd, process->in, &scratch);
> + } else
> err = write_packetized_from_buf(src, len, process->in);Isn't this just putting the buffer onto the stack anyway? Your scratch_space struct is really just a big array. You'd want to make it static here, but then we haven't really solved anything. :)
I think instead that:
Show 20 quoted lines
> -int write_packetized_from_fd(int fd_in, int fd_out)
> +int write_packetized_from_fd(int fd_in, int fd_out,
> + struct packet_scratch_space *scratch)
> {
> - static char buf[LARGE_PACKET_DATA_MAX];
> int err = 0;
> ssize_t bytes_to_write;
>
> while (!err) {
> - bytes_to_write = xread(fd_in, buf, sizeof(buf));
> + bytes_to_write = xread(fd_in, scratch->buffer,
> + sizeof(scratch->buffer));
> if (bytes_to_write < 0)
> return COPY_READ_ERROR;
> if (bytes_to_write == 0)
> break;
> - err = packet_write_gently(fd_out, buf, bytes_to_write);
> + err = packet_write_gently(fd_out, scratch->buffer,
> + bytes_to_write);
> }...just heap-allocating the buffer in this function would be fine. It's one malloc for the whole sequence of pktlines, which is unlikely to be a problem.
-Peff