larsxschneider@xxxxxxxxx writes:
> From: Lars Schneider <larsxschneider@xxxxxxxxx>
>
> packet_write_fmt() has two shortcomings. First, it uses format_packet()
> which lets the caller only send string data via "%s". That means it
> cannot be used for arbitrary data that may contain NULs. Second, it will
> always die on error.
As you introduced _gently in 3/13, the latter is no longer a valid
excuse to add this function. Just remove the sentence "Second, ...".
> Add packet_write_gently() which writes arbitrary data and returns `0`
> for success and `-1` for an error. This function is used by other
> pkt-line functions in a subsequent patch.
>
> Signed-off-by: Lars Schneider <larsxschneider@xxxxxxxxx>
> ---
> pkt-line.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index cad26df..7e8a803 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -3,6 +3,7 @@
> #include "run-command.h"
>
> char packet_buffer[LARGE_PACKET_MAX];
> +static char packet_write_buffer[LARGE_PACKET_MAX];
> static const char *packet_trace_prefix = "git";
> static struct trace_key trace_packet = TRACE_KEY_INIT(PACKET);
> static struct trace_key trace_pack = TRACE_KEY_INIT(PACKFILE);
> @@ -155,6 +156,17 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)
> return (write_in_full(fd, buf.buf, buf.len) == buf.len ? 0 : -1);
> }
>
> +int packet_write_gently(const int fd_out, const char *buf, size_t size)
> +{
> + if (size > sizeof(packet_write_buffer) - 4)
> + return -1;
> + packet_trace(buf, size, 1);
> + memmove(packet_write_buffer + 4, buf, size);
> + size += 4;
> + set_packet_header(packet_write_buffer, size);
It may not matter all that much, but from code-reader's point of
view, when you know packet_write_buffer[] will contain things A and
B in this order, and when you have enough information to compute A
before stasrting to fill packet_write_buffer[], I would prefer to
see you actually fill the buffer in that natural order.
Do you anticipate future need of non-gently variant of this
function? If so, perhaps a helper that takes a boolean "am I
working for the gently variant?" may help share more code.
> + return (write_in_full(fd_out, packet_write_buffer, size) == size ? 0 : -1);
> +}
> +
> void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
> {
> va_list args;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html