Re: [PATCH v8 02/11] pkt-line: extract set_packet_header()

[Lars Schneider]

On 24 Sep 2016, at 23:22, Jakub Narębski  wrote:

> W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:
> 
>> From: Lars Schneider 
>> 
>> Subject: [PATCH v8 02/11] pkt-line: extract set_packet_header()
>> 
>> set_packet_header() converts an integer to a 4 byte hex string. Make
>> this function locally available so that other pkt-line functions can
>> use it.
> 
> Ah. I have trouble understanding this commit message, as the
> set_packet_header() was not available before this patch, but it
> is good if one reads it together with commit summary / title.
> 
> Writing
> 
>  Extracted set_packet_header() function converts...
> 
> or
> 
>  New set_packet_header() function converts... 
> 
> would make it more clear, but it is all right as it is now.
> Perhaps also
> 
>  ... could use it.
> 
> as currently no other pkt-line function but the one set_packet_header()
> was extracted from, namely format_packet(), uses it.
> 
> But that is just nitpicking; no need to change on that account.

Changed it:

Extracted set_packet_header() function converts an integer to a 4 byte 
hex string. Make this function locally available so that other pkt-line 
functions could use it.

Thanks,
Lars

Re: [PATCH v8 02/11] pkt-line: extract set_packet_header()

[Jakub Narębski]

W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:

> From: Lars Schneider 
>
> Subject: [PATCH v8 02/11] pkt-line: extract set_packet_header()
> 
> set_packet_header() converts an integer to a 4 byte hex string. Make
> this function locally available so that other pkt-line functions can
> use it.

Ah. I have trouble understanding this commit message, as the
set_packet_header() was not available before this patch, but it
is good if one reads it together with commit summary / title.

Writing

  Extracted set_packet_header() function converts...

or

  New set_packet_header() function converts... 

would make it more clear, but it is all right as it is now.
Perhaps also

  ... could use it.

as currently no other pkt-line function but the one set_packet_header()
was extracted from, namely format_packet(), uses it.

But that is just nitpicking; no need to change on that account.

> 
> Signed-off-by: Lars Schneider 

[PATCH v8 02/11] pkt-line: extract set_packet_header()

[larsxschneider]

From: Lars Schneider 

set_packet_header() converts an integer to a 4 byte hex string. Make
this function locally available so that other pkt-line functions can
use it.

Signed-off-by: Lars Schneider 
---
 pkt-line.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/pkt-line.c b/pkt-line.c
index 0a9b61c..e8adc0f 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -97,10 +97,20 @@ void packet_buf_flush(struct strbuf *buf)
strbuf_add(buf, "", 4);
 }
 
-#define hex(a) (hexchar[(a) & 15])
-static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+static void set_packet_header(char *buf, const int size)
 {
static char hexchar[] = "0123456789abcdef";
+
+   #define hex(a) (hexchar[(a) & 15])
+   buf[0] = hex(size >> 12);
+   buf[1] = hex(size >> 8);
+   buf[2] = hex(size >> 4);
+   buf[3] = hex(size);
+   #undef hex
+}
+
+static void format_packet(struct strbuf *out, const char *fmt, va_list args)
+{
size_t orig_len, n;
 
orig_len = out->len;
@@ -111,10 +121,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);
 }
 
-- 
2.10.0