[multiple] reduce redundant NULL buffer checks

This commit is a large set of code changes and results in removal of
hundreds, perhaps thousands, of CPU instructions, a portion of which
are on hot code paths.

Most (buffer *) used by lighttpd are not NULL, especially since buffers
were inlined into numerous larger structs such as request_st and chunk.

In the small number of instances where that is not the case, a NULL
check is often performed earlier in a function where that buffer is
later used with a buffer_* func.  In the handful of cases that remained,
a NULL check was added, e.g. with r->http_host and r->conf.server_tag.

- check for empty strings at config time and set value to NULL if blank
  string will be ignored at runtime; at runtime, simple pointer check
  for NULL can be used to check for a value that has been set and is not
  blank ("")
- use buffer_is_blank() instead of buffer_string_is_empty(),
  and use buffer_is_unset() instead of buffer_is_empty(),
  where buffer is known not to be NULL so that NULL check can be skipped
- use buffer_clen() instead of buffer_string_length() when buffer is
  known not to be NULL (to avoid NULL check at runtime)
- use buffer_truncate() instead of buffer_string_set_length() to
  truncate string, and use buffer_extend() to extend

Examples where buffer known not to be NULL:
  - cpv->v.b from config_plugin_values_init is not NULL if T_CONFIG_BOOL
    (though we might set it to NULL if buffer_is_blank(cpv->v.b))
  - address of buffer is arg (&foo)
    (compiler optimizer detects this in most, but not all, cases)
  - buffer is checked for NULL earlier in func
  - buffer is accessed in same scope without a NULL check (e.g. b->ptr)

internal behavior change:
  callers must not pass a NULL buffer to some funcs.
  - buffer_init_buffer() requires non-null args
  - buffer_copy_buffer() requires non-null args
  - buffer_append_string_buffer() requires non-null args
  - buffer_string_space() requires non-null arg

1 files changed, 111 insertions, 59 deletions

diff --git a/src/buffer.h b/src/buffer.h
index a9a4c574..93c8ac19 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -13,13 +13,11 @@ struct tm;              /* declaration */
 /* generic string + binary data container; contains a terminating 0 in both
  * cases
  *
- * used == 0 indicates a special "empty" state (unset config values); ptr
- * might be NULL too then. otherwise an empty string has used == 1 (and ptr[0]
- * == 0);
+ * used == 0 indicates a special "empty" state (unset config values);
+ * ptr might be NULL, too.
  *
- * copy/append functions will ensure used >= 1 (i.e. never leave it in the
- * special empty state); only buffer_copy_buffer will copy the special empty
- * state.
+ * copy/append functions will ensure used >= 1
+ * (i.e. never leave it in the special empty state)
  */
 typedef struct {
 	char *ptr;
@@ -34,8 +32,9 @@ typedef struct {
 __attribute_returns_nonnull__
 buffer* buffer_init(void);
 
+__attribute_nonnull__
 __attribute_returns_nonnull__
-buffer* buffer_init_buffer(const buffer *src); /* src can  be NULL */
+buffer* buffer_init_buffer(const buffer *src);
 
 __attribute_returns_nonnull__
 buffer* buffer_init_string(const char *str); /* str can  be NULL */
@@ -81,14 +80,6 @@ char* buffer_extend(buffer * const restrict b, size_t x);
 __attribute_nonnull__
 void buffer_commit(buffer *b, size_t size);
 
-/* sets string length:
- * - always stores a terminating zero to terminate the "new" string
- * - does not modify the string data apart from terminating zero
- * - reallocates the buffer iff needed
- */
-__attribute_nonnull__
-void buffer_string_set_length(buffer *b, uint32_t len);
-
 /* clear buffer
  * - invalidate buffer contents
  * - unsets used chars but does not modify existing ptr contents
@@ -116,13 +107,11 @@ void buffer_free_ptr(buffer *b);
 
 void buffer_copy_string(buffer * restrict b, const char * restrict s);
 void buffer_copy_string_len(buffer * restrict b, const char * restrict s, size_t len);
-static inline void buffer_copy_buffer(buffer * restrict b, const buffer * restrict src);
 
 void buffer_append_string(buffer * restrict b, const char * restrict s);
 void buffer_append_string_len(buffer * restrict b, const char * restrict s, size_t len);
 void buffer_append_str2(buffer * restrict b, const char *s1, size_t len1, const char *s2, size_t len2);
 void buffer_append_str3(buffer * restrict b, const char *s1, size_t len1, const char *s2, size_t len2, const char *s3, size_t len3);
-static inline void buffer_append_string_buffer(buffer * restrict b, const buffer * restrict src);
 
 struct const_iovec {
   const void *iov_base;
@@ -155,16 +144,6 @@ void li_tohex_lc(char * restrict buf, size_t buf_len, const char * restrict s, s
 __attribute_nonnull__
 void li_tohex_uc(char * restrict buf, size_t buf_len, const char * restrict s, size_t s_len);
 
-/* NULL buffer or empty buffer (used == 0);
- * unset "string" (buffer) config options are initialized to used == 0,
- * while setting an empty string leads to used == 1
- */
-__attribute_pure__
-static inline int buffer_is_empty(const buffer *b);
-/* NULL buffer, empty buffer (used == 0) or empty string (used == 1) */
-__attribute_pure__
-static inline int buffer_string_is_empty(const buffer *b);
-
 __attribute_nonnull__
 __attribute_pure__
 int buffer_eq_icase_ssn(const char * const a, const char * const b, const size_t len);
@@ -176,12 +155,10 @@ int buffer_eq_icase_ss(const char * const a, const size_t alen, const char * con
 __attribute_nonnull__
 __attribute_pure__
 int buffer_eq_icase_slen(const buffer * const b, const char * const s, const size_t slen);
-#define buffer_is_equal_caseless_string buffer_eq_icase_slen
 
 __attribute_nonnull__
 __attribute_pure__
 int buffer_eq_slen(const buffer * const b, const char * const s, const size_t slen);
-#define buffer_is_equal_string buffer_eq_slen
 
 __attribute_nonnull__
 __attribute_pure__
@@ -263,16 +240,6 @@ static inline int light_isalnum(int c) {
 #define light_bset(a,b)  ((a) |=  ((uint64_t)1uL << (b)))
 
 
-__attribute_pure__
-static inline uint32_t buffer_string_length(const buffer *b); /* buffer string length without terminating 0 */
-
-__attribute_nonnull__
-__attribute_pure__
-static inline uint32_t buffer_string_space(const buffer *b); /* maximum length of string that can be stored without reallocating */
-
-__attribute_nonnull__
-static inline void buffer_append_slash(buffer *b); /* append '/' no non-empty strings not ending in '/' */
-
 void buffer_append_path_len(buffer * restrict b, const char * restrict a, size_t alen); /* join strings with '/', if '/' not present */
 void buffer_copy_path_len2(buffer * restrict b, const char * restrict s1, size_t len1, const char * restrict s2, size_t len2);
 
@@ -284,17 +251,11 @@ __attribute_nonnull__
 __attribute_pure__
 static inline int buffer_has_pathsep_suffix (const buffer * const b);
 
-#define BUFFER_APPEND_STRING_CONST(x, y) \
-	buffer_append_string_len(x, y, sizeof(y) - 1)
-
-#define BUFFER_COPY_STRING_CONST(x, y) \
-	buffer_copy_string_len(x, y, sizeof(y) - 1)
-
-#define BUFFER_INTLEN_PTR(x) (x)->used ? (int)((x)->used - 1) : 0, (x)->ptr
+#define BUFFER_INTLEN_PTR(x) (int)buffer_clen(x), (x)->ptr
+#define BUF_PTR_LEN(x)       (x)->ptr, buffer_clen(x)
 
 #define CONST_LEN_STR(x) (uint32_t)sizeof(x)-1, x
 #define CONST_STR_LEN(x) x, (uint32_t)sizeof(x) - 1
-#define CONST_BUF_LEN(x) ((x) ? (x)->ptr : NULL), buffer_string_length(x)
 
 
 #define LI_NORETURN __attribute_noreturn__
@@ -304,34 +265,71 @@ void log_failed_assert(const char *filename, unsigned int line, const char *msg)
 #define force_assert(x) do { if (!(x)) log_failed_assert(__FILE__, __LINE__, "assertion failed: " #x); } while(0)
 #define SEGFAULT() log_failed_assert(__FILE__, __LINE__, "aborted");
 
+
 /* inline implementations */
 
-static inline int buffer_is_empty(const buffer *b) {
-	return NULL == b || 0 == b->used;
+__attribute_nonnull__
+__attribute_pure__
+static inline int buffer_is_unset(const buffer *b);
+static inline int buffer_is_unset(const buffer *b) {
+    return 0 == b->used;
 }
-static inline int buffer_string_is_empty(const buffer *b) {
-	return NULL == b || b->used < 2;
+
+__attribute_nonnull__
+__attribute_pure__
+static inline int buffer_is_blank(const buffer *b);
+static inline int buffer_is_blank(const buffer *b) {
+    return b->used < 2; /* buffer_is_blank() || buffer_is_unset() */
 }
 
-static inline uint32_t buffer_string_length(const buffer *b) {
-	return NULL != b && 0 != b->used ? b->used - 1 : 0;
+/* buffer "C" len (bytes) */
+__attribute_nonnull__
+__attribute_pure__
+static inline uint32_t buffer_clen (const buffer *b);
+static inline uint32_t buffer_clen (const buffer *b) {
+    return b->used - (0 != b->used);
 }
 
+/* buffer space remaining to append string without reallocating */
+__attribute_nonnull__
+__attribute_pure__
+static inline uint32_t buffer_string_space(const buffer *b);
 static inline uint32_t buffer_string_space(const buffer *b) {
-	return b->size ? b->size - (b->used | (0 == b->used)) : 0;
+    return b->size ? b->size - (b->used | (0 == b->used)) : 0;
 }
 
+__attribute_nonnull__
+static inline void buffer_copy_buffer(buffer * restrict b, const buffer * restrict src);
 static inline void buffer_copy_buffer(buffer * restrict b, const buffer * restrict src) {
-	buffer_copy_string_len(b, CONST_BUF_LEN(src));
+    buffer_copy_string_len(b, BUF_PTR_LEN(src));
 }
 
-static inline void buffer_append_string_buffer(buffer * restrict b, const buffer * restrict src) {
-	buffer_append_string_len(b, CONST_BUF_LEN(src));
+__attribute_nonnull__
+static inline void buffer_append_buffer(buffer * restrict b, const buffer * restrict src);
+static inline void buffer_append_buffer(buffer * restrict b, const buffer * restrict src) {
+    buffer_append_string_len(b, BUF_PTR_LEN(src));
 }
 
+__attribute_nonnull__
+static inline void buffer_truncate(buffer *b, uint32_t len);
+static inline void buffer_truncate(buffer *b, uint32_t len) {
+    b->ptr[len] = '\0'; /* b->ptr must exist; use buffer_blank() for trunc 0 */
+    b->used = len + 1;
+}
+
+__attribute_nonnull__
+static inline void buffer_blank(buffer *b);
+static inline void buffer_blank(buffer *b) {
+    b->ptr ? buffer_truncate(b, 0) : (void)buffer_extend(b, 0);
+}
+
+/* append '/' to non-empty strings not ending in '/' */
+__attribute_nonnull__
+static inline void buffer_append_slash(buffer *b);
 static inline void buffer_append_slash(buffer *b) {
-	uint32_t len = buffer_string_length(b);
-	if (len > 0 && '/' != b->ptr[len-1]) BUFFER_APPEND_STRING_CONST(b, "/");
+    const uint32_t len = buffer_clen(b);
+    if (len > 0 && '/' != b->ptr[len-1])
+        buffer_append_string_len(b, CONST_STR_LEN("/"));
 }
 
 static inline void buffer_clear(buffer *b) {
@@ -352,4 +350,58 @@ static inline int buffer_has_pathsep_suffix (const buffer * const b) {
     return (b->used > 1 && b->ptr[b->used-2] == '/');
 }
 
+
+/* backwards compat (deprecated; older interfaces) */
+
+#define buffer_append_string_buffer buffer_append_buffer
+#define buffer_is_equal_caseless_string buffer_eq_icase_slen
+#define buffer_is_equal_string buffer_eq_slen
+
+#define BUFFER_APPEND_STRING_CONST(x, y) \
+	buffer_append_string_len(x, y, sizeof(y) - 1)
+
+#define BUFFER_COPY_STRING_CONST(x, y) \
+	buffer_copy_string_len(x, y, sizeof(y) - 1)
+
+#define CONST_BUF_LEN(x) ((x) ? (x)->ptr : NULL), buffer_string_length(x)
+
+/* NULL buffer or empty buffer (used == 0);
+ * unset "string" (buffer) config options are initialized to used == 0,
+ * while setting an empty string leads to used == 1
+ */
+__attribute_pure__
+static inline int buffer_is_empty(const buffer *b);
+static inline int buffer_is_empty(const buffer *b) {
+	return NULL == b || buffer_is_unset(b);
+}
+/* NULL buffer, empty buffer (used == 0) or empty string (used == 1) */
+__attribute_pure__
+static inline int buffer_string_is_empty(const buffer *b);
+static inline int buffer_string_is_empty(const buffer *b) {
+	return NULL == b || buffer_is_blank(b);
+}
+
+/* buffer string length without terminating 0 */
+__attribute_pure__
+static inline uint32_t buffer_string_length(const buffer *b);
+static inline uint32_t buffer_string_length(const buffer *b) {
+	return NULL != b ? buffer_clen(b) : 0;
+}
+
+/* sets string length:
+ * - deprecated; use buffer_truncate() or buffer_extend() instead
+ * - always stores a terminating zero to terminate the "new" string
+ * - does not modify the string data apart from terminating zero
+ * - reallocates the buffer iff needed
+ */
+__attribute_nonnull__
+static inline void buffer_string_set_length(buffer *b, uint32_t len);
+static inline void buffer_string_set_length(buffer *b, uint32_t len) {
+    if (len < b->size)
+        buffer_truncate(b, len);
+    else
+        buffer_extend(b, len - buffer_clen(b));
+}
+
+
 #endif