From 0573f166938e22e58d84e092f6a67d79de7c1759 Mon Sep 17 00:00:00 2001 From: John Lindgren Date: Sun, 14 Apr 2024 14:20:57 -0400 Subject: [PATCH] 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 | 49 +++++++++++++++----- include/common/grab-file.h | 6 ++- src/action.c | 5 +- src/button/button-xbm.c | 8 ++-- src/common/buf.c | 94 +++++++++++++++++++++----------------- src/common/dir.c | 7 ++- src/common/grab-file.c | 11 +++-- src/config/rcxml.c | 6 +-- src/config/session.c | 5 +- src/menu/menu.c | 10 ++-- src/osd.c | 5 +- src/osd_field.c | 6 +-- 12 files changed, 122 insertions(+), 90 deletions(-) diff --git a/include/common/buf.h b/include/common/buf.h index 1051229a..df009edb 100644 --- a/include/common/buf.h +++ b/include/common/buf.h @@ -8,16 +8,28 @@ #ifndef LABWC_BUF_H #define LABWC_BUF_H -#include -#include -#include - 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 */ diff --git a/include/common/grab-file.h b/include/common/grab-file.h index df983f75..6a6de56d 100644 --- a/include/common/grab-file.h +++ b/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 */ diff --git a/src/action.c b/src/action.c index 677e13da..2ba1c531 100644 --- a/src/action.c +++ b/src/action.c @@ -706,12 +706,11 @@ actions_run(struct view *activator, struct server *server, break; case ACTION_TYPE_EXECUTE: { - struct buf cmd; - buf_init(&cmd); + struct buf cmd = BUF_INIT; 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: diff --git a/src/button/button-xbm.c b/src/button/button-xbm.c index 125873c0..0133a052 100644 --- a/src/button/button-xbm.c +++ b/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); - if (token_buffer) { - struct token *tokens = tokenize_xbm(token_buffer); - free(token_buffer); + struct buf token_buf = grab_file(filename); + if (token_buf.len) { + struct token *tokens = tokenize_xbm(token_buf.buf); pixmap = parse_xbm_tokens(tokens); if (tokens) { free(tokens); } } + buf_reset(&token_buf); if (!pixmap.data) { return; } diff --git a/src/common/buf.c b/src/common/buf.c index f6bde97f..849e0df7 100644 --- a/src/common/buf.c +++ b/src/common/buf.c @@ -2,14 +2,15 @@ #include #include #include +#include #include "common/buf.h" +#include "common/macros.h" #include "common/mem.h" void buf_expand_tilde(struct buf *s) { - struct buf new; - buf_init(&new); + struct buf new = BUF_INIT; 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); - s->buf = new.buf; - s->len = new.len; - s->alloc = new.alloc; + buf_move(s, &new); } static void @@ -44,10 +42,8 @@ isvalid(char p) void buf_expand_shell_variables(struct buf *s) { - struct buf new; - struct buf environment_variable; - buf_init(&new); - buf_init(&environment_variable); + struct buf new = BUF_INIT; + struct buf environment_variable = BUF_INIT; 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); - free(s->buf); - s->buf = new.buf; - s->len = new.len; - s->alloc = new.alloc; + buf_reset(&environment_variable); + buf_move(s, &new); } -void -buf_init(struct buf *s) +static void +buf_expand(struct buf *s, int new_alloc) { - /* we can't assert(!s->buf) here because struct may be uninitialized */ - - s->alloc = 256; - s->buf = xmalloc(s->alloc); - s->buf[0] = '\0'; - s->len = 0; + /* + * "s->alloc &&" ensures that s->data is always allocated after + * returning (even if new_alloc == 0). The extra check is not + * really necessary but makes it easier for analyzers to see + * that we never overwrite a string literal. + */ + 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) { - s->alloc = s->alloc + len; - s->buf = xrealloc(s->buf, s->alloc); - } + buf_expand(s, s->len + len + 1); 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); - - if (s->alloc <= s->len + 1) { - s->alloc = s->alloc * 3 / 2 + 16; - s->buf = xrealloc(s->buf, s->alloc); - } + buf_expand(s, s->len + 1); 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); - - s->len = 0; - s->buf[0] = '\0'; + if (s->alloc) { + assert(s->buf); + s->len = 0; + s->buf[0] = '\0'; + } else { + *s = BUF_INIT; + } } void buf_reset(struct buf *s) { - zfree(s->buf); - buf_init(s); + if (s->alloc) { + 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; } diff --git a/src/common/dir.c b/src/common/dir.c index 59db4104..4f173e94 100644 --- a/src/common/dir.c +++ b/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_init(&prefix); + buf_clear(&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 diff --git a/src/common/grab-file.c b/src/common/grab-file.c index daf5bf3c..9c90045c 100644 --- a/src/common/grab-file.c +++ b/src/common/grab-file.c @@ -10,18 +10,19 @@ #include "common/buf.h" #include +#include +#include -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; - buf_init(&buffer); + struct buf buffer = BUF_INIT; 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; } diff --git a/src/config/rcxml.c b/src/config/rcxml.c index 7fb0e806..ded8b646 100644 --- a/src/config/rcxml.c +++ b/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; } diff --git a/src/config/session.c b/src/config/session.c index f2d573f7..2bf4693b 100644 --- a/src/config/session.c +++ b/src/config/session.c @@ -49,13 +49,12 @@ process_line(char *line) return; } - struct buf value; - buf_init(&value); + struct buf value = BUF_INIT; 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 */ diff --git a/src/menu/menu.c b/src/menu/menu.c index 735adde6..7130b213 100644 --- a/src/menu/menu.c +++ b/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); diff --git a/src/osd.c b/src/osd.c index ca53cec8..4bd57a6d 100644 --- a/src/osd.c +++ b/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; - buf_init(&buf); + struct buf buf = BUF_INIT; /* 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); diff --git a/src/osd_field.c b/src/osd_field.c index a804a18b..7728297d 100644 --- a/src/osd_field.c +++ b/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; - buf_init(&field_result); - + struct buf field_result = BUF_INIT; 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] = {