From deae0ac01274b339b7bc6dfbf0ab19d3a6c40b60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Offringa?= <offringa@astron.nl>
Date: Fri, 16 Mar 2012 20:27:03 +0000
Subject: [PATCH] Task #1892: fixing bug causing strategy reader to malfunction
with different locales
---
.gitattributes | 3 +
.../strategy/control/strategyreader.h | 4 +-
.../AOFlagger/test/util/numberparsertest.h | 72 +++++++
.../AOFlagger/test/util/utiltestgroup.h | 37 ++++
.../include/AOFlagger/util/numberparser.h | 188 ++++++++++++++++++
.../src/strategy/control/strategyreader.cpp | 14 +-
CEP/DP3/AOFlagger/test/aotest.cpp | 6 +
7 files changed, 318 insertions(+), 6 deletions(-)
create mode 100644 CEP/DP3/AOFlagger/include/AOFlagger/test/util/numberparsertest.h
create mode 100644 CEP/DP3/AOFlagger/include/AOFlagger/test/util/utiltestgroup.h
create mode 100644 CEP/DP3/AOFlagger/include/AOFlagger/util/numberparser.h
diff --git a/.gitattributes b/.gitattributes
index 955bb687a19..3c6a54f7234 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -500,6 +500,8 @@ CEP/DP3/AOFlagger/include/AOFlagger/test/testingtools/lessthanasserter.h -text
CEP/DP3/AOFlagger/include/AOFlagger/test/testingtools/testgroup.h -text
CEP/DP3/AOFlagger/include/AOFlagger/test/testingtools/testitem.h -text
CEP/DP3/AOFlagger/include/AOFlagger/test/testingtools/unittest.h -text
+CEP/DP3/AOFlagger/include/AOFlagger/test/util/numberparsertest.h -text
+CEP/DP3/AOFlagger/include/AOFlagger/test/util/utiltestgroup.h -text
CEP/DP3/AOFlagger/include/AOFlagger/types.h -text
CEP/DP3/AOFlagger/include/AOFlagger/util/aologger.h -text
CEP/DP3/AOFlagger/include/AOFlagger/util/autoarray.h -text
@@ -507,6 +509,7 @@ CEP/DP3/AOFlagger/include/AOFlagger/util/compress.h -text
CEP/DP3/AOFlagger/include/AOFlagger/util/ffttools.h -text
CEP/DP3/AOFlagger/include/AOFlagger/util/integerdomain.h -text
CEP/DP3/AOFlagger/include/AOFlagger/util/multiplot.h -text
+CEP/DP3/AOFlagger/include/AOFlagger/util/numberparser.h -text
CEP/DP3/AOFlagger/include/AOFlagger/util/parameter.h -text
CEP/DP3/AOFlagger/include/AOFlagger/util/plot.h -text
CEP/DP3/AOFlagger/include/AOFlagger/util/progresslistener.h -text
diff --git a/CEP/DP3/AOFlagger/include/AOFlagger/strategy/control/strategyreader.h b/CEP/DP3/AOFlagger/include/AOFlagger/strategy/control/strategyreader.h
index f0944b12bf9..57165808e52 100644
--- a/CEP/DP3/AOFlagger/include/AOFlagger/strategy/control/strategyreader.h
+++ b/CEP/DP3/AOFlagger/include/AOFlagger/strategy/control/strategyreader.h
@@ -1,6 +1,6 @@
/***************************************************************************
- * Copyright (C) 2008 by A.R. Offringa *
- * offringa@astro.rug.nl *
+ * Copyright (C) 2012 by A.R. Offringa *
+ * offringa@astro.rug.nl *
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
diff --git a/CEP/DP3/AOFlagger/include/AOFlagger/test/util/numberparsertest.h b/CEP/DP3/AOFlagger/include/AOFlagger/test/util/numberparsertest.h
new file mode 100644
index 00000000000..6dc17cf96d2
--- /dev/null
+++ b/CEP/DP3/AOFlagger/include/AOFlagger/test/util/numberparsertest.h
@@ -0,0 +1,72 @@
+/***************************************************************************
+ * Copyright (C) 2008 by A.R. Offringa *
+ * offringa@astro.rug.nl *
+ * *
+ * This program is free software; you can redistribute it and/or modify *
+ * it under the terms of the GNU General Public License as published by *
+ * the Free Software Foundation; either version 2 of the License, or *
+ * (at your option) any later version. *
+ * *
+ * This program is distributed in the hope that it will be useful, *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
+ * GNU General Public License for more details. *
+ * *
+ * You should have received a copy of the GNU General Public License *
+ * along with this program; if not, write to the *
+ * Free Software Foundation, Inc., *
+ * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
+ ***************************************************************************/
+#ifndef AOFLAGGER_NUMBERPARSERTEST_H
+#define AOFLAGGER_NUMBERPARSERTEST_H
+
+#include <AOFlagger/test/testingtools/asserter.h>
+#include <AOFlagger/test/testingtools/unittest.h>
+
+#include <AOFlagger/util/numberparser.h>
+
+class NumberParserTest : public UnitTest {
+ public:
+ NumberParserTest() : UnitTest("Number parser")
+ {
+ AddTest(TestToDouble(), "Parsing double");
+ }
+
+ private:
+ struct TestToDouble : public Asserter
+ {
+ void operator()();
+ };
+};
+
+inline void NumberParserTest::TestToDouble::operator()()
+{
+ AssertEquals<double>(NumberParser::ToDouble("1"), 1.0);
+ AssertEquals<double>(NumberParser::ToDouble("1."), 1.0);
+ AssertEquals<double>(NumberParser::ToDouble("1.000000"), 1.0);
+ AssertEquals<double>(NumberParser::ToDouble("-1"), -1.0);
+ AssertEquals<double>(NumberParser::ToDouble("-1.00000"), -1.0);
+
+ AssertEquals<double>(NumberParser::ToDouble("3.14159265"), 3.14159265);
+ AssertEquals<double>(NumberParser::ToDouble("0.00002"), 0.00002);
+ AssertEquals<double>(NumberParser::ToDouble("234567"), 234567.0);
+ AssertEquals<double>(NumberParser::ToDouble("234.567"), 234.567);
+
+ AssertEquals<double>(NumberParser::ToDouble("0"), 0.0);
+ AssertEquals<double>(NumberParser::ToDouble("0.0"), 0.0);
+ AssertEquals<double>(NumberParser::ToDouble("-0.0"), 0.0);
+
+ AssertEquals<double>(NumberParser::ToDouble("0.0e5"), 0.0);
+ AssertEquals<double>(NumberParser::ToDouble("0.0e100"), 0.0);
+ AssertEquals<double>(NumberParser::ToDouble("0.0e-100"), 0.0);
+ AssertEquals<double>(NumberParser::ToDouble("0.0E5"), 0.0);
+ AssertEquals<double>(NumberParser::ToDouble("0.0E100"), 0.0);
+ AssertEquals<double>(NumberParser::ToDouble("0.0E-100"), 0.0);
+
+ AssertAlmostEqual(NumberParser::ToDouble("1.0e5"), 1.0e5);
+ AssertAlmostEqual(NumberParser::ToDouble("1.0e-5"), 1.0e-5);
+ AssertAlmostEqual(NumberParser::ToDouble("0.3e0"), 0.3);
+ AssertAlmostEqual(NumberParser::ToDouble("642.135e8"), 642.135e8);
+}
+
+#endif
diff --git a/CEP/DP3/AOFlagger/include/AOFlagger/test/util/utiltestgroup.h b/CEP/DP3/AOFlagger/include/AOFlagger/test/util/utiltestgroup.h
new file mode 100644
index 00000000000..3fad2e5debb
--- /dev/null
+++ b/CEP/DP3/AOFlagger/include/AOFlagger/test/util/utiltestgroup.h
@@ -0,0 +1,37 @@
+/***************************************************************************
+ * Copyright (C) 2008 by A.R. Offringa *
+ * offringa@astro.rug.nl *
+ * *
+ * This program is free software; you can redistribute it and/or modify *
+ * it under the terms of the GNU General Public License as published by *
+ * the Free Software Foundation; either version 2 of the License, or *
+ * (at your option) any later version. *
+ * *
+ * This program is distributed in the hope that it will be useful, *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
+ * GNU General Public License for more details. *
+ * *
+ * You should have received a copy of the GNU General Public License *
+ * along with this program; if not, write to the *
+ * Free Software Foundation, Inc., *
+ * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
+ ***************************************************************************/
+#ifndef AOFLAGGER_UTILTESTGROUP_H
+#define AOFLAGGER_UTILTESTGROUP_H
+
+#include <AOFlagger/test/testingtools/testgroup.h>
+
+#include <AOFlagger/test/util/numberparsertest.h>
+
+class UtilTestGroup : public TestGroup {
+ public:
+ UtilTestGroup() : TestGroup("Common utilities") { }
+
+ virtual void Initialize()
+ {
+ Add(new NumberParserTest());
+ }
+};
+
+#endif
diff --git a/CEP/DP3/AOFlagger/include/AOFlagger/util/numberparser.h b/CEP/DP3/AOFlagger/include/AOFlagger/util/numberparser.h
new file mode 100644
index 00000000000..af55efa0ed4
--- /dev/null
+++ b/CEP/DP3/AOFlagger/include/AOFlagger/util/numberparser.h
@@ -0,0 +1,188 @@
+/***************************************************************************
+ * Copyright (C) 2012 by A.R. Offringa *
+ * offringa@astro.rug.nl *
+ * *
+ * This program is free software; you can redistribute it and/or modify *
+ * it under the terms of the GNU General Public License as published by *
+ * the Free Software Foundation; either version 2 of the License, or *
+ * (at your option) any later version. *
+ * *
+ * This program is distributed in the hope that it will be useful, *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
+ * GNU General Public License for more details. *
+ * *
+ * You should have received a copy of the GNU General Public License *
+ * along with this program; if not, write to the *
+ * Free Software Foundation, Inc., *
+ * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
+ ***************************************************************************/
+
+#include <stdexcept>
+
+#include <cmath>
+
+class NumberParsingException : public std::runtime_error
+{
+ public:
+ explicit NumberParsingException(const std::string &str)
+ : std::runtime_error(std::string("Failed to parse number: ") + str)
+ {
+ }
+};
+
+/**
+ * This class implements functions that copy the behaviour of atof and strtod, etc.
+ * Unfortunately, atof are locale-dependent and e.g. a Dutch machine will interpret
+ * a decimal point differently. This is undesirable, hence the reimplementation here.
+ */
+class NumberParser
+{
+ public:
+ /**
+ * @throws NumberParsingException when @c str can not be parsed.
+ */
+ static double ToDouble(const char *str)
+ {
+ return toNumber<double>(str);
+ }
+
+ /**
+ * @throws NumberParsingException when @c str can not be parsed.
+ */
+ static short ToShort(const char *str)
+ {
+ return toSignedInteger<short>(str);
+ }
+ private:
+ /**
+ * @throws NumberParsingException when @c str can not be parsed.
+ */
+ template<typename T>
+ static T toNumber(const char *str)
+ {
+ if(*str == 0)
+ throw NumberParsingException("Supplied string is empty");
+
+ // Read optional sign
+ bool isNegative;
+ if(isNegativeSymbol(*str)) { isNegative = true; ++str; }
+ else if(isPositiveSymbol(*str)) { isNegative = false; ++str; }
+ else isNegative = false;
+
+ // Read everything before decimal point
+ if(!isDigit(*str))
+ throw NumberParsingException("Number did not start with digits after optional sign symbol");
+ T val = (T) ((*str) - '0');
+ ++str;
+
+ while(isDigit(*str))
+ {
+ val = val*10.0 + (T) ((*str) - '0');
+ ++str;
+ }
+
+ // Skip decimal point if given
+ if(isDecimalPoint(*str)) ++str;
+
+ // Read decimals
+ T currentValue = 0.1;
+ while(isDigit(*str))
+ {
+ val += currentValue * (T) ((*str) - '0');
+ currentValue /= 10.0;
+ ++str;
+ }
+
+ // Read optional exponent
+ if(isExponentSymbol(*str))
+ {
+ ++str;
+
+ try {
+ short e = ToShort(str);
+ val = val * intPow10(e);
+ } catch(NumberParsingException &e)
+ {
+ throw NumberParsingException("Could not parse exponent");
+ }
+ }
+ // If there was an exponent, ToShort has checked for garbage at the end and
+ // str is not updated, otherwise we still need to check that...
+ else if(*str != 0)
+ throw NumberParsingException("The number contains invalid characters after its digits");
+
+ if(isNegative)
+ return -val;
+ else
+ return val;
+ }
+
+ template<typename T>
+ static T toSignedInteger(const char *str)
+ {
+ if(*str == 0)
+ throw NumberParsingException("Supplied string is empty");
+
+ // Read optional sign
+ bool isNegative;
+ if(isNegativeSymbol(*str)) { isNegative = true; ++str; }
+ else if(isPositiveSymbol(*str)) { isNegative = false; ++str; }
+ else isNegative = false;
+
+ // Read digits
+ if(!isDigit(*str))
+ throw NumberParsingException("Integer did not start with digits after optional sign symbol");
+ T val = (T) ((*str) - '0');
+ ++str;
+
+ while(isDigit(*str))
+ {
+ val = val*10 + (T) ((*str) - '0');
+ ++str;
+ }
+
+ // Check if str is completely read.
+ if(*str == 0)
+ {
+ if(isNegative)
+ return -val;
+ else
+ return val;
+ }
+ else
+ throw NumberParsingException("The integer contains invalid characters after its digits");
+ }
+
+ static double intPow10(int par)
+ {
+ // TODO this can be done a lot faster and more accurate with knowledge that
+ // par = int.
+ return pow10((double) par);
+ }
+
+ static bool isDigit(char c)
+ {
+ return c >= '0' && c <= '9';
+ }
+
+ static bool isPositiveSymbol(char c)
+ {
+ return c == '+';
+ }
+
+ static bool isNegativeSymbol(char c)
+ {
+ return c == '-';
+ }
+
+ static bool isDecimalPoint(char c)
+ {
+ return c == '.';
+ }
+
+ static bool isExponentSymbol(char c)
+ {
+ return c == 'e' || c == 'E';
+ }
+};
diff --git a/CEP/DP3/AOFlagger/src/strategy/control/strategyreader.cpp b/CEP/DP3/AOFlagger/src/strategy/control/strategyreader.cpp
index ab2ce9d70a9..ca82c6f80f4 100644
--- a/CEP/DP3/AOFlagger/src/strategy/control/strategyreader.cpp
+++ b/CEP/DP3/AOFlagger/src/strategy/control/strategyreader.cpp
@@ -19,6 +19,8 @@
***************************************************************************/
#include <AOFlagger/strategy/control/strategyreader.h>
+#include <AOFlagger/util/numberparser.h>
+
#include <AOFlagger/strategy/actions/absthresholdaction.h>
#include <AOFlagger/strategy/actions/action.h>
#include <AOFlagger/strategy/actions/adapter.h>
@@ -100,19 +102,23 @@ Strategy *StrategyReader::CreateStrategyFromFile(const std::string &filename)
xmlChar *formatVersionCh = xmlGetProp(curNode, BAD_CAST "format-version");
if(formatVersionCh == 0)
throw StrategyReaderError("Missing attribute 'format-version'");
- double formatVersion = atof((const char*) formatVersionCh);
+ double formatVersion = NumberParser::ToDouble((const char*) formatVersionCh);
xmlFree(formatVersionCh);
xmlChar *readerVersionRequiredCh = xmlGetProp(curNode, BAD_CAST "reader-version-required");
if(readerVersionRequiredCh == 0)
throw StrategyReaderError("Missing attribute 'reader-version-required'");
- double readerVersionRequired = atof((const char*) readerVersionRequiredCh);
+ double readerVersionRequired = NumberParser::ToDouble((const char*) readerVersionRequiredCh);
xmlFree(readerVersionRequiredCh);
if(readerVersionRequired > STRATEGY_FILE_FORMAT_VERSION)
throw StrategyReaderError("This file requires a newer software version");
if(formatVersion < STRATEGY_FILE_FORMAT_VERSION_REQUIRED)
- throw StrategyReaderError("This file is too old for the software, please recreate the strategy");
+ {
+ std::stringstream s;
+ s << "This file is too old for the software, please recreate the strategy. File format version: " << formatVersion << ", oldest version that this software understands: " << STRATEGY_FILE_FORMAT_VERSION_REQUIRED << " (these versions are numbered differently from the software).";
+ throw StrategyReaderError(s.str());
+ }
strategy = parseRootChildren(curNode);
}
@@ -213,7 +219,7 @@ int StrategyReader::getInt(xmlNode *node, const char *name) const
double StrategyReader::getDouble(xmlNode *node, const char *name) const
{
xmlNode *valNode = getTextNode(node, name);
- return atof((const char *) valNode->content);
+ return NumberParser::ToDouble((const char *) valNode->content);
}
std::string StrategyReader::getString(xmlNode *node, const char *name) const
diff --git a/CEP/DP3/AOFlagger/test/aotest.cpp b/CEP/DP3/AOFlagger/test/aotest.cpp
index bfae02d7ded..91b92eef391 100644
--- a/CEP/DP3/AOFlagger/test/aotest.cpp
+++ b/CEP/DP3/AOFlagger/test/aotest.cpp
@@ -4,6 +4,7 @@
#include <AOFlagger/test/experiments/experimentstestgroup.h>
#include <AOFlagger/test/msio/msiotestgroup.h>
#include <AOFlagger/test/quality/qualitytestgroup.h>
+#include <AOFlagger/test/util/utiltestgroup.h>
int main(int argc, char *argv[])
{
@@ -24,6 +25,11 @@ int main(int argc, char *argv[])
qualityGroup.Run();
successes += qualityGroup.Successes();
failures += qualityGroup.Failures();
+
+ UtilTestGroup utilGroup;
+ utilGroup.Run();
+ successes += utilGroup.Successes();
+ failures += utilGroup.Failures();
}
if(argc > 1 && (std::string(argv[1])=="all" || std::string(argv[1])=="only"))
--
GitLab