@@ -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.
*/
@@ -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;
}
@@ -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);
@@ -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;
@@ -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;
}
@@ -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);
}
/*
@@ -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;
}
@@ -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);
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(-)