From b48c84e15aa4fed58e3a16d519e5ee63c9981c69 Mon Sep 17 00:00:00 2001 From: Robin Date: Fri, 26 Feb 2021 08:27:40 -0600 Subject: [PATCH] fix: memory leaks using unique_ptr --- include/expression.hpp | 15 +++--- include/parse.hpp | 2 +- plugins/ByteBeat/ByteBeat.cpp | 9 +--- plugins/ByteBeat/ByteBeat.hpp | 3 +- src/main.cpp | 2 +- src/parse.cpp | 80 ++++++++++++++--------------- test/test_expression.cpp | 96 +++++++++++++++++------------------ test/test_parse.cpp | 18 +++---- 8 files changed, 107 insertions(+), 118 deletions(-) diff --git a/include/expression.hpp b/include/expression.hpp index 6ded613..45ca7d5 100644 --- a/include/expression.hpp +++ b/include/expression.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include using namespace std; @@ -14,6 +15,8 @@ namespace bb virtual string to_string() const = 0; }; + using ExpressionPtr = unique_ptr; + class Variable : public Expression { public: @@ -50,13 +53,7 @@ namespace bb class BinaryExpression : public Expression { public: - explicit BinaryExpression(Expression *left, Expression *right) : left(left), right(right){}; - - ~BinaryExpression() - { - delete left; - delete right; - } + explicit BinaryExpression(ExpressionPtr left, ExpressionPtr right) : left(move(left)), right(move(right)){}; string to_string() const { @@ -66,8 +63,8 @@ namespace bb protected: virtual string operand() const = 0; - Expression *left; - Expression *right; + ExpressionPtr left; + ExpressionPtr right; }; class Add : public BinaryExpression diff --git a/include/parse.hpp b/include/parse.hpp index bdd179e..22313f7 100644 --- a/include/parse.hpp +++ b/include/parse.hpp @@ -8,5 +8,5 @@ using namespace std; namespace bb { - Expression *parse(string &input); + ExpressionPtr parse(string &input); } diff --git a/plugins/ByteBeat/ByteBeat.cpp b/plugins/ByteBeat/ByteBeat.cpp index ae234ac..1bc62ec 100644 --- a/plugins/ByteBeat/ByteBeat.cpp +++ b/plugins/ByteBeat/ByteBeat.cpp @@ -21,12 +21,7 @@ namespace ByteBeat // 8-bit unsigned integers, then transformed to floats with a range of // +/-1.0. An expression that emits a constant value of 128 will // correspond roughly to 0.0 at the output. - mExpression = new bb::Constant(128); - } - - ByteBeat::~ByteBeat() - { - delete mExpression; + mExpression = bb::ExpressionPtr(new bb::Constant(128)); } void ByteBeat::parse(const char *input) @@ -35,9 +30,7 @@ namespace ByteBeat try { - bb::Expression *prevExpr = mExpression; mExpression = bb::parse(s); - delete prevExpr; } catch (invalid_argument &ex) { diff --git a/plugins/ByteBeat/ByteBeat.hpp b/plugins/ByteBeat/ByteBeat.hpp index 1388073..e7ddfc8 100644 --- a/plugins/ByteBeat/ByteBeat.hpp +++ b/plugins/ByteBeat/ByteBeat.hpp @@ -22,7 +22,6 @@ namespace ByteBeat { public: ByteBeat(); - ~ByteBeat(); /** * Parse the incoming expression and replace the existing expression. @@ -42,6 +41,6 @@ namespace ByteBeat int mPrevT = 0; /** bytebeat expression used to generate audio samples */ - bb::Expression *mExpression; + bb::ExpressionPtr mExpression; }; } diff --git a/src/main.cpp b/src/main.cpp index 7d602f6..e8aa87c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -23,7 +23,7 @@ int main(int argc, char *argv[]) } string input{argv[1]}; - Expression *expr; + ExpressionPtr expr; try { expr = parse(input); diff --git a/src/parse.cpp b/src/parse.cpp index e8d56c3..a1d5560 100644 --- a/src/parse.cpp +++ b/src/parse.cpp @@ -7,17 +7,17 @@ namespace bb { using TokenIter = vector::iterator; - Expression *parse_expression(TokenIter &it, TokenIter &end); - Expression *parse_expression_inner(TokenIter &it, - TokenIter &end, - Expression *lhs, - int min_precedence); - Expression *parse_primary(TokenIter &it, TokenIter &end); + ExpressionPtr parse_expression(TokenIter &it, TokenIter &end); + ExpressionPtr parse_expression_inner(TokenIter &it, + TokenIter &end, + ExpressionPtr lhs, + int min_precedence); + ExpressionPtr parse_primary(TokenIter &it, TokenIter &end); int get_precedence(string &token); - Expression *make_binary_op(string &op, Expression *lhs, Expression *rhs); + ExpressionPtr make_binary_op(string &op, ExpressionPtr lhs, ExpressionPtr rhs); - Expression *parse(string &input) + ExpressionPtr parse(string &input) { vector tokens = tokenize(input); if (tokens.empty()) @@ -27,7 +27,7 @@ namespace bb auto it = tokens.begin(); auto end = tokens.end(); - Expression *expr = parse_expression(it, end); + ExpressionPtr expr = parse_expression(it, end); if (it != tokens.end()) { throw invalid_argument("not all tokens consumed"); @@ -36,16 +36,16 @@ namespace bb return expr; } - Expression *parse_expression(TokenIter &it, TokenIter &end) + ExpressionPtr parse_expression(TokenIter &it, TokenIter &end) { - Expression *lhs = parse_primary(it, end); - return parse_expression_inner(it, end, lhs, 0); + ExpressionPtr lhs = parse_primary(it, end); + return parse_expression_inner(it, end, move(lhs), 0); } - Expression *parse_expression_inner(TokenIter &it, - TokenIter &end, - Expression *lhs, - int min_precedence) + ExpressionPtr parse_expression_inner(TokenIter &it, + TokenIter &end, + ExpressionPtr lhs, + int min_precedence) { if (it == end) { @@ -60,10 +60,10 @@ namespace bb string op = lookahead; ++it; - Expression *rhs = parse_primary(it, end); + ExpressionPtr rhs = parse_primary(it, end); if (it == end) { - return make_binary_op(op, lhs, rhs); + return make_binary_op(op, move(lhs), move(rhs)); } lookahead = *it; @@ -71,7 +71,7 @@ namespace bb while (next_precedence > precedence) { - rhs = parse_expression_inner(it, end, rhs, next_precedence); + rhs = parse_expression_inner(it, end, move(rhs), next_precedence); if (it == end) { next_precedence = -1; @@ -83,13 +83,13 @@ namespace bb } precedence = next_precedence; - lhs = make_binary_op(op, lhs, rhs); + lhs = make_binary_op(op, move(lhs), move(rhs)); } return lhs; } - Expression *parse_primary(TokenIter &it, TokenIter &end) + ExpressionPtr parse_primary(TokenIter &it, TokenIter &end) { if (it == end) { @@ -101,7 +101,7 @@ namespace bb if (token == "t") { ++it; - return new Variable(); + return ExpressionPtr(new Variable()); } if (token == "(") @@ -120,7 +120,7 @@ namespace bb { int n = stoi(token); ++it; - return new Constant(n); + return ExpressionPtr(new Constant(n)); } catch (invalid_argument &ex) { @@ -166,86 +166,86 @@ namespace bb return -1; } - Expression *make_binary_op(std::string &op, Expression *lhs, Expression *rhs) + ExpressionPtr make_binary_op(std::string &op, ExpressionPtr lhs, ExpressionPtr rhs) { if (op == "+") { - return new Add(lhs, rhs); + return ExpressionPtr(new Add(move(lhs), move(rhs))); } if (op == "-") { - return new Subtract(lhs, rhs); + return ExpressionPtr(new Subtract(move(lhs), move(rhs))); } if (op == "*") { - return new Multiply(lhs, rhs); + return ExpressionPtr(new Multiply(move(lhs), move(rhs))); } if (op == "/") { - return new Divide(lhs, rhs); + return ExpressionPtr(new Divide(move(lhs), move(rhs))); } if (op == "%") { - return new Modulo(lhs, rhs); + return ExpressionPtr(new Modulo(move(lhs), move(rhs))); } if (op == "&") { - return new BitwiseAnd(lhs, rhs); + return ExpressionPtr(new BitwiseAnd(move(lhs), move(rhs))); } if (op == "|") { - return new BitwiseOr(lhs, rhs); + return ExpressionPtr(new BitwiseOr(move(lhs), move(rhs))); } if (op == "^") { - return new BitwiseXor(lhs, rhs); + return ExpressionPtr(new BitwiseXor(move(lhs), move(rhs))); } if (op == "<<") { - return new BitwiseShiftLeft(lhs, rhs); + return ExpressionPtr(new BitwiseShiftLeft(move(lhs), move(rhs))); } if (op == ">>") { - return new BitwiseShiftRight(lhs, rhs); + return ExpressionPtr(new BitwiseShiftRight(move(lhs), move(rhs))); } if (op == "<") { - return new LessThan(lhs, rhs); + return ExpressionPtr(new LessThan(move(lhs), move(rhs))); } if (op == ">") { - return new GreaterThan(lhs, rhs); + return ExpressionPtr(new GreaterThan(move(lhs), move(rhs))); } if (op == "<=") { - return new LessThanOrEqual(lhs, rhs); + return ExpressionPtr(new LessThanOrEqual(move(lhs), move(rhs))); } if (op == ">=") { - return new GreaterThanOrEqual(lhs, rhs); + return ExpressionPtr(new GreaterThanOrEqual(move(lhs), move(rhs))); } if (op == "==") { - return new Equal(lhs, rhs); + return ExpressionPtr(new Equal(move(lhs), move(rhs))); } if (op == "!=") { - return new NotEqual(lhs, rhs); + return ExpressionPtr(new NotEqual(move(lhs), move(rhs))); } throw invalid_argument("unrecognized operator"); diff --git a/test/test_expression.cpp b/test/test_expression.cpp index e703e61..cd88d26 100644 --- a/test/test_expression.cpp +++ b/test/test_expression.cpp @@ -18,79 +18,79 @@ TEST_CASE("expression", "[expression]") SECTION("add") { - Expression *left = new Variable(); - Expression *right = new Constant(99); - REQUIRE(Add(left, right).evaluate(1) == 100); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(99)); + REQUIRE(Add(move(left), move(right)).evaluate(1) == 100); } SECTION("subtract") { - Expression *left = new Variable(); - Expression *right = new Constant(99); - REQUIRE(Subtract(left, right).evaluate(100) == 1); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(99)); + REQUIRE(Subtract(move(left), move(right)).evaluate(100) == 1); } SECTION("multiply") { - Expression *left = new Variable(); - Expression *right = new Constant(2); - REQUIRE(Multiply(left, right).evaluate(2) == 4); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(2)); + REQUIRE(Multiply(move(left), move(right)).evaluate(2) == 4); } SECTION("divide") { - Expression *left = new Variable(); - Expression *right = new Constant(2); - REQUIRE(Divide(left, right).evaluate(3) == 1); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(2)); + REQUIRE(Divide(move(left), move(right)).evaluate(3) == 1); } SECTION("modulo") { - Expression *left = new Variable(); - Expression *right = new Constant(2); - REQUIRE(Modulo(left, right).evaluate(3) == 1); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(2)); + REQUIRE(Modulo(move(left), move(right)).evaluate(3) == 1); } SECTION("bitwise and") { - Expression *left = new Variable(); - Expression *right = new Constant(2); - REQUIRE(BitwiseAnd(left, right).evaluate(1) == 0); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(2)); + REQUIRE(BitwiseAnd(move(left), move(right)).evaluate(1) == 0); } SECTION("bitwise or") { - Expression *left = new Variable(); - Expression *right = new Constant(2); - REQUIRE(BitwiseOr(left, right).evaluate(1) == 3); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(2)); + REQUIRE(BitwiseOr(move(left), move(right)).evaluate(1) == 3); } SECTION("bitwise xor") { - Expression *left = new Variable(); - Expression *right = new Constant(1); - REQUIRE(BitwiseXor(left, right).evaluate(1) == 0); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(1)); + REQUIRE(BitwiseXor(move(left), move(right)).evaluate(1) == 0); } SECTION("bitwise shift left") { - Expression *left = new Variable(); - Expression *right = new Constant(1); - REQUIRE(BitwiseShiftLeft(left, right).evaluate(1) == 2); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(1)); + REQUIRE(BitwiseShiftLeft(move(left), move(right)).evaluate(1) == 2); } SECTION("bitwise shift right") { - Expression *left = new Variable(); - Expression *right = new Constant(1); - REQUIRE(BitwiseShiftRight(left, right).evaluate(2) == 1); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(1)); + REQUIRE(BitwiseShiftRight(move(left), move(right)).evaluate(2) == 1); } SECTION("less than") { - Expression *left = new Variable(); - Expression *right = new Constant(2); - LessThan lt(left, right); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(2)); + LessThan lt(move(left), move(right)); REQUIRE(lt.evaluate(1) == 1); REQUIRE(lt.evaluate(2) == 0); REQUIRE(lt.evaluate(3) == 0); @@ -98,9 +98,9 @@ TEST_CASE("expression", "[expression]") SECTION("greater than") { - Expression *left = new Variable(); - Expression *right = new Constant(1); - GreaterThan gt(left, right); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(1)); + GreaterThan gt(move(left), move(right)); REQUIRE(gt.evaluate(2) == 1); REQUIRE(gt.evaluate(1) == 0); REQUIRE(gt.evaluate(0) == 0); @@ -108,9 +108,9 @@ TEST_CASE("expression", "[expression]") SECTION("less than or equal") { - Expression *left = new Variable(); - Expression *right = new Constant(1); - LessThanOrEqual lte(left, right); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(1)); + LessThanOrEqual lte(move(left), move(right)); REQUIRE(lte.evaluate(0) == 1); REQUIRE(lte.evaluate(1) == 1); REQUIRE(lte.evaluate(2) == 0); @@ -118,9 +118,9 @@ TEST_CASE("expression", "[expression]") SECTION("greater than or equal") { - Expression *left = new Variable(); - Expression *right = new Constant(1); - GreaterThanOrEqual gte(left, right); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(1)); + GreaterThanOrEqual gte(move(left), move(right)); REQUIRE(gte.evaluate(2) == 1); REQUIRE(gte.evaluate(1) == 1); REQUIRE(gte.evaluate(0) == 0); @@ -128,18 +128,18 @@ TEST_CASE("expression", "[expression]") SECTION("equal") { - Expression *left = new Variable(); - Expression *right = new Constant(1); - Equal eq(left, right); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(1)); + Equal eq(move(left), move(right)); REQUIRE(eq.evaluate(1) == 1); REQUIRE(eq.evaluate(2) == 0); } SECTION("not equal") { - Expression *left = new Variable(); - Expression *right = new Constant(1); - NotEqual neq(left, right); + ExpressionPtr left = ExpressionPtr(new Variable()); + ExpressionPtr right = ExpressionPtr(new Constant(1)); + NotEqual neq(move(left), move(right)); REQUIRE(neq.evaluate(1) == 0); REQUIRE(neq.evaluate(2) == 1); } diff --git a/test/test_parse.cpp b/test/test_parse.cpp index 3bf55d5..11d409c 100644 --- a/test/test_parse.cpp +++ b/test/test_parse.cpp @@ -11,7 +11,7 @@ TEST_CASE("parse", "[parse]") SECTION("simple expression") { string input = "t+1"; - Expression *expr = parse(input); + auto expr = parse(input); REQUIRE(expr->to_string() == "(t+1)"); REQUIRE(expr->evaluate(2) == 3); } @@ -19,7 +19,7 @@ TEST_CASE("parse", "[parse]") SECTION("simple expression w/o parentheses") { string input = "t+2*t"; - Expression *expr = parse(input); + auto expr = parse(input); REQUIRE(expr->to_string() == "(t+(2*t))"); REQUIRE(expr->evaluate(2) == 6); } @@ -27,7 +27,7 @@ TEST_CASE("parse", "[parse]") SECTION("simple expression w/ parentheses") { string input = "(t+1)*t"; - Expression *expr = parse(input); + auto expr = parse(input); REQUIRE(expr->to_string() == "((t+1)*t)"); REQUIRE(expr->evaluate(2) == 6); } @@ -59,7 +59,7 @@ TEST_CASE("parse", "[parse]") SECTION("atomic expression") { string input = "t"; - Expression *expr = parse(input); + auto expr = parse(input); REQUIRE(expr->to_string() == "t"); REQUIRE(expr->evaluate(42) == 42); } @@ -67,7 +67,7 @@ TEST_CASE("parse", "[parse]") SECTION("multiple expressions") { string input = "(t+3)|(t*2)"; - Expression *expr = parse(input); + auto expr = parse(input); REQUIRE(expr->to_string() == "((t+3)|(t*2))"); REQUIRE(expr->evaluate(1) == 6); } @@ -75,7 +75,7 @@ TEST_CASE("parse", "[parse]") SECTION("precedence") { string input = "t*2+t*3"; - Expression *expr = parse(input); + auto expr = parse(input); REQUIRE(expr->to_string() == "((t*2)+(t*3))"); REQUIRE(expr->evaluate(1) == 5); } @@ -83,7 +83,7 @@ TEST_CASE("parse", "[parse]") SECTION("deep nesting") { string input = "(((t*t)))"; - Expression *expr = parse(input); + auto expr = parse(input); REQUIRE(expr->to_string() == "(t*t)"); REQUIRE(expr->evaluate(2) == 4); } @@ -91,7 +91,7 @@ TEST_CASE("parse", "[parse]") SECTION("parse crowd") { string input = "((t<<1)^((t<<1)+(t>>7)&t>>12))|t>>(4-(1^7&(t>>19)))|t>>7"; - Expression *expr = parse(input); + auto expr = parse(input); REQUIRE(expr); } @@ -104,7 +104,7 @@ TEST_CASE("parse", "[parse]") return parse(input); }; - Expression *crowd = parse(input); + auto crowd = parse(input); CHECK(crowd); int t = 0;