Skip to content

Commit

Permalink
Free the original pointer if realloc failed.
Browse files Browse the repository at this point in the history
The POSIX `realloc` keep the original pointer untouched, so it can
easily leads to memory leakage. `mrb_realloc()` should handle those
bookkeeping, while `mrb_realloc_simple()` keeps the original `realloc`
behavior.
  • Loading branch information
matz committed Jun 24, 2020
1 parent 95360a1 commit 9cdf439
Showing 1 changed file with 3 additions and 8 deletions.
11 changes: 3 additions & 8 deletions src/gc.c
Expand Up @@ -225,14 +225,9 @@ mrb_realloc(mrb_state *mrb, void *p, size_t len)
p2 = mrb_realloc_simple(mrb, p, len);
if (len == 0) return p2;
if (p2 == NULL) {
if (mrb->gc.out_of_memory) {
mrb_raise_nomemory(mrb);
/* mrb_panic(mrb); */
}
else {
mrb->gc.out_of_memory = TRUE;
mrb_raise_nomemory(mrb);
}
mrb_free(mrb, p);
mrb->gc.out_of_memory = TRUE;
mrb_raise_nomemory(mrb);
}
else {
mrb->gc.out_of_memory = FALSE;
Expand Down

2 comments on commit 9cdf439

@dearblue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change mean that if the string object fails to expand, for example, the string object will continue to hold an invalid pointer, leading to a double free?
I think it's better to have a separate function that does this commit behavior.
How about mrb_reallocf()?
This name comes from FreeBSD's own reallocf(), which seems to be available on Solaris and illumos recently.

@matz
Copy link
Member Author

@matz matz commented on 9cdf439 Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I will stop freeing inside of realloc.

Please sign in to comment.