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...
> 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:
> -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
