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

[PATCH v2 01/11] wt-status: avoid strbuf_split*()

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 31, 2025, 22:54 UTC
Message-ID
<20250731225433.4028872-2-gitster@pobox.com>
In-Reply-To
<20250731225433.4028872-1-gitster@pobox.com>

strbuf is a very good data structure to work with string data without having to worry about running past the end of the string, but strbuf_split() is a wrong API and an array of strbuf that the function produces is a wrong thing to use in general. You do not edit these N strings split out of a single strbuf simultaneously. Often it is much better off to split a string into string_list and work with the resulting strings.

wt-status.c:abbrev_oid_in_line() takes one line of rebase todo list (like "pick e813a0200a7121b97fec535f0d0b460b0a33356c title"), and for instructions that has an object name as the second token on the line, replace the object name with its unique abbreviation. After splitting these tokens out of a single line, no simultaneous edit on any of these pieces of string that takes advantage of strbuf API takes place. The final string is composed with strbuf API, but these split pieces are merely used as pieces of strings and there is no need for them to be stored in individual strbuf.

Instead, split the line into a string_list, and compose the final string using these pieces.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 454601afa1..a34dc144ee 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1351,8 +1351,8 @@ static int split_commit_in_progress(struct wt_status *s)
  */
 static void abbrev_oid_in_line(struct strbuf *line)
 {
-	struct strbuf **split;
-	int i;
+	struct string_list split = STRING_LIST_INIT_DUP;
+	struct object_id oid;
 
 	if (starts_with(line->buf, "exec ") ||
 	    starts_with(line->buf, "x ") ||
@@ -1360,26 +1360,15 @@ static void abbrev_oid_in_line(struct strbuf *line)
 	    starts_with(line->buf, "l "))
 		return;
 
-	split = strbuf_split_max(line, ' ', 3);
-	if (split[0] && split[1]) {
-		struct object_id oid;
-
-		/*
-		 * strbuf_split_max left a space. Trim it and re-add
-		 * it after abbreviation.
-		 */
-		strbuf_trim(split[1]);
-		if (!repo_get_oid(the_repository, split[1]->buf, &oid)) {
-			strbuf_reset(split[1]);
-			strbuf_add_unique_abbrev(split[1], &oid,
-						 DEFAULT_ABBREV);
-			strbuf_addch(split[1], ' ');
-			strbuf_reset(line);
-			for (i = 0; split[i]; i++)
-				strbuf_addbuf(line, split[i]);
-		}
+	if ((2 <= string_list_split(&split, line->buf, " ", 2)) &&
+	    !repo_get_oid(the_repository, split.items[1].string, &oid)) {
+		strbuf_reset(line);
+		strbuf_addf(line, "%s ", split.items[0].string);
+		strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);
+		for (size_t i = 2; i < split.nr; i++)
+			strbuf_addf(line, " %s", split.items[i].string);
 	}
-	strbuf_list_free(split);
+	string_list_clear(&split, 0);
 }
 
 static int read_rebase_todolist(const char *fname, struct string_list *lines)
-- 
2.50.1-618-g45d530d26b
Previous: Junio C HamanoNext: Junio C Hamano
Message 16 of 33 in “do not overuse strbuf_split*()”
  1. 0/9 do not overuse strbuf_split*()Junio C Hamano, Jul 31, 2025
  2. 1/9 wt-status: avoid strbuf_split*()Junio C Hamano, Jul 31, 2025
  3. 2/9 clean: do not pass strbuf by valueJunio C Hamano, Jul 31, 2025
  4. 3/9 clean: do not use strbuf_split*() [part 1]Junio C Hamano, Jul 31, 2025
  5. 4/9 clean: do not use strbuf_split*() [part 2]Junio C Hamano, Jul 31, 2025
  6. 5/9 merge-tree: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  7. 6/9 notes: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  8. Eric SunshineJul 31, 2025
  9. 7/9 config: do not use strbuf_split()Junio C Hamano, Jul 31, 2025
  10. Eric SunshineJul 31, 2025
  11. 8/9 environment: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  12. 9/9 sub-process: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  13. Christian CouderJul 31, 2025
  14. Junio C HamanoJul 31, 2025
  15. 00/11 do not overuse strbuf_split*()Junio C Hamano, Jul 31, 2025
  16. 01/11 wt-status: avoid strbuf_split*()Junio C Hamano, Jul 31, 2025
  17. 02/11 clean: do not pass strbuf by valueJunio C Hamano, Jul 31, 2025
  18. Jeff KingAug 2, 2025
  19. Junio C HamanoAug 2, 2025
  20. Jeff KingAug 2, 2025
  21. 03/11 clean: do not use strbuf_split*() [part 1]Junio C Hamano, Jul 31, 2025
  22. 04/11 clean: do not use strbuf_split*() [part 2]Junio C Hamano, Jul 31, 2025
  23. 05/11 merge-tree: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  24. Jeff KingAug 2, 2025
  25. 06/11 notes: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  26. 07/11 config: do not use strbuf_split()Junio C Hamano, Jul 31, 2025
  27. 08/11 environment: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  28. 09/11 sub-process: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  29. 10/11 trace2: trim_trailing_newline followed by trim is a no-opJunio C Hamano, Jul 31, 2025
  30. 11/11 trace2: do not use strbuf_split*()Junio C Hamano, Jul 31, 2025
  31. Jeff KingAug 2, 2025
  32. Junio C HamanoAug 2, 2025
  33. Jeff KingAug 2, 2025

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.