git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: "Jeff King" <peff@peff.net>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Subject: [PATCH v2] pkt-line: use string versions of functions
Date: Sun, 14 Jun 2020 05:07:54 -0400	[thread overview]
Message-ID: <da9ba5fb2d0fb9481f81ce525cbabaedd722858d.1592125442.git.liu.denton@gmail.com> (raw)
In-Reply-To: <20200614083131.GD3405@danh.dev>

We have many cases where we are writing a control packet as a string
constant out and we need to specify the length of the string. Currently,
the length is specified as a magical `4` literal.

Change these instances to use a function that calls strlen() to
determine the length of the string removing the need to specify the
length at all. Since these functions are inline, the strlen()s should be
replaced with constants at compile-time so this should not result in any
performance penalty.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Hi Đoàn,

Perhaps something like this?

 pkt-line.c | 48 ++++++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/pkt-line.c b/pkt-line.c
index 8f9bc68ee2..022376f00f 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -81,49 +81,61 @@ static void packet_trace(const char *buf, unsigned int len, int write)
 	strbuf_release(&out);
 }
 
+static inline void packet_trace_str(const char *buf, int write)
+{
+	packet_trace(buf, strlen(buf), write);
+}
+
+#define control_packet_write(fd, s, errstr) \
+	do { \
+		(void)s"is a string constant"; \
+		packet_trace_str((s), 1); \
+		if (write_str_in_full((fd), (s)) < 0) \
+			die_errno((errstr)); \
+	} while (0)
+
 /*
  * If we buffered things up above (we don't, but we should),
  * we'd flush it here
  */
 void packet_flush(int fd)
 {
-	packet_trace("0000", 4, 1);
-	if (write_in_full(fd, "0000", 4) < 0)
-		die_errno(_("unable to write flush packet"));
+	control_packet_write(fd, "0000", _("unable to write flush packet"));
 }
 
 void packet_delim(int fd)
 {
-	packet_trace("0001", 4, 1);
-	if (write_in_full(fd, "0001", 4) < 0)
-		die_errno(_("unable to write delim packet"));
+	control_packet_write(fd, "0001", _("unable to write delim packet"));
 }
 
 void packet_response_end(int fd)
 {
-	packet_trace("0002", 4, 1);
-	if (write_in_full(fd, "0002", 4) < 0)
-		die_errno(_("unable to write stateless separator packet"));
+	control_packet_write(fd, "0002", _("unable to write stateless separator packet"));
 }
 
 int packet_flush_gently(int fd)
 {
-	packet_trace("0000", 4, 1);
-	if (write_in_full(fd, "0000", 4) < 0)
+	packet_trace_str("0000", 1);
+	if (write_str_in_full(fd, "0000") < 0)
 		return error(_("flush packet write failed"));
 	return 0;
 }
 
+#define control_packet_buf_write(buf, s) \
+	do { \
+		(void)s"is a string constant"; \
+		packet_trace_str((s), 1); \
+		strbuf_addstr((buf), (s)); \
+	} while (0)
+
 void packet_buf_flush(struct strbuf *buf)
 {
-	packet_trace("0000", 4, 1);
-	strbuf_add(buf, "0000", 4);
+	control_packet_buf_write(buf, "0000");
 }
 
 void packet_buf_delim(struct strbuf *buf)
 {
-	packet_trace("0001", 4, 1);
-	strbuf_add(buf, "0001", 4);
+	control_packet_buf_write(buf, "0001");
 }
 
 void set_packet_header(char *buf, int size)
@@ -337,15 +349,15 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
 	if (len < 0) {
 		die(_("protocol error: bad line length character: %.4s"), linelen);
 	} else if (!len) {
-		packet_trace("0000", 4, 0);
+		packet_trace_str("0000", 0);
 		*pktlen = 0;
 		return PACKET_READ_FLUSH;
 	} else if (len == 1) {
-		packet_trace("0001", 4, 0);
+		packet_trace_str("0001", 0);
 		*pktlen = 0;
 		return PACKET_READ_DELIM;
 	} else if (len == 2) {
-		packet_trace("0002", 4, 0);
+		packet_trace_str("0002", 0);
 		*pktlen = 0;
 		return PACKET_READ_RESPONSE_END;
 	} else if (len < 4) {
-- 
2.27.0.132.g321788e831


  reply	other threads:[~2020-06-14  9:08 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-13 13:43 [PATCH] pkt-line: extract out PACKET_HEADER_SIZE Denton Liu
2020-06-13 14:23 ` Đoàn Trần Công Danh
2020-06-13 14:39   ` Denton Liu
2020-06-13 16:51   ` Junio C Hamano
2020-06-14 18:24     ` Junio C Hamano
2020-06-14  7:31 ` Denton Liu
2020-06-14  7:31 ` [PATCH v2 0/3] pkt-line: war on magical `4` literal Denton Liu
2020-06-14  7:31   ` [PATCH v2 1/3] remote-curl: use strlen() instead of magic numbers Denton Liu
2020-06-14  7:31   ` [PATCH v2 2/3] pkt-line: use string versions of functions Denton Liu
2020-06-14  8:31     ` Đoàn Trần Công Danh
2020-06-14  9:07       ` Denton Liu [this message]
2020-06-14 21:35         ` [PATCH v2] " Junio C Hamano
2020-06-14 22:28           ` Denton Liu
2020-06-15 12:32         ` Đoàn Trần Công Danh
2020-06-14  7:32   ` [PATCH v2 3/3] pkt-line: extract out PACKET_HEADER_SIZE Denton Liu
2020-06-14 21:32   ` [PATCH v2 0/3] pkt-line: war on magical `4` literal Junio C Hamano
2020-06-23 17:55   ` [PATCH v3 " Denton Liu
2020-06-23 17:55     ` [PATCH v3 1/3] remote-curl: use strlen() instead of magic numbers Denton Liu
2020-06-23 18:54       ` Jeff King
2020-06-23 19:39         ` Junio C Hamano
2020-06-23 17:55     ` [PATCH v3 2/3] pkt-line: use string versions of functions Denton Liu
2020-06-23 19:11       ` Jeff King
2020-06-23 17:55     ` [PATCH v3 3/3] pkt-line: extract out PACKET_HEADER_SIZE Denton Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=da9ba5fb2d0fb9481f81ce525cbabaedd722858d.1592125442.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).