From 8b7c142555884036873f43fc50a6fcfe28959ce2 Mon Sep 17 00:00:00 2001 From: henadzit Date: Tue, 3 Nov 2015 14:30:41 +0300 Subject: [PATCH 1/2] Fixed memory leaks in children, children_with_values, has_children?. Removed redundant code from has_children?. Version bump to 0.5.2 --- VERSION.yml | 4 ++-- ext/trie/trie.c | 57 +++++++++-------------------------------------- fast_trie.gemspec | 2 +- spec/trie_spec.rb | 25 +++++++++++++++++++++ 4 files changed, 39 insertions(+), 49 deletions(-) diff --git a/VERSION.yml b/VERSION.yml index 7c2afee..556a2d3 100644 --- a/VERSION.yml +++ b/VERSION.yml @@ -1,5 +1,5 @@ ---- +--- :major: 0 :minor: 5 -:patch: 1 +:patch: 2 :build: diff --git a/ext/trie/trie.c b/ext/trie/trie.c index a2e7a4b..1eaecb7 100644 --- a/ext/trie/trie.c +++ b/ext/trie/trie.c @@ -169,10 +169,11 @@ static VALUE walk_all_paths(Trie *trie, VALUE children, TrieState *state, char * char *word = (char*) malloc(prefix_size + 2); memcpy(word, prefix, prefix_size + 2); rb_ary_push(children, rb_str_new2(word)); + free(word); } walk_all_paths(trie, children, next_state, prefix, prefix_size + 1); - + prefix[prefix_size] = 0; trie_state_free(next_state); } @@ -211,17 +212,20 @@ static VALUE rb_trie_children(VALUE self, VALUE prefix) { int prefix_size = RSTRING_LEN(prefix); TrieState *state = trie_root(trie); VALUE children = rb_ary_new(); + TrieChar *char_prefix = (TrieChar*)RSTRING_PTR(prefix); - + if(!traverse(state, char_prefix)) { + trie_state_free(state); return children; } if(trie_state_is_terminal(state)) rb_ary_push(children, prefix); - + char prefix_buffer[1024]; memcpy(prefix_buffer, char_prefix, prefix_size); + prefix_buffer[prefix_size] = 0; walk_all_paths(trie, children, state, prefix_buffer, prefix_size); @@ -230,34 +234,6 @@ static VALUE rb_trie_children(VALUE self, VALUE prefix) { return children; } -static Bool walk_all_paths_until_first_terminal(Trie *trie, TrieState *state, char *prefix, int prefix_size) { - int c; - Bool ret = FALSE; - for(c = 1; c < 256; c++) { - if(trie_state_is_walkable(state,c)) { - TrieState *next_state = trie_state_clone(state); - trie_state_walk(next_state, c); - - prefix[prefix_size] = c; - prefix[prefix_size + 1] = 0; - - if(trie_state_is_terminal(next_state)) { - return TRUE; - } - - ret = walk_all_paths_until_first_terminal(trie, next_state, prefix, prefix_size + 1); - - prefix[prefix_size] = 0; - trie_state_free(next_state); - - if (ret == TRUE) { - return ret; - } - } - } - - return ret; -} static VALUE rb_trie_has_children(VALUE self, VALUE prefix) { if(NIL_P(prefix)) @@ -272,21 +248,9 @@ static VALUE rb_trie_has_children(VALUE self, VALUE prefix) { TrieState *state = trie_root(trie); TrieChar *char_prefix = (TrieChar*)RSTRING_PTR(prefix); - if(!traverse(state, char_prefix)) { - return Qfalse; - } - - if(trie_state_is_terminal(state)) - return Qtrue; - - char prefix_buffer[1024]; - memcpy(prefix_buffer, char_prefix, prefix_size); - prefix_buffer[prefix_size] = 0; - - Bool ret = walk_all_paths_until_first_terminal(trie, state, prefix_buffer, prefix_size); - - trie_state_free(state); - return ret == TRUE ? Qtrue : Qfalse; + Bool ret = traverse(state, char_prefix); + trie_state_free(state); + return ret == TRUE ? Qtrue : Qfalse; } static VALUE walk_all_paths_with_values(Trie *trie, VALUE children, TrieState *state, char *prefix, int prefix_size) { @@ -308,6 +272,7 @@ static VALUE walk_all_paths_with_values(Trie *trie, VALUE children, TrieState *s VALUE tuple = rb_ary_new(); rb_ary_push(tuple, rb_str_new2(word)); + free(word); TrieData trie_data = trie_state_get_data(end_state); rb_ary_push(tuple, (VALUE)trie_data); diff --git a/fast_trie.gemspec b/fast_trie.gemspec index 0816621..77db09f 100644 --- a/fast_trie.gemspec +++ b/fast_trie.gemspec @@ -12,7 +12,7 @@ Gem::Specification.new do |s| s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version= s.require_paths = ["ext"] s.authors = ["Tyler McMullen", "Matt Hickford"] - s.date = "2015-07-27" + s.date = "2015-11-03" s.description = "Ruby Trie based on libdatrie." s.email = "tyler@scribd.com" s.extensions = ["ext/trie/extconf.rb"] diff --git a/spec/trie_spec.rb b/spec/trie_spec.rb index d8c3d57..a12ec0f 100644 --- a/spec/trie_spec.rb +++ b/spec/trie_spec.rb @@ -65,6 +65,17 @@ children.should include('rocket') end + it 'returns all children by first character' do + children = @trie.children('r') + children.size.should == 2 + children.should include('rock') + children.should include('rocket') + + children = @trie.children('f') + children.size.should == 1 + children.should include('frederico') + end + it 'returns blank array if prefix does not exist' do @trie.children('ajsodij').should == [] end @@ -176,12 +187,26 @@ @trie.has_children?('rock').should be_true @trie.has_children?('rocket').should be_true + + @trie.has_children?('f').should be_true + @trie.has_children?('fred').should be_true end it 'returns false when there are no children matching prefix' do @trie.has_children?('no').should be_false @trie.has_children?('rome').should be_false @trie.has_children?('roc_').should be_false + @trie.has_children?('rocketttt').should be_false + end + + it 'handles keys with values' do + @trie.add('value1', 1) + @trie.add('value2', 2) + + @trie.has_children?('value1').should be_true + @trie.has_children?('value2').should be_true + @trie.has_children?('value').should be_true + @trie.has_children?('volue').should be_false end end end From 83253b5b66e2b2374ba4718542d9b5b2cb88436b Mon Sep 17 00:00:00 2001 From: henadzit Date: Mon, 9 Nov 2015 23:22:03 +0300 Subject: [PATCH 2/2] Updated gemspec to have new version. --- fast_trie.gemspec | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fast_trie.gemspec b/fast_trie.gemspec index 77db09f..c3c8b71 100644 --- a/fast_trie.gemspec +++ b/fast_trie.gemspec @@ -2,17 +2,17 @@ # DO NOT EDIT THIS FILE DIRECTLY # Instead, edit Jeweler::Tasks in Rakefile, and run 'rake gemspec' # -*- encoding: utf-8 -*- -# stub: fast_trie 0.5.1 ruby ext +# stub: fast_trie 0.5.2 ruby ext # stub: ext/trie/extconf.rb Gem::Specification.new do |s| s.name = "fast_trie" - s.version = "0.5.1" + s.version = "0.5.2" s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version= s.require_paths = ["ext"] s.authors = ["Tyler McMullen", "Matt Hickford"] - s.date = "2015-11-03" + s.date = "2015-11-09" s.description = "Ruby Trie based on libdatrie." s.email = "tyler@scribd.com" s.extensions = ["ext/trie/extconf.rb"]