common: remove buf_init(), add BUF_INIT and buf_move()

Add a BUF_INIT macro, which makes it easier to initialize a struct buf
to an empty string (without a heap allocation).

Add buf_move() to move the contents of one struct buf to another (the
source is reset to BUF_INIT, analogous to C++ move-assignment).

Use buf_reset() instead of directly calling `free(s->buf)` since the
internal buf may not always be allocated by malloc() now.

include/common/buf.h

@@ -8,16 +8,28 @@
 #ifndef LABWC_BUF_H
 #define LABWC_BUF_H
 
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
 struct buf {
+	/**
+	 * Pointer to underlying string buffer. If alloc != 0, then
+	 * this was allocated via malloc().
+	 */
 	char *buf;
+	/**
+	 * Allocated length of buf. If zero, buf was not allocated
+	 * (either NULL or literal empty string).
+	 */
 	int alloc;
+	/**
+	 * Length of string contents (not including terminating NUL).
+	 * Currently this must be zero if alloc is zero (i.e. non-empty
+	 * literal strings are not allowed).
+	 */
 	int len;
 };
 
+/** Value used to initialize a struct buf to an empty string */
+#define BUF_INIT ((struct buf){.buf = ""})
+
 /**
  * buf_expand_tilde - expand ~ in buffer
  * @s: buffer
@@ -31,13 +43,6 @@ void buf_expand_tilde(struct buf *s);
  */
 void buf_expand_shell_variables(struct buf *s);
 
-/**
- * buf_init - allocate NULL-terminated C string buffer
- * @s: buffer
- * Note: use free(s->buf) to free it
- */
-void buf_init(struct buf *s);
-
 /**
  * buf_add - add data to C string buffer
  * @s: buffer
@@ -55,13 +60,35 @@ void buf_add_char(struct buf *s, char data);
 /**
  * buf_clear - clear the buffer, internal allocations are preserved
  * @s: buffer
+ *
+ * The buffer will be set to a NUL-terminated empty string.
+ *
+ * This is the appropriate function to call to re-use the buffer
+ * in a loop or similar situations as it reuses the existing heap
+ * allocation.
  */
 void buf_clear(struct buf *s);
 
 /**
  * buf_reset - reset the buffer, internal allocations are free'd
  * @s: buffer
+ *
+ * The buffer will be re-initialized to BUF_INIT (empty string).
+ *
+ * Inside a loop, consider using buf_clear() instead, as it allows
+ * reusing the existing heap allocation. buf_reset() should still be
+ * called after exiting the loop.
  */
 void buf_reset(struct buf *s);
 
+/**
+ * buf_move - move the contents of src to dst, freeing any previous
+ * allocation of dst and resetting src to BUF_INIT.
+ *
+ * dst must either have been initialized with BUF_INIT
+ * or zeroed out (e.g. created by znew() or on the stack
+ * with something like struct buf foo = {0}).
+ */
+void buf_move(struct buf *dst, struct buf *src);
+
 #endif /* LABWC_BUF_H */

include/common/grab-file.h

@@ -8,11 +8,13 @@
 #ifndef LABWC_GRAB_FILE_H
 #define LABWC_GRAB_FILE_H
 
+#include "common/buf.h"
+
 /**
  * grab_file - read file into memory buffer
  * @filename: file to read
- * Returns pointer to buffer. Free with free().
+ * Free returned buffer with buf_reset().
  */
-char *grab_file(const char *filename);
+struct buf grab_file(const char *filename);
 
 #endif /* LABWC_GRAB_FILE_H */

src/action.c

@@ -706,12 +706,11 @@ actions_run(struct view *activator, struct server *server,
 			break;
 		case ACTION_TYPE_EXECUTE:
 			{
-				struct buf cmd;
+				struct buf cmd = BUF_INIT;
-				buf_init(&cmd);
 				buf_add(&cmd, action_get_str(action, "command", NULL));
 				buf_expand_tilde(&cmd);
 				spawn_async_no_shell(cmd.buf);
-				free(cmd.buf);
+				buf_reset(&cmd);
 			}
 			break;
 		case ACTION_TYPE_EXIT:

src/button/button-xbm.c

@@ -288,15 +288,15 @@ button_xbm_load(const char *button_name, struct lab_data_buffer **buffer,
 	/* Read file into memory as it's easier to tokenize that way */
 	char filename[4096] = { 0 };
 	button_filename(button_name, filename, sizeof(filename));
-	char *token_buffer = grab_file(filename);
+	struct buf token_buf = grab_file(filename);
-	if (token_buffer) {
+	if (token_buf.len) {
-		struct token *tokens = tokenize_xbm(token_buffer);
+		struct token *tokens = tokenize_xbm(token_buf.buf);
-		free(token_buffer);
 		pixmap = parse_xbm_tokens(tokens);
 		if (tokens) {
 			free(tokens);
 		}
 	}
+	buf_reset(&token_buf);
 	if (!pixmap.data) {
 		return;
 	}

src/common/buf.c

@@ -2,14 +2,15 @@
 #include <assert.h>
 #include <ctype.h>
 #include <stdbool.h>
+#include <string.h>
 #include "common/buf.h"
+#include "common/macros.h"
 #include "common/mem.h"
 
 void
 buf_expand_tilde(struct buf *s)
 {
-	struct buf new;
+	struct buf new = BUF_INIT;
-	buf_init(&new);
 	for (int i = 0 ; i < s->len ; i++) {
 		if (s->buf[i] == '~') {
 			buf_add(&new, getenv("HOME"));
@@ -17,10 +18,7 @@ buf_expand_tilde(struct buf *s)
 			buf_add_char(&new, s->buf[i]);
 		}
 	}
-	free(s->buf);
+	buf_move(s, &new);
-	s->buf = new.buf;
-	s->len = new.len;
-	s->alloc = new.alloc;
 }
 
 static void
@@ -44,10 +42,8 @@ isvalid(char p)
 void
 buf_expand_shell_variables(struct buf *s)
 {
-	struct buf new;
+	struct buf new = BUF_INIT;
-	struct buf environment_variable;
+	struct buf environment_variable = BUF_INIT;
-	buf_init(&new);
-	buf_init(&environment_variable);
 
 	for (int i = 0 ; i < s->len ; i++) {
 		if (s->buf[i] == '$' && isvalid(s->buf[i+1])) {
@@ -69,37 +65,43 @@ buf_expand_shell_variables(struct buf *s)
 			buf_add_char(&new, s->buf[i]);
 		}
 	}
-	free(environment_variable.buf);
+	buf_reset(&environment_variable);
-	free(s->buf);
+	buf_move(s, &new);
-	s->buf = new.buf;
-	s->len = new.len;
-	s->alloc = new.alloc;
 }
 
-void
+static void
-buf_init(struct buf *s)
+buf_expand(struct buf *s, int new_alloc)
 {
-	/* we can't assert(!s->buf) here because struct may be uninitialized */
+	/*
-
+	 * "s->alloc &&" ensures that s->data is always allocated after
-	s->alloc = 256;
+	 * returning (even if new_alloc == 0). The extra check is not
-	s->buf = xmalloc(s->alloc);
+	 * really necessary but makes it easier for analyzers to see
-	s->buf[0] = '\0';
+	 * that we never overwrite a string literal.
-	s->len = 0;
+	 */
+	if (s->alloc && new_alloc <= s->alloc) {
+		return;
+	}
+	new_alloc = MAX(new_alloc, 256);
+	new_alloc = MAX(new_alloc, s->alloc * 3 / 2);
+	if (s->alloc) {
+		assert(s->buf);
+		s->buf = xrealloc(s->buf, new_alloc);
+	} else {
+		assert(!s->len);
+		s->buf = xmalloc(new_alloc);
+		s->buf[0] = '\0';
+	}
+	s->alloc = new_alloc;
 }
 
 void
 buf_add(struct buf *s, const char *data)
 {
-	assert(s->buf);
-
 	if (!data || data[0] == '\0') {
 		return;
 	}
 	int len = strlen(data);
-	if (s->alloc <= s->len + len + 1) {
+	buf_expand(s, s->len + len + 1);
-		s->alloc = s->alloc + len;
-		s->buf = xrealloc(s->buf, s->alloc);
-	}
 	memcpy(s->buf + s->len, data, len);
 	s->len += len;
 	s->buf[s->len] = 0;
@@ -108,12 +110,7 @@ buf_add(struct buf *s, const char *data)
 void
 buf_add_char(struct buf *s, char ch)
 {
-	assert(s->buf);
+	buf_expand(s, s->len + 1);
-
-	if (s->alloc <= s->len + 1) {
-		s->alloc = s->alloc * 3 / 2 + 16;
-		s->buf = xrealloc(s->buf, s->alloc);
-	}
 	s->buf[s->len++] = ch;
 	s->buf[s->len] = '\0';
 }
@@ -121,15 +118,30 @@ buf_add_char(struct buf *s, char ch)
 void
 buf_clear(struct buf *s)
 {
-	assert(s->buf);
+	if (s->alloc) {
-
+		assert(s->buf);
-	s->len = 0;
+		s->len = 0;
-	s->buf[0] = '\0';
+		s->buf[0] = '\0';
+	} else {
+		*s = BUF_INIT;
+	}
 }
 
 void
 buf_reset(struct buf *s)
 {
-	zfree(s->buf);
+	if (s->alloc) {
-	buf_init(s);
+		free(s->buf);
+	}
+	*s = BUF_INIT;
+}
+
+void
+buf_move(struct buf *dst, struct buf *src)
+{
+	if (dst->alloc) {
+		free(dst->buf);
+	}
+	*dst = *src;
+	*src = BUF_INIT;
 }

src/common/dir.c

@@ -87,10 +87,10 @@ find_dir(struct ctx *ctx)
 {
 	char *debug = getenv("LABWC_DEBUG_DIR_CONFIG_AND_THEME");
 
+	struct buf prefix = BUF_INIT;
 	for (int i = 0; ctx->dirs[i].path; i++) {
 		struct dir d = ctx->dirs[i];
-		struct buf prefix;
+		buf_clear(&prefix);
-		buf_init(&prefix);
 
 		/*
 		 * Replace (rather than augment) $HOME/.config with
@@ -100,7 +100,6 @@ find_dir(struct ctx *ctx)
 		char *pfxenv = getenv(d.prefix);
 		buf_add(&prefix, pfxenv ? pfxenv : d.default_prefix);
 		if (!prefix.len) {
-			free(prefix.buf);
 			continue;
 		}
 
@@ -130,8 +129,8 @@ find_dir(struct ctx *ctx)
 			wl_list_append(ctx->list, &path->link);
 		}
 		g_strfreev(prefixes);
-		free(prefix.buf);
 	}
+	buf_reset(&prefix);
 }
 
 void

src/common/grab-file.c

@@ -10,18 +10,19 @@
 #include "common/buf.h"
 
 #include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
 
-char *
+struct buf
 grab_file(const char *filename)
 {
 	char *line = NULL;
 	size_t len = 0;
 	FILE *stream = fopen(filename, "r");
 	if (!stream) {
-		return NULL;
+		return BUF_INIT;
 	}
-	struct buf buffer;
+	struct buf buffer = BUF_INIT;
-	buf_init(&buffer);
 	while ((getline(&line, &len, stream) != -1)) {
 		char *p = strrchr(line, '\n');
 		if (p) {
@@ -31,5 +32,5 @@ grab_file(const char *filename)
 	}
 	free(line);
 	fclose(stream);
-	return buffer.buf;
+	return buffer;
 }

src/config/rcxml.c

@@ -1530,8 +1530,6 @@ rcxml_read(const char *filename)
 	}
 
 	/* Reading file into buffer before parsing - better for unit tests */
-	struct buf b;
-
 	bool should_merge_config = rc.merge_config;
 	struct wl_list *(*iter)(struct wl_list *list);
 	iter = should_merge_config ? paths_get_prev : paths_get_next;
@@ -1555,7 +1553,7 @@ rcxml_read(const char *filename)
 
 		wlr_log(WLR_INFO, "read config file %s", path->string);
 
-		buf_init(&b);
+		struct buf b = BUF_INIT;
 		char *line = NULL;
 		size_t len = 0;
 		while (getline(&line, &len, stream) != -1) {
@@ -1568,7 +1566,7 @@ rcxml_read(const char *filename)
 		zfree(line);
 		fclose(stream);
 		rcxml_parse_xml(&b);
-		zfree(b.buf);
+		buf_reset(&b);
 		if (!should_merge_config) {
 			break;
 		}

src/config/session.c

@@ -49,13 +49,12 @@ process_line(char *line)
 		return;
 	}
 
-	struct buf value;
+	struct buf value = BUF_INIT;
-	buf_init(&value);
 	buf_add(&value, string_strip(++p));
 	buf_expand_shell_variables(&value);
 	buf_expand_tilde(&value);
 	setenv(key, value.buf, 1);
-	free(value.buf);
+	buf_reset(&value);
 }
 
 /* return true on successful read */

src/menu/menu.c

@@ -15,7 +15,6 @@
 #include "common/dir.h"
 #include "common/font.h"
 #include "common/list.h"
-#include "common/match.h"
 #include "common/mem.h"
 #include "common/nodename.h"
 #include "common/scaled_font_buffer.h"
@@ -639,9 +638,8 @@ parse_stream(struct server *server, FILE *stream)
 {
 	char *line = NULL;
 	size_t len = 0;
-	struct buf b;
+	struct buf b = BUF_INIT;
 
-	buf_init(&b);
 	while (getline(&line, &len, stream) != -1) {
 		char *p = strrchr(line, '\n');
 		if (p) {
@@ -651,7 +649,7 @@ parse_stream(struct server *server, FILE *stream)
 	}
 	free(line);
 	parse_buf(server, &b);
-	free(b.buf);
+	buf_reset(&b);
 }
 
 static void
@@ -1140,7 +1138,7 @@ pipemenu_ctx_destroy(struct pipe_context *ctx)
 	wl_event_source_remove(ctx->event_read);
 	wl_event_source_remove(ctx->event_timeout);
 	spawn_piped_close(ctx->pid, ctx->pipe_fd);
-	free(ctx->buf.buf);
+	buf_reset(&ctx->buf);
 	free(ctx);
 	waiting_for_pipe_menu = false;
 }
@@ -1230,7 +1228,7 @@ parse_pipemenu(struct menuitem *item)
 	ctx->item = item;
 	ctx->pid = pid;
 	ctx->pipe_fd = pipe_fd;
-	buf_init(&ctx->buf);
+	ctx->buf = BUF_INIT;
 
 	ctx->event_read = wl_event_loop_add_fd(ctx->server->wl_event_loop,
 		pipe_fd, WL_EVENT_READABLE, handle_pipemenu_readable, ctx);

src/osd.c

@@ -251,8 +251,7 @@ render_osd(struct server *server, cairo_t *cairo, int w, int h,
 	}
 	pango_font_description_free(desc);
 
-	struct buf buf;
+	struct buf buf = BUF_INIT;
-	buf_init(&buf);
 
 	/* This is the width of the area available for text fields */
 	int available_width = w - 2 * theme->osd_border_width
@@ -319,7 +318,7 @@ render_osd(struct server *server, cairo_t *cairo, int w, int h,
 
 		y += theme->osd_window_switcher_item_height;
 	}
-	free(buf.buf);
+	buf_reset(&buf);
 	g_object_unref(layout);
 
 	cairo_surface_flush(surf);

src/osd_field.c

@@ -189,9 +189,7 @@ field_set_custom(struct buf *buf, struct view *view, const char *format)
 	char fmt[LAB_FIELD_SINGLE_FMT_MAX_LEN];
 	unsigned char fmt_position = 0;
 
-	struct buf field_result;
+	struct buf field_result = BUF_INIT;
-	buf_init(&field_result);
-
 	char converted_field[4096];
 
 	for (const char *p = format; *p; p++) {
@@ -255,7 +253,7 @@ reset_format:
 		buf_clear(&field_result);
 		fmt_position = 0;
 	}
-	free(field_result.buf);
+	buf_reset(&field_result);
 }
 
 static const struct field_converter field_converter[LAB_FIELD_COUNT] = {