[PATCH 2/4] strbuf: introduce `strbuf_attachstr_len()`
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Apr 19, 2020, 12:32 UTC
- Message-ID
- <d9db28d6e6616017a4a8e57508d166ab5aa2f185.1587297254.git.martin.agren@gmail.com>
- In-Reply-To
- <cover.1587297254.git.martin.agren@gmail.com>
Most callers of `strbuf_attach()` provide `len + 1` for the `mem` parameter. That's a bit tedious and, as we will see in the next commit, also fairly prone to mistakes.
Provide `strbuf_attachstr_len()` for this common case to simplify several callers of `strbuf_attach()` by making this new function simply assume that the size of the allocated buffer is one greater than the length.
We know that the string has already been allocated with space for the trailing NUL, meaning it is safe to compute `len + 1`.
Disallow NULL-pointers entirely. We could handle `(buf, len) == (NULL, 0)` specially, but none of the callers we convert here seem to worry about such a case. Handling this corner case specially can still be done using the regular `strbuf_attach()`.
Another edge case is where someone doesn't have a NUL at `buf[len]`, i.e., they are (hopefully) trying to attach a substring of some larger string. One could argue that they should be using `strbuf_attach()` and that this is BUG-worthy, or that it would be easy enough for us to place a NUL there for robustness and carry on. This commit does the latter, but does not have a strong opinion about it.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- strbuf.h | 19 +++++++++++++++++++ apply.c | 2 +- archive.c | 2 +- blame.c | 2 +- convert.c | 4 ++-- imap-send.c | 2 +- merge-recursive.c | 2 +- pretty.c | 2 +- 8 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/strbuf.h b/strbuf.h index 2a462f70cc..7d0aeda434 100644 --- a/strbuf.h +++ b/strbuf.h @@ -121,6 +121,25 @@ char *strbuf_detach(struct strbuf *sb, size_t *sz); */ void strbuf_attach(struct strbuf *sb, void *str, size_t len, size_t mem); +/** + * Attach a string to a buffer. You should specify the string to attach + * and its length. + * + * The amount of allocated memory will be assumed to be one greater than + * its length. The string you pass _must_ be a NUL-terminated string. + * This string _must_ be malloc()ed, and after attaching, the pointer + * cannot be relied upon anymore, nor should it be free()d directly. + * + * Do _not_ use this to truncate the string. That is, the length really + * must be `len` already. To truncate (yet keeping track of the amount + * of allocated memory), use `strbuf_attach()`. + */ +static inline void strbuf_attachstr_len(struct strbuf *sb, + char *str, size_t len) +{ + strbuf_attach(sb, str, len, len + 1); +} + /** * Swap the contents of two string buffers. */ diff --git a/apply.c b/apply.c index 144c19aaca..cab4055ea4 100644 --- a/apply.c +++ b/apply.c @@ -3251,7 +3251,7 @@ static int read_blob_object(struct strbuf *buf, const struct object_id *oid, uns if (!result) return -1; /* XXX read_sha1_file NUL-terminates */ - strbuf_attach(buf, result, sz, sz + 1); + strbuf_attachstr_len(buf, result, sz); } return 0; } diff --git a/archive.c b/archive.c index fb39706120..17b8add930 100644 --- a/archive.c +++ b/archive.c @@ -89,7 +89,7 @@ void *object_file_to_archive(const struct archiver_args *args, struct strbuf buf = STRBUF_INIT; size_t size = 0; - strbuf_attach(&buf, buffer, *sizep, *sizep + 1); + strbuf_attachstr_len(&buf, buffer, *sizep); convert_to_working_tree(args->repo->index, path, buf.buf, buf.len, &buf, &meta); if (commit) format_subst(commit, buf.buf, buf.len, &buf); diff --git a/blame.c b/blame.c index 29770e5c81..12ce104fcb 100644 --- a/blame.c +++ b/blame.c @@ -241,7 +241,7 @@ static struct commit *fake_working_tree_commit(struct repository *r, case S_IFREG: if (opt->flags.allow_textconv && textconv_object(r, read_from, mode, &null_oid, 0, &buf_ptr, &buf_len)) - strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1); + strbuf_attachstr_len(&buf, buf_ptr, buf_len); else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size) die_errno("cannot open or read '%s'", read_from); break; diff --git a/convert.c b/convert.c index 5aa87d45e3..9b3a1218a7 100644 --- a/convert.c +++ b/convert.c @@ -467,7 +467,7 @@ static int encode_to_git(const char *path, const char *src, size_t src_len, free(re_src); } - strbuf_attach(buf, dst, dst_len, dst_len + 1); + strbuf_attachstr_len(buf, dst, dst_len); return 1; } @@ -492,7 +492,7 @@ static int encode_to_worktree(const char *path, const char *src, size_t src_len, return 0; } - strbuf_attach(buf, dst, dst_len, dst_len + 1); + strbuf_attachstr_len(buf, dst, dst_len); return 1; } diff --git a/imap-send.c b/imap-send.c index 6c54d8c29d..37e5b13e51 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1212,7 +1212,7 @@ static void lf_to_crlf(struct strbuf *msg) new_msg[j++] = '\r'; lastc = new_msg[j++] = msg->buf[i]; } - strbuf_attach(msg, new_msg, j, j + 1); + strbuf_attachstr_len(msg, new_msg, j); } /* diff --git a/merge-recursive.c b/merge-recursive.c index d92e2acf1e..ef259e4b74 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2963,7 +2963,7 @@ static int read_oid_strbuf(struct merge_options *opt, free(buf); return err(opt, _("object %s is not a blob"), oid_to_hex(oid)); } - strbuf_attach(dst, buf, size, size + 1); + strbuf_attachstr_len(dst, buf, size); return 0; } diff --git a/pretty.c b/pretty.c index 28afc701b6..e171830389 100644 --- a/pretty.c +++ b/pretty.c @@ -1687,7 +1687,7 @@ void repo_format_commit_message(struct repository *r, char *out = reencode_string_len(sb->buf, sb->len, output_enc, utf8, &outsz); if (out) - strbuf_attach(sb, out, outsz, outsz + 1); + strbuf_attachstr_len(sb, out, outsz); } free(context.commit_encoding);
-- 2.26.1