From 7fe5c2e260cf53048cfad5bfdc064e985cd11d67 Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Thu, 8 Jan 2026 08:22:15 +0900 Subject: [PATCH] gc.c: rename mrb_alloca() to mrb_temp_alloc() and fix memory leaks rename mrb_alloca() to mrb_temp_alloc() for clearer naming - the new name better describes its purpose as GC-managed temporary allocation. keep mrb_alloca() as a macro alias for backward compatibility. apply mrb_temp_alloc() to fix potential memory leaks in: - mruby-strftime: if mrb_str_cat() raises, allocated buffers now cleaned by GC - mruby-io File.readlink: if mrb_str_new() raises, buffer now cleaned by GC Co-authored-by: Claude --- include/mruby.h | 3 ++- mrbgems/mruby-io/src/file.c | 16 ++++++------- mrbgems/mruby-strftime/src/strftime.c | 33 +++++---------------------- src/gc.c | 2 +- 4 files changed, 16 insertions(+), 38 deletions(-) diff --git a/include/mruby.h b/include/mruby.h index 136018f9a..bfaf5d6ee 100644 --- a/include/mruby.h +++ b/include/mruby.h @@ -1571,7 +1571,8 @@ MRB_API mrb_value mrb_fiber_alive_p(mrb_state *mrb, mrb_value fib); MRB_API void mrb_stack_extend(mrb_state*, mrb_int); /* temporary memory allocation, only effective while GC arena is kept */ -MRB_API void* mrb_alloca(mrb_state *mrb, size_t); +MRB_API void* mrb_temp_alloc(mrb_state *mrb, size_t); +#define mrb_alloca(mrb, size) mrb_temp_alloc(mrb, size) /* for compatibility */ MRB_API void mrb_state_atexit(mrb_state *mrb, mrb_atexit_func func); diff --git a/mrbgems/mruby-io/src/file.c b/mrbgems/mruby-io/src/file.c index 30ecd22a0..81e552918 100644 --- a/mrbgems/mruby-io/src/file.c +++ b/mrbgems/mruby-io/src/file.c @@ -885,32 +885,30 @@ mrb_file_s_chmod(mrb_state *mrb, mrb_value klass) * File.symlink("testfile", "link-to-test") #=> 0 * File.readlink("link-to-test") #=> "testfile" */ +#ifndef PATH_MAX +#define PATH_MAX 4096 +#endif + static mrb_value mrb_file_s_readlink(mrb_state *mrb, mrb_value klass) { const char *path; - size_t bufsize = 100; mrb_get_args(mrb, "z", &path); char *tmp = mrb_locale_from_utf8(path, -1); - char *buf = (char*)mrb_malloc(mrb, bufsize); + /* Use mrb_temp_alloc for exception safety - GC will clean up on exception */ + char *buf = (char*)mrb_temp_alloc(mrb, PATH_MAX); - int64_t rc; - while ((rc = mrb_hal_io_readlink(mrb, tmp, buf, bufsize)) == (int64_t)bufsize) { - bufsize += 100; - buf = (char*)mrb_realloc(mrb, buf, bufsize); - } + int64_t rc = mrb_hal_io_readlink(mrb, tmp, buf, PATH_MAX); mrb_locale_free(tmp); if (rc == -1) { - mrb_free(mrb, buf); mrb_sys_fail(mrb, path); } tmp = mrb_utf8_from_locale(buf, -1); mrb_value ret = mrb_str_new(mrb, tmp, rc); mrb_utf8_free(tmp); - mrb_free(mrb, buf); return ret; } diff --git a/mrbgems/mruby-strftime/src/strftime.c b/mrbgems/mruby-strftime/src/strftime.c index e328d30f7..02a2d8675 100644 --- a/mrbgems/mruby-strftime/src/strftime.c +++ b/mrbgems/mruby-strftime/src/strftime.c @@ -12,7 +12,6 @@ #include #include -#define INITIAL_BUFFER_SIZE 64 #define MAX_BUFFER_SIZE 4096 /* @@ -60,12 +59,12 @@ mrb_time_strftime(mrb_state *mrb, mrb_value self) /* Process this segment (up to NUL or end of string) */ if (segment_len > 0) { char *segment; - size_t buf_size; char *buf; size_t n; /* Create null-terminated copy of this segment */ - segment = (char *)mrb_malloc(mrb, (size_t)segment_len + 1); + /* Use mrb_temp_alloc for exception safety - GC will clean up on exception */ + segment = (char *)mrb_temp_alloc(mrb, (size_t)segment_len + 1); memcpy(segment, fmt_ptr, (size_t)segment_len); segment[segment_len] = '\0'; @@ -74,7 +73,6 @@ mrb_time_strftime(mrb_state *mrb, mrb_value self) /* Scan for %- patterns in the format string */ for (const char *p = segment; *p != '\0'; p++) { if (p[0] == '%' && p[1] == '-') { - mrb_free(mrb, segment); mrb_raisef(mrb, E_ARGUMENT_ERROR, "strftime format flag '%-' not supported on this platform (use '%%#' on Windows)"); } @@ -82,33 +80,14 @@ mrb_time_strftime(mrb_state *mrb, mrb_value self) #endif /* Allocate buffer for formatted output */ - buf_size = INITIAL_BUFFER_SIZE; - buf = (char *)mrb_malloc(mrb, buf_size); + /* Use mrb_temp_alloc with max size for exception safety */ + buf = (char *)mrb_temp_alloc(mrb, MAX_BUFFER_SIZE); - /* Try formatting; grow buffer if needed */ - while (1) { - n = strftime(buf, buf_size, segment, tm); - - /* - * strftime returns 0 if: - * 1. Buffer is too small (retry with larger buffer) - * 2. Format produces empty result (stop retrying) - * We distinguish by checking buffer size limit. - */ - if (n > 0 || buf_size >= MAX_BUFFER_SIZE) { - break; - } - - /* Double buffer size and retry */ - buf_size *= 2; - buf = (char *)mrb_realloc(mrb, buf, buf_size); - } + /* Try formatting with max buffer size */ + n = strftime(buf, MAX_BUFFER_SIZE, segment, tm); /* Append formatted output to result */ mrb_str_cat(mrb, result, buf, n); - - mrb_free(mrb, buf); - mrb_free(mrb, segment); } /* If there was a NUL, append it to result and advance past it */ diff --git a/src/gc.c b/src/gc.c index 49850d99a..adc4c7ffb 100644 --- a/src/gc.c +++ b/src/gc.c @@ -266,7 +266,7 @@ mrb_free(mrb_state *mrb, void *p) } MRB_API void* -mrb_alloca(mrb_state *mrb, size_t size) +mrb_temp_alloc(mrb_state *mrb, size_t size) { struct RString *s; s = MRB_OBJ_ALLOC(mrb, MRB_TT_STRING, NULL);