[PATCH 3/4] strbuf: introduce `strbuf_attachstr()`
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Apr 19, 2020, 12:32 UTC
- Message-ID
- <c3012f1da361af354a904f821b83d61f2534ccb2.1587297254.git.martin.agren@gmail.com>
- In-Reply-To
- <cover.1587297254.git.martin.agren@gmail.com>
Similar to the previous commit, introduce `strbuf_attachstr()` where we don't even have to pass in the length of the string that we want to attach. Convert existing callers of `strbuf_attachstr()` that use `strlen()`.
Note how only one caller passes in `mem == len + 1` and that the others have been using `strbuf_attach()` in direct contradiction to how it was (incorrectly) documented up until a few commits ago.
Now that the documentation has been fixed, you might say these are all fine. But the calling convention of `strbuf_attach()` seems sufficiently hard to get right that it's probably a good idea to introduce this helper.
This could help reduce reallocations and memory waste. When we pessimistically pass in `strlen(foo)` for `mem`, the strbuf will have `alloc == len` and will do a reallocation, not just to get one more byte for the NUL (which would have been a no-op), but because we're using `ALLOC_GROW` under the hood, we will ask for 16 more bytes and another 50% on top of that.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- strbuf.h | 11 +++++++++++ path.c | 3 +-- pretty.c | 2 +- refs/files-backend.c | 3 +-- trailer.c | 2 +- 5 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/strbuf.h b/strbuf.h index 7d0aeda434..32cc15de0c 100644 --- a/strbuf.h +++ b/strbuf.h @@ -140,6 +140,17 @@ static inline void strbuf_attachstr_len(struct strbuf *sb, strbuf_attach(sb, str, len, len + 1); } +/** + * Attach a string to a buffer similar to `strbuf_attachstr_len()`. + * Useful if you do not know the length of the string. + */ +static inline void strbuf_attachstr(struct strbuf *sb, char *str) +{ + size_t len = strlen(str); + + strbuf_attach(sb, str, len, len + 1); +} + /** * Swap the contents of two string buffers. */ diff --git a/path.c b/path.c index 9bd717c307..3cd8fd56b4 100644 --- a/path.c +++ b/path.c @@ -815,8 +815,7 @@ const char *enter_repo(const char *path, int strict) char *newpath = expand_user_path(used_path.buf, 0); if (!newpath) return NULL; - strbuf_attach(&used_path, newpath, strlen(newpath), - strlen(newpath)); + strbuf_attachstr(&used_path, newpath); } for (i = 0; suffix[i]; i++) { struct stat st; diff --git a/pretty.c b/pretty.c index e171830389..5ecdf0cbb2 100644 --- a/pretty.c +++ b/pretty.c @@ -590,7 +590,7 @@ static char *replace_encoding_header(char *buf, const char *encoding) return buf; /* should not happen but be defensive */ len = cp + 1 - (buf + start); - strbuf_attach(&tmp, buf, strlen(buf), strlen(buf) + 1); + strbuf_attachstr(&tmp, buf); if (is_encoding_utf8(encoding)) { /* we have re-coded to UTF-8; drop the header */ strbuf_remove(&tmp, start, len); diff --git a/refs/files-backend.c b/refs/files-backend.c index 561c33ac8a..eb058d85b6 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1511,10 +1511,9 @@ static int commit_ref(struct ref_lock *lock) * the lockfile to. Hopefully it is empty; try to * delete it. */ - size_t len = strlen(path); struct strbuf sb_path = STRBUF_INIT; - strbuf_attach(&sb_path, path, len, len); + strbuf_attachstr(&sb_path, path); /* * If this fails, commit_lock_file() will also fail diff --git a/trailer.c b/trailer.c index 0c414f2fed..56c4027943 100644 --- a/trailer.c +++ b/trailer.c @@ -1095,7 +1095,7 @@ void trailer_info_get(struct trailer_info *info, const char *str, for (ptr = trailer_lines; *ptr; ptr++) { if (last && isspace((*ptr)->buf[0])) { struct strbuf sb = STRBUF_INIT; - strbuf_attach(&sb, *last, strlen(*last), strlen(*last)); + strbuf_attachstr(&sb, *last); strbuf_addbuf(&sb, *ptr); *last = strbuf_detach(&sb, NULL); continue;
-- 2.26.1