W dniu 30.07.2016 o 01:37, larsxschneider@xxxxxxxxx pisze:
> From: Lars Schneider <larsxschneider@xxxxxxxxx>
>
> The packet_trace() call is not ideal in format_packet() as we would print
Style; I think the following is more readable:
The packet_trace() call in format_packet() is not ideal, as we would...
> a trace when a packet is formatted and (potentially) when the packet is
> actually send. This was no problem up until now because format_packet()
> was only used by one function. Fix it by moving the trace call into the
> function that actally sends the packet.
s/actally/actually/
I don't buy this explanation. If you want to trace packets, you might
do it on input (when formatting packet), or on output (when writing
packet). It's when there are more than one formatting function, but
one writing function, then placing trace call in write function means
less code duplication; and of course the reverse.
Another issue is that something may happen between formatting packet
and sending it, and we probably want to packet_trace() when packet
is actually send.
Neither of those is visible in commit message.
>
> Signed-off-by: Lars Schneider <larsxschneider@xxxxxxxxx>
> ---
> pkt-line.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index 1728690..32c0a34 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -126,7 +126,6 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)
> die("protocol error: impossibly long line");
>
> set_packet_header(&out->buf[orig_len], n);
> - packet_trace(out->buf + orig_len + 4, n - 4, 1);
> }
>
> void packet_write(int fd, const char *fmt, ...)
> @@ -138,6 +137,7 @@ void packet_write(int fd, const char *fmt, ...)
> va_start(args, fmt);
> format_packet(&buf, fmt, args);
> va_end(args);
> + packet_trace(buf.buf + 4, buf.len - 4, 1);
> write_or_die(fd, buf.buf, buf.len);
> }
>
>
--
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