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 an array of strbuf is often a wrong data structure. You rarely
have need to be able to edit multiple strings represented by such an
array simultaneously. And strbuf_split*() that produces result in
such a shape is a misdesigned API function.
The most common use case of strbuf_split*() family of functions
seems to be to trim away the whitespaces around each piece of split
string. With modern string_list_split*(), it is often no longer
necessary.
This series builds on top of the other series that extends string
list API to allow string_list_split() to take more than one delimiter
bytes, and to optionally trim the resulting string pieces.
I do not plan to eradicate all the uses of strbuf_split*() myself,
not because I found some valid use cases in the existing code (I
haven't yet), but these patches would give interested others enough
material to study and mimic to continue the effort and I can safely
leave it as #leftoverbits to rewrite them.
Relative to v2, this iteration v3 adds one more clean-up step to
correct a callee that insists on taking a while strbuf when it can
work with any NUL-terminated strings, and comes with a handful of
typofixes.
Junio C Hamano (12):
wt-status: avoid strbuf_split*()
clean: do not pass strbuf by value
clean: do not use strbuf_split*() [part 1]
clean: do not pass the whole structure when it is not necessary
clean: do not use strbuf_split*() [part 2]
merge-tree: do not use strbuf_split*()
notes: do not use strbuf_split*()
config: do not use strbuf_split()
environment: do not use strbuf_split*()
sub-process: do not use strbuf_split*()
trace2: trim_trailing_newline followed by trim is a no-op
trace2: do not use strbuf_split*()
builtin/clean.c | 74 ++++++++++++++++++++--------------------
builtin/merge-tree.c | 30 +++++++++--------
builtin/notes.c | 23 +++++++------
config.c | 23 ++++++-------
environment.c | 19 +++++++----
sub-process.c | 15 ++++-----
trace2/tr2_cfg.c | 80 +++++++++++++++-----------------------------
wt-status.c | 31 ++++++-----------
8 files changed, 129 insertions(+), 166 deletions(-)
Range-diff against v2:
1: 27de3d9a92 = 1: 2efe707054 wt-status: avoid strbuf_split*()
2: 8f096e5a2d = 2: 899ff9c175 clean: do not pass strbuf by value
3: 768b08907e = 3: 7a4acc3607 clean: do not use strbuf_split*() [part 1]
-: ---------- > 4: 4985f72ea5 clean: do not pass the whole structure when it is not necessary
4: 0f8583e798 = 5: 4f60672f6f clean: do not use strbuf_split*() [part 2]
5: cefc2ec9f5 = 6: d33091220d merge-tree: do not use strbuf_split*()
6: 1c8ea097f6 ! 7: 566e910495 notes: do not use strbuf_split*()
@@ Metadata
## Commit message ##
notes: do not use strbuf_split*()
- When reading the copy instruction from the standard input, the
- program reads a line, splits it into tokens at whitespace, and trims
- each of the tokens before using. We no longer need to use strbuf
- just to be able to trimming, as string_list_split*() family now can
- trim while splitting a string.
+ When reading copy instructions from the standard input, the program
+ reads a line, splits it into tokens at whitespace, and trims each of
+ the tokens before using. We no longer need to use strbuf just to be
+ able to trim, as string_list_split*() family now can trim while
+ splitting a string.
- Retire the use of strbuf_split().
+ Retire the use of strbuf_split() from this code path.
Note that this loop is a bit sloppy in that it ensures at least
there are two tokens on each line, but ignores if there are extra
7: a472688ec1 ! 8: dcecac2580 config: do not use strbuf_split()
@@ Commit message
config: do not use strbuf_split()
When parsing an old-style GIT_CONFIG_PARAMETERS environment
- variable, the code parses the key=value pair by spliting them at '='
- into an array of strbuf's. As strbuf_split() leafes the delimiter
+ variable, the code parses key=value pairs by splitting them at '='
+ into an array of strbuf's. As strbuf_split() leaves the delimiter
at the end of the split piece, the code has to manually trim it.
If we split with string_list_split(), that becomes unnecessary.
- Retire the use of strbuf_split().
+ Retire the use of strbuf_split() from this code path.
Note that the max parameter of string_list_split() is of
an ergonomically iffy design---it specifies the maximum number of
8: 2b9957f31c = 9: b894d4481f environment: do not use strbuf_split*()
9: 4a5599836d = 10: d6fd08bd76 sub-process: do not use strbuf_split*()
10: cf6ecd2090 ! 11: cb8e82a641 trace2: trim_trailing_newline followed by trim is a no-op
@@ Commit message
of a string. If the code plans to call strbuf_trim() immediately
after doing so, the code is better off skipping the EOL trimming in
the first place. After all, LF/CRLF at the end is a mere special
- case of whitespaces at the right end of the string, which will be
- removed by strbuf_rtrim().
+ case of whitespaces at the end of the string, which will be removed
+ by strbuf_rtrim() anyway.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
11: c2578b6b1c ! 12: 838fe56920 trace2: do not use strbuf_split*()
@@ Metadata
## Commit message ##
trace2: do not use strbuf_split*()
- tr2_cfg_load_patterns() and tr2_load_env_vars() functions are copied
- and pasted pair of functions that each reads an environment
- variable, split the value at ',' boundaries and trims the resulting
- string pieces into an array of strbufs. But the code paths that
- later use these strbufs take no advantage of the strbuf-ness of the
- result (they do not benefit from <ptr,len> representation to avoid
- having to run strlne(<ptr>), for example).
+ tr2_cfg_load_patterns() and tr2_load_env_vars() functions are
+ functions with very similar structure that each reads an environment
+ variable, splits its value at the ',' boundaries, and trims the
+ resulting string pieces into an array of strbufs.
+
+ But the code paths that later use these strbufs take no advantage of
+ the strbuf-ness of the result (they do not benefit from <ptr,len>
+ representation to avoid having to run strlen(<ptr>), for example).
Simplify the code by teaching these functions to split into a string
- list instead.
+ list instead; even the trimming comes for free ;-).
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
--
2.50.1-633-g69dfdd50af
