Task #1892: fixing bug causing strategy reader to malfunction with different locales

.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

CEP/DP3/AOFlagger/include/AOFlagger/strategy/control/strategyreader.h

 /***************************************************************************
- *   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  *

CEP/DP3/AOFlagger/include/AOFlagger/test/util/numberparsertest.h

+/***************************************************************************
+ *   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

CEP/DP3/AOFlagger/include/AOFlagger/test/util/utiltestgroup.h

+/***************************************************************************
+ *   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

CEP/DP3/AOFlagger/include/AOFlagger/util/numberparser.h

+/***************************************************************************
+ *   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';
+		}
+};

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 

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"))