Skip to content

Backport file_expand memory usage fix #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 13, 2014
Merged

Backport file_expand memory usage fix #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2aa6072
string.c: shrink too big buffer
nobu Jun 12, 2014
abacb94
file.c: shrink expanded path
nobu Jun 12, 2014
b74a7d3
string.c: consider capacity
nobu Jun 12, 2014
97c12cc
add more test coverage for [ruby-core:63136] [Bug #9934]
tmm1 Jun 13, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3397,6 +3397,16 @@ rb_file_expand_path_internal(VALUE fname, VALUE dname, int abs_mode, int long_na

#define EXPAND_PATH_BUFFER() rb_usascii_str_new(0, MAXPATHLEN + 2)

static VALUE
str_shrink(VALUE str)
{
rb_str_resize(str, RSTRING_LEN(str));
return str;
}

#define expand_path(fname, dname, abs_mode, long_name, result) \
str_shrink(rb_file_expand_path_internal(fname, dname, abs_mode, long_name, result))

#define check_expand_path_args(fname, dname) \
(((fname) = rb_get_path(fname)), \
(void)(NIL_P(dname) ? (dname) : ((dname) = rb_get_path(dname))))
Expand All @@ -3411,13 +3421,13 @@ VALUE
rb_file_expand_path(VALUE fname, VALUE dname)
{
check_expand_path_args(fname, dname);
return rb_file_expand_path_internal(fname, dname, 0, 1, EXPAND_PATH_BUFFER());
return expand_path(fname, dname, 0, 1, EXPAND_PATH_BUFFER());
}

VALUE
rb_file_expand_path_fast(VALUE fname, VALUE dname)
{
return rb_file_expand_path_internal(fname, dname, 0, 0, EXPAND_PATH_BUFFER());
return expand_path(fname, dname, 0, 0, EXPAND_PATH_BUFFER());
}

/*
Expand Down Expand Up @@ -3465,7 +3475,7 @@ VALUE
rb_file_absolute_path(VALUE fname, VALUE dname)
{
check_expand_path_args(fname, dname);
return rb_file_expand_path_internal(fname, dname, 1, 1, EXPAND_PATH_BUFFER());
return expand_path(fname, dname, 1, 1, EXPAND_PATH_BUFFER());
}

/*
Expand Down Expand Up @@ -5408,6 +5418,7 @@ is_explicit_relative(const char *path)
static VALUE
copy_path_class(VALUE path, VALUE orig)
{
str_shrink(path);
RBASIC_SET_CLASS(path, rb_obj_class(orig));
OBJ_FREEZE(path);
return path;
Expand Down
8 changes: 6 additions & 2 deletions string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2034,9 +2034,11 @@ rb_str_resize(VALUE str, long len)
independent = str_independent(str);
ENC_CODERANGE_CLEAR(str);
slen = RSTRING_LEN(str);
if (len != slen) {

{
const int termlen = TERM_LEN(str);
if (STR_EMBED_P(str)) {
if (len == slen) return str;
if (len + termlen <= RSTRING_EMBED_LEN_MAX + 1) {
STR_SET_EMBED_LEN(str, len);
TERM_FILL(RSTRING(str)->as.ary + len, termlen);
Expand All @@ -2056,11 +2058,13 @@ rb_str_resize(VALUE str, long len)
return str;
}
else if (!independent) {
if (len == slen) return str;
str_make_independent_expand(str, len - slen);
}
else if (slen < len || slen - len > 1024) {
else if (slen < len || (RSTRING(str)->as.heap.aux.capa - len) > (len < 1024 ? len : 1024)) {
REALLOC_N(RSTRING(str)->as.heap.ptr, char, len + termlen);
}
else if (len == slen) return str;
if (!STR_NOCAPA_P(str)) {
RSTRING(str)->as.heap.aux.capa = len;
}
Expand Down
9 changes: 9 additions & 0 deletions test/ruby/test_file_exhaustive.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,15 @@ def test_expand_path
end
end

def test_expand_path_memsize
bug9934 = '[ruby-core:63114] [Bug #9934]'
require "objspace"
path = File.expand_path("/foo")
assert_operator(ObjectSpace.memsize_of(path), :<=, path.bytesize, bug9934)
path = File.expand_path("/a"*25)
assert_equal(51, ObjectSpace.memsize_of(path), bug9934)
end

def test_expand_path_encoding
drive = (DRIVE ? 'C:' : '')
if Encoding.find("filesystem") == Encoding::CP1251
Expand Down