libdrgn: string_builder: add STRING_BUILDER scope guard

Signed-off-by: Omar Sandoval <[email protected]>

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;
 }
 

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)

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);

libdrgn/language_c.c

@@ -536,32 +536,30 @@ 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;
 }
 
 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;
 }
 

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)

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,39 +192,36 @@ 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,
 						       &column);
 	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,

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)

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.