Re: [PATCH v4 01/12] pkt-line: eliminate the need for static buffer in packet_write_gently()
flat view
From: Jeff King <hidden>
Date: 2021-02-26 07:22:51
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...
quoted hunk ↗ jump to hunk
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:
quoted hunk ↗ jump to hunk
-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