From aefe704b030f26b46131246cebd2d8659ea21b6e Mon Sep 17 00:00:00 2001 From: Omar Sandoval Date: Sat, 7 Oct 2023 22:30:38 -0700 Subject: [PATCH] libdrgn: string_builder: add STRING_BUILDER scope guard Signed-off-by: Omar Sandoval --- libdrgn/debug_info.c | 4 +-- libdrgn/error.c | 7 +++--- libdrgn/error.h | 2 +- libdrgn/language_c.c | 27 +++++++++----------- libdrgn/python/program.c | 12 +++------ libdrgn/stack_trace.c | 54 ++++++++++++++++++---------------------- libdrgn/string_builder.c | 6 ++--- libdrgn/string_builder.h | 40 ++++++++++++++++++++++++----- 8 files changed, 83 insertions(+), 69 deletions(-) diff --git a/libdrgn/debug_info.c b/libdrgn/debug_info.c index 71414e566..4825ddbd6 100644 --- a/libdrgn/debug_info.c +++ b/libdrgn/debug_info.c @@ -2007,7 +2007,7 @@ drgn_debug_info_report_finalize_errors(struct drgn_debug_info_load_state *load) (!string_builder_line_break(&load->errors) || !string_builder_appendf(&load->errors, "... %u more", load->num_errors - load->max_errors))) { - free(load->errors.str); + string_builder_deinit(&load->errors); return &drgn_enomem; } if (load->num_errors) { @@ -2076,7 +2076,7 @@ struct drgn_error *drgn_debug_info_load(struct drgn_debug_info *dbinfo, err: drgn_debug_info_free_modules(dbinfo, false, false); - free(load.errors.str); + string_builder_deinit(&load.errors); goto out; } diff --git a/libdrgn/error.c b/libdrgn/error.c index cec02eff3..a37bf3342 100644 --- a/libdrgn/error.c +++ b/libdrgn/error.c @@ -161,12 +161,11 @@ drgn_error_format_fault(uint64_t address, const char *format, ...) struct drgn_error *drgn_error_from_string_builder(enum drgn_error_code code, struct string_builder *sb) { - char *message = string_builder_null_terminate(sb); - if (!message) { - free(sb->str); + if (!string_builder_null_terminate(sb)) { + string_builder_deinit(sb); return &drgn_enomem; } - return drgn_error_create_nodup(code, message); + return drgn_error_create_nodup(code, sb->str); } LIBDRGN_PUBLIC struct drgn_error *drgn_error_copy(struct drgn_error *src) diff --git a/libdrgn/error.h b/libdrgn/error.h index eee7c774d..7d30f27ca 100644 --- a/libdrgn/error.h +++ b/libdrgn/error.h @@ -37,7 +37,7 @@ struct string_builder; /** * Create a @ref drgn_error with a message from a @ref string_builder. * - * This finalizes the string builder. + * This deinitializes the string builder. */ struct drgn_error *drgn_error_from_string_builder(enum drgn_error_code code, struct string_builder *sb); diff --git a/libdrgn/language_c.c b/libdrgn/language_c.c index 248b2a836..bd36b773f 100644 --- a/libdrgn/language_c.c +++ b/libdrgn/language_c.c @@ -536,14 +536,13 @@ static struct drgn_error * c_format_type_name(struct drgn_qualified_type qualified_type, char **ret) { struct drgn_error *err; - struct string_builder sb = STRING_BUILDER_INIT; + STRING_BUILDER(sb); err = c_format_type_name_impl(qualified_type, &sb); - if (err) { - free(sb.str); + if (err) return err; - } - if (!(*ret = string_builder_null_terminate(&sb))) + if (!string_builder_null_terminate(&sb)) return &drgn_enomem; + *ret = string_builder_steal(&sb); return NULL; } @@ -551,17 +550,16 @@ static struct drgn_error * c_format_type(struct drgn_qualified_type qualified_type, char **ret) { struct drgn_error *err; - struct string_builder sb = STRING_BUILDER_INIT; + STRING_BUILDER(sb); if (drgn_type_is_complete(qualified_type.type)) err = c_define_type(qualified_type, 0, &sb); else err = c_format_type_name_impl(qualified_type, &sb); - if (err) { - free(sb.str); + if (err) return err; - } - if (!(*ret = string_builder_null_terminate(&sb))) + if (!string_builder_null_terminate(&sb)) return &drgn_enomem; + *ret = string_builder_steal(&sb); return NULL; } @@ -1604,15 +1602,14 @@ static struct drgn_error *c_format_object(const struct drgn_object *obj, char **ret) { struct drgn_error *err; - struct string_builder sb = STRING_BUILDER_INIT; + STRING_BUILDER(sb); err = c_format_object_impl(obj, 0, columns, max(columns, (size_t)1), flags, &sb); - if (err) { - free(sb.str); + if (err) return err; - } - if (!(*ret = string_builder_null_terminate(&sb))) + if (!string_builder_null_terminate(&sb)) return &drgn_enomem; + *ret = string_builder_steal(&sb); return NULL; } diff --git a/libdrgn/python/program.c b/libdrgn/python/program.c index f35fefc4c..af62218c4 100644 --- a/libdrgn/python/program.c +++ b/libdrgn/python/program.c @@ -21,13 +21,13 @@ static void drgnpy_log_fn(struct drgn_program *prog, void *arg, enum drgn_log_level level, const char *format, va_list ap, struct drgn_error *err) { - struct string_builder sb = STRING_BUILDER_INIT; + STRING_BUILDER(sb); if (!string_builder_vappendf(&sb, format, ap)) - goto out; + return; if (err && !string_builder_append_error(&sb, err)) - goto out; + return; - PyGILState_STATE gstate = PyGILState_Ensure(); + PyGILState_guard(); PyObject *ret = PyObject_CallFunction(logger_log, "iOs#", (level + 1) * 10, percent_s, sb.str ? sb.str : "", @@ -36,10 +36,6 @@ static void drgnpy_log_fn(struct drgn_program *prog, void *arg, Py_DECREF(ret); else PyErr_WriteUnraisable(logger_log); - PyGILState_Release(gstate); - -out: - free(sb.str); } static int get_log_level(void) diff --git a/libdrgn/stack_trace.c b/libdrgn/stack_trace.c index 714dea27e..f2ebdbd23 100644 --- a/libdrgn/stack_trace.c +++ b/libdrgn/stack_trace.c @@ -109,17 +109,17 @@ drgn_stack_trace_num_frames(struct drgn_stack_trace *trace) LIBDRGN_PUBLIC struct drgn_error * drgn_format_stack_trace(struct drgn_stack_trace *trace, char **ret) { - struct string_builder str = STRING_BUILDER_INIT; + STRING_BUILDER(str); for (size_t frame = 0; frame < trace->num_frames; frame++) { if (!string_builder_appendf(&str, "#%-2zu ", frame)) - goto enomem; + return &drgn_enomem; struct drgn_register_state *regs = trace->frames[frame].regs; struct optional_uint64 pc; const char *name = drgn_stack_frame_name(trace, frame); if (name) { if (!string_builder_append(&str, name)) - goto enomem; + return &drgn_enomem; } else if ((pc = drgn_register_state_get_pc(regs)).has_value) { Dwfl_Module *dwfl_module = regs->module ? regs->module->dwfl_module : NULL; @@ -134,15 +134,15 @@ drgn_format_stack_trace(struct drgn_stack_trace *trace, char **ret) sym.name, pc.value - sym.address, sym.size)) - goto enomem; + return &drgn_enomem; } else { if (!string_builder_appendf(&str, "0x%" PRIx64, pc.value)) - goto enomem; + return &drgn_enomem; } } else { if (!string_builder_append(&str, "???")) - goto enomem; + return &drgn_enomem; } int line, column; @@ -151,38 +151,35 @@ drgn_format_stack_trace(struct drgn_stack_trace *trace, char **ret) if (filename && column) { if (!string_builder_appendf(&str, " (%s:%d:%d)", filename, line, column)) - goto enomem; + return &drgn_enomem; } else if (filename) { if (!string_builder_appendf(&str, " (%s:%d)", filename, line)) - goto enomem; + return &drgn_enomem; } if (frame != trace->num_frames - 1 && !string_builder_appendc(&str, '\n')) - goto enomem; + return &drgn_enomem; } - if (!(*ret = string_builder_null_terminate(&str))) - goto enomem; + if (!string_builder_null_terminate(&str)) + return &drgn_enomem; + *ret = string_builder_steal(&str); return NULL; - -enomem: - free(str.str); - return &drgn_enomem; } LIBDRGN_PUBLIC struct drgn_error * drgn_format_stack_frame(struct drgn_stack_trace *trace, size_t frame, char **ret) { - struct string_builder str = STRING_BUILDER_INIT; + STRING_BUILDER(str); struct drgn_register_state *regs = trace->frames[frame].regs; if (!string_builder_appendf(&str, "#%zu at ", frame)) - goto enomem; + return &drgn_enomem; struct optional_uint64 pc = drgn_register_state_get_pc(regs); if (pc.has_value) { if (!string_builder_appendf(&str, "%#" PRIx64, pc.value)) - goto enomem; + return &drgn_enomem; Dwfl_Module *dwfl_module = regs->module ? regs->module->dwfl_module : NULL; @@ -195,15 +192,15 @@ drgn_format_stack_frame(struct drgn_stack_trace *trace, size_t frame, char **ret !string_builder_appendf(&str, " (%s+0x%" PRIx64 "/0x%" PRIx64 ")", sym.name, pc.value - sym.address, sym.size)) - goto enomem; + return &drgn_enomem; } else { if (!string_builder_append(&str, "???")) - goto enomem; + return &drgn_enomem; } const char *name = drgn_stack_frame_name(trace, frame); if (name && !string_builder_appendf(&str, " in %s", name)) - goto enomem; + return &drgn_enomem; int line, column; const char *filename = drgn_stack_frame_source(trace, frame, &line, @@ -211,23 +208,20 @@ drgn_format_stack_frame(struct drgn_stack_trace *trace, size_t frame, char **ret if (filename && column) { if (!string_builder_appendf(&str, " at %s:%d:%d", filename, line, column)) - goto enomem; + return &drgn_enomem; } else if (filename) { if (!string_builder_appendf(&str, " at %s:%d", filename, line)) - goto enomem; + return &drgn_enomem; } if (drgn_stack_frame_is_inline(trace, frame) && !string_builder_append(&str, " (inlined)")) - goto enomem; + return &drgn_enomem; - if (!(*ret = string_builder_null_terminate(&str))) - goto enomem; + if (!string_builder_null_terminate(&str)) + return &drgn_enomem; + *ret = string_builder_steal(&str); return NULL; - -enomem: - free(str.str); - return &drgn_enomem; } LIBDRGN_PUBLIC const char *drgn_stack_frame_name(struct drgn_stack_trace *trace, diff --git a/libdrgn/string_builder.c b/libdrgn/string_builder.c index 703539743..0bd852189 100644 --- a/libdrgn/string_builder.c +++ b/libdrgn/string_builder.c @@ -9,12 +9,12 @@ #include "string_builder.h" #include "util.h" -char *string_builder_null_terminate(struct string_builder *sb) +bool string_builder_null_terminate(struct string_builder *sb) { if (!string_builder_reserve_for_append(sb, 1)) - return NULL; + return false; sb->str[sb->len] = '\0'; - return sb->str; + return true; } bool string_builder_reserve(struct string_builder *sb, size_t capacity) diff --git a/libdrgn/string_builder.h b/libdrgn/string_builder.h index 8496ed242..8df038b13 100644 --- a/libdrgn/string_builder.h +++ b/libdrgn/string_builder.h @@ -41,8 +41,7 @@ struct string_builder { /** * Current string buffer. * - * This may be reallocated when appending. It must be freed with @c - * free() when it will no longer be used. + * This may be reallocated when appending. */ char *str; /** Length of @c str. */ @@ -54,15 +53,44 @@ struct string_builder { /** String builder initializer. */ #define STRING_BUILDER_INIT { 0 } +/** Free memory allocated by a @ref string_builder. */ +static inline void string_builder_deinit(struct string_builder *sb) +{ + free(sb->str); +} + +/** + * Define and initialize a @ref string_builder named @p sb that is automatically + * deinitialized when it goes out of scope. + */ +#define STRING_BUILDER(sb) \ + __attribute__((__cleanup__(string_builder_deinit))) \ + struct string_builder sb = STRING_BUILDER_INIT + /** - * Null-terminate and return a string from a @ref string_builder. + * Steal the string buffer from a @ref string_builder. + * + * The string builder can no longer be used except to be passed to @ref + * string_builder_deinit(), which will do nothing. + * + * @return String buffer. This must be freed with @c free(). + */ +static inline char *string_builder_steal(struct string_builder *sb) +{ + char *str = sb->str; + sb->str = NULL; + return str; +} + +/** + * Null-terminate a @ref string_builder. * * This appends a null character without incrementing @ref string_builder::len. * - * @return @ref string_builder::str on success, @c NULL on error (if we couldn't - * allocate memory). + * @return @c true on success, @c false on error (if we couldn't allocate + * memory). */ -char *string_builder_null_terminate(struct string_builder *sb); +bool string_builder_null_terminate(struct string_builder *sb); /** * Resize the buffer of a @ref string_builder to a given capacity.