From 0c21dfb7c4f0414f68a6d6e1b4309479a22b91bc Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Mar 2014 21:04:11 +0800 Subject: [PATCH 1/8] Enable braces match check of cpplint. --- Gruntfile.coffee | 4 ---- src/onig-result.cc | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Gruntfile.coffee b/Gruntfile.coffee index 858d200..9c9acd8 100644 --- a/Gruntfile.coffee +++ b/Gruntfile.coffee @@ -29,10 +29,6 @@ module.exports = (grunt) -> include: false legal: copyright: false - readability: - braces: false - runtime: - sizeof: false whitespace: line_length: false diff --git a/src/onig-result.cc b/src/onig-result.cc index 3b6ed7a..0414c92 100644 --- a/src/onig-result.cc +++ b/src/onig-result.cc @@ -27,6 +27,7 @@ int OnigResult::LengthAt(int index) { if (bytes > 0) { const char *search = searchString_.data() + *(region_->beg + index); return UnicodeUtils::characters_in_bytes(search, bytes); - } else + } else { return 0; + } } From ab7813ce739075fbe3dee39cbc1480fea60b7ac7 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Mar 2014 21:12:21 +0800 Subject: [PATCH 2/8] regex_ should be initialized to NULL. Otherwise when onig_new failed we would get a non-NULL pointer and onig_free would try to free it in destructor. --- src/onig-reg-exp.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/onig-reg-exp.cc b/src/onig-reg-exp.cc index 316b4f8..3cf79c4 100644 --- a/src/onig-reg-exp.cc +++ b/src/onig-reg-exp.cc @@ -6,7 +6,9 @@ using ::v8::Exception; using ::v8::String; -OnigRegExp::OnigRegExp(const string& source) : source_(source) { +OnigRegExp::OnigRegExp(const string& source) + : source_(source), + regex_(NULL) { OnigErrorInfo error; const UChar* sourceData = (const UChar*)source.data(); int status = onig_new(®ex_, sourceData, sourceData + source.length(), From 9a8e173e98fb18be6d22bba5b0f6f6485e985cdc Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Mar 2014 21:13:51 +0800 Subject: [PATCH 3/8] Ignore vim swap files. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 2a31a9d..7dffb6d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ /node_modules /build /lib/*.js +*.swp *.o *.lo *.la From e4368616f1af3acf7b90dbf7187022f0406b365b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Mar 2014 21:15:53 +0800 Subject: [PATCH 4/8] Guard against NULL when searching. --- src/onig-reg-exp.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/onig-reg-exp.cc b/src/onig-reg-exp.cc index 3cf79c4..4f97017 100644 --- a/src/onig-reg-exp.cc +++ b/src/onig-reg-exp.cc @@ -31,6 +31,9 @@ bool OnigRegExp::Contains(const string& value) { } OnigResult* OnigRegExp::Search(const string& searchString, size_t position) { + if (!regex_) + ThrowException(Exception::Error(String::New("RegExp is not valid"))); + int end = searchString.size(); OnigRegion* region = onig_region_new(); const UChar* searchData = (const UChar*)searchString.data(); From 4bb991fbb487c1b814d10db42ebd23e4fd45881d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Mar 2014 21:31:55 +0800 Subject: [PATCH 5/8] Fix memory leak when searching regexp. --- src/onig-reg-exp.cc | 9 ++++++--- src/onig-reg-exp.h | 4 +++- src/onig-scanner.cc | 10 +++++----- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/onig-reg-exp.cc b/src/onig-reg-exp.cc index 4f97017..85c14ac 100644 --- a/src/onig-reg-exp.cc +++ b/src/onig-reg-exp.cc @@ -30,9 +30,12 @@ bool OnigRegExp::Contains(const string& value) { return source_.find(value) != string::npos; } -OnigResult* OnigRegExp::Search(const string& searchString, size_t position) { - if (!regex_) +shared_ptr OnigRegExp::Search(const string& searchString, + size_t position) { + if (!regex_) { ThrowException(Exception::Error(String::New("RegExp is not valid"))); + return NULL; + } int end = searchString.size(); OnigRegion* region = onig_region_new(); @@ -42,7 +45,7 @@ OnigResult* OnigRegExp::Search(const string& searchString, size_t position) { ONIG_OPTION_NONE); if (status != ONIG_MISMATCH) { - return new OnigResult(region, searchString); + return shared_ptr(new OnigResult(region, searchString)); } else { onig_region_free(region, 1); return NULL; diff --git a/src/onig-reg-exp.h b/src/onig-reg-exp.h index 8be9d01..517a2e6 100644 --- a/src/onig-reg-exp.h +++ b/src/onig-reg-exp.h @@ -1,10 +1,12 @@ #ifndef SRC_ONIG_REG_EXP_H_ #define SRC_ONIG_REG_EXP_H_ +#include #include #include "oniguruma.h" +using ::std::shared_ptr; using ::std::string; class OnigResult; @@ -16,7 +18,7 @@ class OnigRegExp { bool Contains(const string& value); int LocationAt(int index); - OnigResult *Search(const string &searchString, size_t position); + shared_ptr Search(const string &searchString, size_t position); private: OnigRegExp(const OnigRegExp&); // Disallow copying diff --git a/src/onig-scanner.cc b/src/onig-scanner.cc index cb71cbb..e5cd4ca 100644 --- a/src/onig-scanner.cc +++ b/src/onig-scanner.cc @@ -76,16 +76,16 @@ Handle OnigScanner::FindNextMatch(Handle v8String, Handle OnigRegExp *regExp = (*iter).get(); bool useCachedResult = false; - OnigResult *result = NULL; + shared_ptr result; if (useCachedResults && index <= maxCachedIndex) { - result = cachedResults[index].get(); - useCachedResult = (result == NULL || result->LocationAt(0) >= charOffset); + result = cachedResults[index]; + useCachedResult = (!result || result->LocationAt(0) >= charOffset); } if (!useCachedResult) { result = regExp->Search(string, byteOffset); - cachedResults[index] = shared_ptr(result); + cachedResults[index] = result; maxCachedIndex = index; } @@ -93,7 +93,7 @@ Handle OnigScanner::FindNextMatch(Handle v8String, Handle int location = result->LocationAt(0); if (bestIndex == -1 || location < bestLocation) { bestLocation = location; - bestResult = result; + bestResult = result.get(); bestIndex = index; } From 167e8155732ffb067528b28de4625cb220c5c5f8 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Mar 2014 21:33:50 +0800 Subject: [PATCH 6/8] Do member data initialization in initializer list. --- src/onig-result.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/onig-result.cc b/src/onig-result.cc index 0414c92..20fb219 100644 --- a/src/onig-result.cc +++ b/src/onig-result.cc @@ -2,8 +2,9 @@ #include "onig-result.h" #include "unicode-utils.h" -OnigResult::OnigResult(OnigRegion* region, const string& searchString) : searchString_(searchString) { - region_ = region; +OnigResult::OnigResult(OnigRegion* region, const string& searchString) + : searchString_(searchString), + region_(region) { } OnigResult::~OnigResult() { From 9b70069cc36ffb4a7e057749a7e8c7eeb5a435c5 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 12 Mar 2014 09:43:21 +0800 Subject: [PATCH 7/8] Actually use cache. Previously the condition to use cache is never met. --- src/onig-scanner.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onig-scanner.cc b/src/onig-scanner.cc index e5cd4ca..6405662 100644 --- a/src/onig-scanner.cc +++ b/src/onig-scanner.cc @@ -78,7 +78,7 @@ Handle OnigScanner::FindNextMatch(Handle v8String, Handle bool useCachedResult = false; shared_ptr result; - if (useCachedResults && index <= maxCachedIndex) { + if (index <= maxCachedIndex) { result = cachedResults[index]; useCachedResult = (!result || result->LocationAt(0) >= charOffset); } From 1086d8ad115e1421a529d3e97b98dc83990d6b81 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 12 Mar 2014 09:52:45 +0800 Subject: [PATCH 8/8] Revert "Actually use cache." This reverts commit 9b70069cc36ffb4a7e057749a7e8c7eeb5a435c5. --- src/onig-scanner.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onig-scanner.cc b/src/onig-scanner.cc index 6405662..e5cd4ca 100644 --- a/src/onig-scanner.cc +++ b/src/onig-scanner.cc @@ -78,7 +78,7 @@ Handle OnigScanner::FindNextMatch(Handle v8String, Handle bool useCachedResult = false; shared_ptr result; - if (index <= maxCachedIndex) { + if (useCachedResults && index <= maxCachedIndex) { result = cachedResults[index]; useCachedResult = (!result || result->LocationAt(0) >= charOffset); }