kindred/create
1
1
Fork 0
Code Issues 43 Pull Requests Actions 2 Packages Projects 9 Releases 2 Wiki Activity

Spreadsheet: make setContent use unique_ptr and cleanup

Browse Source 
Make `Cell::setContent` use `unique_ptr` and
`make_unique` for expressions to avoid potential memory
leaks.
Also renames `expo` to `newExpr` to avoid mixup with the
member variable `expression`.

Both changes was made at the request of @chennes.
This commit is contained in:
Benjamin Nauck
2021-01-19 02:55:49 +01:00
parent 991e8bfb2b
commit 8589c4e782
1 changed files with 18 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
src/Mod/Spreadsheet/App/Cell.cpp
View File

@@ -38,6 +38,7 @@
#include <Base/UnitsApi.h>
#include <Base/Writer.h>
#include <Base/Console.h>
#include <Base/StdStlTools.h>
#include <App/ExpressionParser.h>
#include "Sheet.h"
#include <iomanip>
@@ -280,10 +281,10 @@ void Cell::afterRestore() {
void Cell::setContent(const char * value)
{
PropertySheet::AtomicPropertyChange signaller(*owner);
App::Expression * expr = nullptr;
ExpressionPtr newExpr;
clearException();
if (value != nullptr) {
if (value) {
if (owner->sheet()->isRestoring()) {
expression.reset(new App::StringExpression(owner->sheet(), value));
setUsed(EXPRESSION_SET, true);
@@ -291,15 +292,15 @@ void Cell::setContent(const char * value)
}
if (*value == '=') {
try {
expr = App::ExpressionParser::parse(owner->sheet(), value + 1);
newExpr = ExpressionPtr(App::ExpressionParser::parse(owner->sheet(), value + 1));
}
catch (Base::Exception & e) {
expr = new App::StringExpression(owner->sheet(), value);
newExpr = std::make_unique<App::StringExpression>(owner->sheet(), value);
setParseException(e.what());
}
}
else if (*value == '\'') {
expr = new App::StringExpression(owner->sheet(), value + 1);
newExpr = std::make_unique<App::StringExpression>(owner->sheet(), value + 1);
}
else if (*value != '\0') {
// check if value is just a number
@@ -309,22 +310,22 @@ void Cell::setContent(const char * value)
if (errno == 0) {
const bool isEndEmpty = *end == '\0' || strspn(end, " \t\n\r") == strlen(end);
if (isEndEmpty) {
expr = new App::NumberExpression(owner->sheet(), Quantity(float_value));
newExpr = std::make_unique<App::NumberExpression>(owner->sheet(), Quantity(float_value));
}
}
// if not a float, check if it is a quantity or compatible fraction
const bool isStartingWithNumber = value != end;
if (expr == nullptr && isStartingWithNumber) {
if (!newExpr && isStartingWithNumber) {
try {
auto parsedExpr = App::ExpressionParser::parse(owner->sheet(), value);
ExpressionPtr parsedExpr(App::ExpressionParser::parse(owner->sheet(), value));
if (const auto fraction = freecad_dynamic_cast<OperatorExpression>(parsedExpr)) {
if (const auto fraction = freecad_dynamic_cast<OperatorExpression>(parsedExpr.get())) {
if (fraction->getOperator() == OperatorExpression::UNIT) {
const auto left = freecad_dynamic_cast<NumberExpression>(fraction->getLeft());
const auto right = freecad_dynamic_cast<UnitExpression>(fraction->getRight());
if (left && right) {
expr = parsedExpr;
newExpr = std::move(parsedExpr);
}
}
else if (fraction->getOperator() == OperatorExpression::DIV) {
@@ -348,33 +349,29 @@ void Cell::setContent(const char * value)
const bool isNomValid = isNumberNom || isQuantityNom;
const bool isDenomValid = isNumberDenom || isQuantityDenom || isUnitDenom;
if (isNomValid && isDenomValid) {
expr = parsedExpr;
newExpr = std::move(parsedExpr);
}
}
}
else if (const auto number = freecad_dynamic_cast<NumberExpression>(parsedExpr)) {
else if (const auto number = freecad_dynamic_cast<NumberExpression>(parsedExpr.get())) {
// NumbersExpressions can accept more than can be parsed with strtod.
// Example: 12.34 and 12,34 are both valid NumberExpressions
expr = parsedExpr;
}
if (expr == nullptr) {
delete parsedExpr;
newExpr = std::move(parsedExpr);
}
}
catch (...) {}
}
}
if (expr == nullptr && *value != '\0') {
expr = new App::StringExpression(owner->sheet(), value);
if (!newExpr && *value != '\0') {
newExpr = std::make_unique<App::StringExpression>(owner->sheet(), value);
}
// trying to add an empty string will make expr = nullptr
// trying to add an empty string will make newExpr = nullptr
}
// set expression, or delete the current expression by setting nullptr if empty string was entered
setExpression(App::ExpressionPtr(expr));
setExpression(std::move(newExpr));
signaller.tryInvoke();
}