Skip to content

Commit f2e85c2

Browse files
committed
wt-status: avoid strbuf_split*()
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 e813a02 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 <[email protected]>
1 parent f4864b2 commit f2e85c2

File tree

1 file changed

+10
-21
lines changed

1 file changed

+10
-21
lines changed

wt-status.c

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,35 +1351,24 @@ static int split_commit_in_progress(struct wt_status *s)
13511351
*/
13521352
static void abbrev_oid_in_line(struct strbuf *line)
13531353
{
1354-
struct strbuf **split;
1355-
int i;
1354+
struct string_list split = STRING_LIST_INIT_DUP;
1355+
struct object_id oid;
13561356

13571357
if (starts_with(line->buf, "exec ") ||
13581358
starts_with(line->buf, "x ") ||
13591359
starts_with(line->buf, "label ") ||
13601360
starts_with(line->buf, "l "))
13611361
return;
13621362

1363-
split = strbuf_split_max(line, ' ', 3);
1364-
if (split[0] && split[1]) {
1365-
struct object_id oid;
1366-
1367-
/*
1368-
* strbuf_split_max left a space. Trim it and re-add
1369-
* it after abbreviation.
1370-
*/
1371-
strbuf_trim(split[1]);
1372-
if (!repo_get_oid(the_repository, split[1]->buf, &oid)) {
1373-
strbuf_reset(split[1]);
1374-
strbuf_add_unique_abbrev(split[1], &oid,
1375-
DEFAULT_ABBREV);
1376-
strbuf_addch(split[1], ' ');
1377-
strbuf_reset(line);
1378-
for (i = 0; split[i]; i++)
1379-
strbuf_addbuf(line, split[i]);
1380-
}
1363+
if ((2 <= string_list_split(&split, line->buf, " ", 2)) &&
1364+
!repo_get_oid(the_repository, split.items[1].string, &oid)) {
1365+
strbuf_reset(line);
1366+
strbuf_addf(line, "%s ", split.items[0].string);
1367+
strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);
1368+
for (size_t i = 2; i < split.nr; i++)
1369+
strbuf_addf(line, " %s", split.items[i].string);
13811370
}
1382-
strbuf_list_free(split);
1371+
string_list_clear(&split, 0);
13831372
}
13841373

13851374
static int read_rebase_todolist(const char *fname, struct string_list *lines)

0 commit comments

Comments
 (0)