[PATCH 1/6] strbuf: add header for 'safe' API

["Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>]

From: Derrick Stolee <stolee@gmail.com>

The strbuf library is an important API used all over the Git codebase.
Contributors use it in nearly any string-manipulating action. However, the
implementation uses other helping functions that die() on failure instead of
returning an error code. Thus, the strbuf API isn't _safe_.

In particular, we cannot include 'banned-die.h' in 'strbuf.c'.

To start the creation of a safe strbuf API, move the struct definition into
a new 'strbuf-safe.h' header file. All consumers of 'strbuf.h' will consume
that header transitively.

In the future, we will hope to have consumers that need a 'safe' API will
include 'strbuf-safe.h' instead of 'strbuf.h'.

We will see in future changes the inclusion of new implementations that
return an error code instead of halting.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 strbuf-safe.h | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++
 strbuf.h      | 74 +++----------------------------------------
 2 files changed, 92 insertions(+), 70 deletions(-)
 create mode 100644 strbuf-safe.h

diff --git a/strbuf-safe.h b/strbuf-safe.h
new file mode 100644
index 0000000000..3cf14545bb
--- /dev/null
+++ b/strbuf-safe.h
@@ -0,0 +1,88 @@
+#ifndef STRBUF_SAFE_H
+#define STRBUF_SAFE_H
+
+/*
+ * NOTE FOR STRBUF DEVELOPERS
+ *
+ * strbuf is a low-level primitive; as such it should interact only
+ * with other low-level primitives. Do not introduce new functions
+ * which interact with higher-level APIs.
+ *
+ * This header file specifically conatins the "safe" API surface for
+ * working with strbufs. The implementations of these methods avoid
+ * using die() and other exits. Thus, these methods are appropriate
+ * for use within lower-level APIs such as trace2.
+ */
+
+struct string_list;
+
+/**
+ * strbufs are meant to be used with all the usual C string and memory
+ * APIs. Given that the length of the buffer is known, it's often better to
+ * use the mem* functions than a str* one (e.g., memchr vs. strchr).
+ * Though, one has to be careful about the fact that str* functions often
+ * stop on NULs and that strbufs may have embedded NULs.
+ *
+ * A strbuf is NUL terminated for convenience, but no function in the
+ * strbuf API actually relies on the string being free of NULs.
+ *
+ * strbufs have some invariants that are very important to keep in mind:
+ *
+ *  - The `buf` member is never NULL, so it can be used in any usual C
+ *    string operations safely. strbufs _have_ to be initialized either by
+ *    `strbuf_init()` or by `= STRBUF_INIT` before the invariants, though.
+ *
+ *    Do *not* assume anything on what `buf` really is (e.g. if it is
+ *    allocated memory or not), use `strbuf_detach()` to unwrap a memory
+ *    buffer from its strbuf shell in a safe way. That is the sole supported
+ *    way. This will give you a malloced buffer that you can later `free()`.
+ *
+ *    However, it is totally safe to modify anything in the string pointed by
+ *    the `buf` member, between the indices `0` and `len-1` (inclusive).
+ *
+ *  - The `buf` member is a byte array that has at least `len + 1` bytes
+ *    allocated. The extra byte is used to store a `'\0'`, allowing the
+ *    `buf` member to be a valid C-string. All strbuf functions ensure this
+ *    invariant is preserved.
+ *
+ *    NOTE: It is OK to "play" with the buffer directly if you work it this
+ *    way:
+ *
+ *        strbuf_grow(sb, SOME_SIZE); <1>
+ *        strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
+ *
+ *    <1> Here, the memory array starting at `sb->buf`, and of length
+ *    `strbuf_avail(sb)` is all yours, and you can be sure that
+ *    `strbuf_avail(sb)` is at least `SOME_SIZE`.
+ *
+ *    NOTE: `SOME_OTHER_SIZE` must be smaller or equal to `strbuf_avail(sb)`.
+ *
+ *    Doing so is safe, though if it has to be done in many places, adding the
+ *    missing API to the strbuf module is the way to go.
+ *
+ *    WARNING: Do _not_ assume that the area that is yours is of size `alloc
+ *    - 1` even if it's true in the current implementation. Alloc is somehow a
+ *    "private" member that should not be messed with. Use `strbuf_avail()`
+ *    instead.
+*/
+
+/**
+ * Data Structures
+ * ---------------
+ */
+
+/**
+ * This is the string buffer structure. The `len` member can be used to
+ * determine the current length of the string, and `buf` member provides
+ * access to the string itself.
+ */
+struct strbuf {
+	size_t alloc;
+	size_t len;
+	char *buf;
+};
+
+extern char strbuf_slopbuf[];
+#define STRBUF_INIT  { .buf = strbuf_slopbuf }
+
+#endif /* STRBUF_SAFE_H */
diff --git a/strbuf.h b/strbuf.h
index 1089ae687b..b41f8ef901 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -1,85 +1,19 @@
 #ifndef STRBUF_H
 #define STRBUF_H
 
+#include "strbuf-safe.h"
+
 /*
  * NOTE FOR STRBUF DEVELOPERS
  *
  * strbuf is a low-level primitive; as such it should interact only
  * with other low-level primitives. Do not introduce new functions
  * which interact with higher-level APIs.
- */
-
-struct string_list;
-
-/**
- * strbufs are meant to be used with all the usual C string and memory
- * APIs. Given that the length of the buffer is known, it's often better to
- * use the mem* functions than a str* one (e.g., memchr vs. strchr).
- * Though, one has to be careful about the fact that str* functions often
- * stop on NULs and that strbufs may have embedded NULs.
- *
- * A strbuf is NUL terminated for convenience, but no function in the
- * strbuf API actually relies on the string being free of NULs.
- *
- * strbufs have some invariants that are very important to keep in mind:
- *
- *  - The `buf` member is never NULL, so it can be used in any usual C
- *    string operations safely. strbufs _have_ to be initialized either by
- *    `strbuf_init()` or by `= STRBUF_INIT` before the invariants, though.
- *
- *    Do *not* assume anything on what `buf` really is (e.g. if it is
- *    allocated memory or not), use `strbuf_detach()` to unwrap a memory
- *    buffer from its strbuf shell in a safe way. That is the sole supported
- *    way. This will give you a malloced buffer that you can later `free()`.
- *
- *    However, it is totally safe to modify anything in the string pointed by
- *    the `buf` member, between the indices `0` and `len-1` (inclusive).
- *
- *  - The `buf` member is a byte array that has at least `len + 1` bytes
- *    allocated. The extra byte is used to store a `'\0'`, allowing the
- *    `buf` member to be a valid C-string. All strbuf functions ensure this
- *    invariant is preserved.
- *
- *    NOTE: It is OK to "play" with the buffer directly if you work it this
- *    way:
  *
- *        strbuf_grow(sb, SOME_SIZE); <1>
- *        strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
- *
- *    <1> Here, the memory array starting at `sb->buf`, and of length
- *    `strbuf_avail(sb)` is all yours, and you can be sure that
- *    `strbuf_avail(sb)` is at least `SOME_SIZE`.
- *
- *    NOTE: `SOME_OTHER_SIZE` must be smaller or equal to `strbuf_avail(sb)`.
- *
- *    Doing so is safe, though if it has to be done in many places, adding the
- *    missing API to the strbuf module is the way to go.
- *
- *    WARNING: Do _not_ assume that the area that is yours is of size `alloc
- *    - 1` even if it's true in the current implementation. Alloc is somehow a
- *    "private" member that should not be messed with. Use `strbuf_avail()`
- *    instead.
-*/
-
-/**
- * Data Structures
- * ---------------
+ * Also see strbuf-safe.h for the struct definitions and safe versions
+ * of some methods declared in this header file.
  */
 
-/**
- * This is the string buffer structure. The `len` member can be used to
- * determine the current length of the string, and `buf` member provides
- * access to the string itself.
- */
-struct strbuf {
-	size_t alloc;
-	size_t len;
-	char *buf;
-};
-
-extern char strbuf_slopbuf[];
-#define STRBUF_INIT  { .buf = strbuf_slopbuf }
-
 struct object_id;
 
 /**
-- 
gitgitgadget