Partially resolve google/benchmark#17 by fixing regular expression leak.

This adds a unit test to validate the wrapper without running the entirety of
benchmark_test.
This commit is contained in:
Chris Kennelly 2014-04-23 00:56:17 -07:00
parent 6087edda9d
commit fa908926c7
6 changed files with 162 additions and 16 deletions

View File

@ -1,6 +1,8 @@
set(SOURCE_FILES "benchmark.cc" "colorprint.cc" "commandlineflags.cc" "sleep.cc" "sysinfo.cc" "walltime.cc")
set(RE_FILES "re.cc")
add_library(benchmark STATIC ${SOURCE_FILES})
add_library(benchmark_re STATIC ${RE_FILES})
add_library(benchmark STATIC ${SOURCE_FILES} ${RE_FILES})
# Install target (will install the library to specified CMAKE_INSTALL_PREFIX variable)
install(

View File

@ -17,6 +17,7 @@
#include "colorprint.h"
#include "commandlineflags.h"
#include "mutex_lock.h"
#include "re.h"
#include "sleep.h"
#include "stat.h"
#include "sysinfo.h"
@ -27,12 +28,6 @@
#include <semaphore.h>
#include <string.h>
#if defined OS_FREEBSD
#include <gnuregex.h>
#else
#include <regex.h>
#endif
#include <algorithm>
#include <atomic>
#include <iostream>
@ -745,14 +740,10 @@ std::vector<Benchmark::Instance> Benchmark::CreateBenchmarkInstances(
void Benchmark::FindBenchmarks(const std::string& spec,
std::vector<Instance>* benchmarks) {
// Make regular expression out of command-line flag
regex_t re;
int ec = regcomp(&re, spec.c_str(), REG_EXTENDED | REG_NOSUB);
if (ec != 0) {
size_t needed = regerror(ec, &re, NULL, 0);
char* errbuf = new char[needed];
regerror(ec, &re, errbuf, needed);
std::cerr << "Could not compile benchmark re: " << errbuf << "\n";
delete[] errbuf;
Regex re;
std::string re_error;
if (!re.Init(spec, &re_error)) {
std::cerr << "Could not compile benchmark re: " << re_error << std::endl;
return;
}
@ -762,7 +753,7 @@ void Benchmark::FindBenchmarks(const std::string& spec,
if (family == nullptr) continue; // Family was deleted
// Match against filter.
if (regexec(&re, family->name_.c_str(), 0, NULL, 0) != 0) {
if (!re.Match(family->name_)) {
#ifdef DEBUG
std::cout << "Skipping " << family->name_ << "\n";
#endif

59
src/re.cc Normal file
View File

@ -0,0 +1,59 @@
// Copyright 2014 Google Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include <benchmark/macros.h>
#include "re.h"
namespace benchmark {
Regex::Regex() : init_(false) { }
bool Regex::Init(const std::string& spec, std::string* error) {
int ec = regcomp(&re_, spec.c_str(), REG_EXTENDED | REG_NOSUB);
if (ec != 0) {
if (error) {
size_t needed = regerror(ec, &re_, NULL, 0);
char* errbuf = new char[needed];
regerror(ec, &re_, errbuf, needed);
// regerror returns the number of bytes necessary to null terminate
// the string, so we move that when assigning to error.
CHECK_NE(needed, 0);
error->assign(errbuf, needed - 1);
delete[] errbuf;
}
return false;
}
init_ = true;
return true;
}
Regex::~Regex() {
if (init_) {
regfree(&re_);
}
}
bool Regex::Match(const std::string& str) {
if (!init_) {
return false;
}
return regexec(&re_, str.c_str(), 0, NULL, 0) == 0;
}
} // end namespace benchmark

50
src/re.h Normal file
View File

@ -0,0 +1,50 @@
// Copyright 2014 Google Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#ifndef BENCHMARK_RE_H_
#define BENCHMARK_RE_H_
#if defined OS_FREEBSD
#include <gnuregex.h>
#else
#include <regex.h>
#endif
#include <string>
namespace benchmark {
// A wrapper around the POSIX regular expression API that provides automatic
// cleanup
class Regex {
public:
Regex();
~Regex();
// Compile a regular expression matcher from spec. Returns true on success.
//
// On failure (and if error is not NULL), error is populated with a human
// readable error message if an error occurs.
bool Init(const std::string& spec, std::string* error);
// Returns whether str matches the compiled regular expression.
bool Match(const std::string& str);
private:
bool init_;
// Underlying regular expression object
regex_t re_;
};
} // end namespace benchmark
#endif // BENCHMARK_RE_H_

View File

@ -1,3 +1,8 @@
# Demonstration executable
add_executable(benchmark_test benchmark_test.cc)
target_link_libraries(benchmark_test benchmark ${CMAKE_THREAD_LIBS_INIT})
# Test harness for regex wrapper
add_executable(re_test ${RE_FILES} "re_test.cc")
target_link_libraries(re_test benchmark_re ${CMAKE_THREAD_LIBS_INIT} gtest gtest_main)
add_test(regex re_test)

39
test/re_test.cc Normal file
View File

@ -0,0 +1,39 @@
// Copyright 2014 Google Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include <gtest/gtest.h>
#include "re.h"
TEST(Regex, RegexSimple) {
benchmark::Regex re;
EXPECT_TRUE(re.Init("a+", NULL));
EXPECT_FALSE(re.Match(""));
EXPECT_TRUE(re.Match("a"));
EXPECT_TRUE(re.Match("aa"));
EXPECT_FALSE(re.Match("b"));
}
TEST(Regex, InvalidNoErrorMessage) {
benchmark::Regex re;
EXPECT_FALSE(re.Init("[", NULL));
}
TEST(Regex, Invalid) {
std::string error;
benchmark::Regex re;
EXPECT_FALSE(re.Init("[", &error));
EXPECT_NE("", error);
}