git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/4] test-line-buffer: simplify command parsing

From
Jeff King <peff@peff.net>
Date
Sep 21, 2017, 06:22 UTC
Message-ID
<20170921062242.t63u2gvpysrrmijs@sigill.intra.peff.net>
In-Reply-To
<20170921062043.la2f3pjsnojirzyw@sigill.intra.peff.net>

The handle_command() function matches an incoming command string with a sequence of starts_with() checks. But it also surrounds these with a switch on the first character of the command, which lets us jump to the right block of starts_with() without going linearly through the list.

However, each case arm of the switch falls through to the one below it. This is pointless (we know that a command starting with 'b' does not need to check any of the commands in the 'c' block), and it makes gcc's -Wimplicit-fallthrough complain.

We could solve this by adding a break at the end of each block. However, this optimization isn't helping anything. Even if it does make matching faster (which is debatable), this is code that is run only in the test suite, and each run receives at most two of these "commands". We should favor simplicity and readability over micro-optimizing.

Instead, let's drop the switch statement completely and replace it with an if/else cascade.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/helper/test-line-buffer.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/t/helper/test-line-buffer.c b/t/helper/test-line-buffer.c
index 81575fe2ab..078dd7e29d 100644
--- a/t/helper/test-line-buffer.c
+++ b/t/helper/test-line-buffer.c
@@ -17,27 +17,17 @@ static uint32_t strtouint32(const char *s)
 
 static void handle_command(const char *command, const char *arg, struct line_buffer *buf)
 {
-	switch (*command) {
-	case 'b':
-		if (starts_with(command, "binary ")) {
-			struct strbuf sb = STRBUF_INIT;
-			strbuf_addch(&sb, '>');
-			buffer_read_binary(buf, &sb, strtouint32(arg));
-			fwrite(sb.buf, 1, sb.len, stdout);
-			strbuf_release(&sb);
-			return;
-		}
-	case 'c':
-		if (starts_with(command, "copy ")) {
-			buffer_copy_bytes(buf, strtouint32(arg));
-			return;
-		}
-	case 's':
-		if (starts_with(command, "skip ")) {
-			buffer_skip_bytes(buf, strtouint32(arg));
-			return;
-		}
-	default:
+	if (starts_with(command, "binary ")) {
+		struct strbuf sb = STRBUF_INIT;
+		strbuf_addch(&sb, '>');
+		buffer_read_binary(buf, &sb, strtouint32(arg));
+		fwrite(sb.buf, 1, sb.len, stdout);
+		strbuf_release(&sb);
+	} else if (starts_with(command, "copy ")) {
+		buffer_copy_bytes(buf, strtouint32(arg));
+	} else if (starts_with(command, "skip ")) {
+		buffer_skip_bytes(buf, strtouint32(arg));
+	} else {
 		die("unrecognized command: %s", command);
 	}
 }
-- 
2.14.1.1051.g8c9143ec35
Previous: Jonathan NiederNext: Junio C Hamano
Message 4 of 7 in “appeasing -Wimplicit-fallthrough”
  1. 0/4 appeasing -Wimplicit-fallthroughJeff King, Sep 21, 2017
  2. 1/4 cat-file: handle NULL object_context.pathJeff King, Sep 21, 2017
  3. Jonathan NiederSep 22, 2017
  4. 2/4 test-line-buffer: simplify command parsingJeff King, Sep 21, 2017
  5. Junio C HamanoSep 22, 2017
  6. 3/4 curl_trace(): eliminate switch fallthroughJeff King, Sep 21, 2017
  7. 4/4 consistently use "fallthrough" comments in switchesJeff King, Sep 21, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.