Re: [PATCH v2 3/5] pkt-line: extract and use `set_packet_header` function
Lars Schneider writes: >> On 27 Jul 2016, at 02:20, Junio C Hamano wrote: >> >> larsxschnei...@gmail.com writes: >> >>> From: Lars Schneider >>> >>> `set_packet_header` converts an integer to a 4 byte hex string. Make >>> this function publicly available so that other parts of Git can easily >>> generate a pkt-line. >> >> I think that having to do this is a strong sign that the design of >> this series is going in a wrong direction. > > Thanks for the feedback. Do you think using "pkt-line" is a move into > the wrong direction in general or do you think only my usage of > "pkt-line" is not ideal? I only meant this: If you try to produce packet-line data without using helper functions in pkt-line.[ch] that are designed to do so (presumably because the current set of helpers lack some capability you want to use), I am not enthused. And I did not see a utility of a public set-packet-header helper unless you are hand-rolling a function that produces packet-line data outside pkt-line.[ch], hence the comment we are discussing here was made before actually seeing how this new helper is used. As to the use of packet-line as the data format, I do not have a strong opinion either way. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/5] pkt-line: extract and use `set_packet_header` function
> On 27 Jul 2016, at 02:20, Junio C Hamano wrote: > > larsxschnei...@gmail.com writes: > >> From: Lars Schneider >> >> `set_packet_header` converts an integer to a 4 byte hex string. Make >> this function publicly available so that other parts of Git can easily >> generate a pkt-line. > > I think that having to do this is a strong sign that the design of > this series is going in a wrong direction. Thanks for the feedback. Do you think using "pkt-line" is a move into the wrong direction in general or do you think only my usage of "pkt-line" is not ideal? > If you need a helper function that writes a pkt-line format that > behaves differently from what is already available (for example, > packet_write()), it would be much better to design that new function > so that it would be generally useful and add that to pkt-line.[ch], > instead of creating random helper functions that use write(2) > directly, bypassing pkt-line API, to write stuff. > > In other words, do not _mimick_ pkt-line; enhance pkt-line as > necessary and use it. OK, I understand your argument. If we agree on the "pkt-line" usage then I will address this issue. Thanks, Lars -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/5] pkt-line: extract and use `set_packet_header` function
larsxschnei...@gmail.com writes: > From: Lars Schneider > > `set_packet_header` converts an integer to a 4 byte hex string. Make > this function publicly available so that other parts of Git can easily > generate a pkt-line. I think that having to do this is a strong sign that the design of this series is going in a wrong direction. If you need a helper function that writes a pkt-line format that behaves differently from what is already available (for example, packet_write()), it would be much better to design that new function so that it would be generally useful and add that to pkt-line.[ch], instead of creating random helper functions that use write(2) directly, bypassing pkt-line API, to write stuff. In other words, do not _mimick_ pkt-line; enhance pkt-line as necessary and use it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] pkt-line: extract and use `set_packet_header` function
From: Lars Schneider
`set_packet_header` converts an integer to a 4 byte hex string. Make
this function publicly available so that other parts of Git can easily
generate a pkt-line.
Signed-off-by: Lars Schneider
---
pkt-line.c | 15 ++-
pkt-line.h | 1 +
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index 62fdb37..6820224 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -98,9 +98,17 @@ void packet_buf_flush(struct strbuf *buf)
}
#define hex(a) (hexchar[(a) & 15])
-static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+void set_packet_header(char *buf, const int size)
{
static char hexchar[] = "0123456789abcdef";
+ buf[0] = hex(size >> 12);
+ buf[1] = hex(size >> 8);
+ buf[2] = hex(size >> 4);
+ buf[3] = hex(size);
+}
+
+static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+{
size_t orig_len, n;
orig_len = out->len;
@@ -111,10 +119,7 @@ static void format_packet(struct strbuf *out, const char
*fmt, va_list args)
if (n > LARGE_PACKET_MAX)
die("protocol error: impossibly long line");
- out->buf[orig_len + 0] = hex(n >> 12);
- out->buf[orig_len + 1] = hex(n >> 8);
- out->buf[orig_len + 2] = hex(n >> 4);
- out->buf[orig_len + 3] = hex(n);
+ set_packet_header(&out->buf[orig_len], n);
packet_trace(out->buf + orig_len + 4, n - 4, 1);
}
diff --git a/pkt-line.h b/pkt-line.h
index 3cb9d91..925c6d3 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -23,6 +23,7 @@ void packet_flush(int fd);
void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf,
2, 3)));
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
+void set_packet_header(char *buf, const int size);
/*
* Read a packetized line into the buffer, which must be at least size bytes
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
