APTLoader: move handling of Viewpoint lines to a separate method

- APTLoader::parseViewpointLine(): new method.
- APTLoader::parseViewpointLine(): check vector size to prevent
                                   past-bounds reading.

More accurate signature for fptypeFromRobinType()

Row codes in the apt.dat spec are always non-negative -> take an
unsigned int instead of just an int.

Use an initialization list for remaining members of APTLoader::APTLoader()

This is slightly more efficient in general. Swap the order of
declarations for the 'cache' and 'currentAirportID' members of
APTLoader to have the initialization list in the same order as the
member declarations (cf. g++'s -Wreorder option and
<http://stackoverflow.com/questions/1828037/whats-the-point-of-g-wreorder>).

Fix detection of blank lines by APTLoader::isBlankOrCommentLine()

In
<https://sourceforge.net/p/flightgear/flightgear/merge-requests/39/#cea6>,
it was decided to let the main apt.dat line reading loop give out
"lines" that may end with '\r', because most of the downstream code will
automatically get rid of this character thanks to its use of
simgear::strutils::split(). However, APTLoader::isBlankOrCommentLine()
didn't detect blank lines properly due to that trailing '\r', which
could cause bad behavior because the subsequent atoi() call could return
anything from a string containing only whitespace (the "anything" in
question being then interpreted as an apt.dat row code...).

Add method APTLoader::cleanLine()

This method returns a copy of the input line with trailing '\r' char(s)
removed.

APTLoader::loadAirports(): clean message when finding an unknown row code

The start of the log message could previously be overwritten by later
text because of the '\r' at the end of input lines (now obtained from
APTLoader::readAptDatFile()). Quite confusing! Use the new
APTLoader::cleanLine() to prevent this from happening.

src/Airports/apt_loader.cxx

@@ -59,7 +59,8 @@
 using std::vector;
 using std::string;
 
-static FGPositioned::Type fptypeFromRobinType(int aType)
+
+static FGPositioned::Type fptypeFromRobinType(unsigned int aType)
 {
   switch (aType) {
   case 1: return FGPositioned::AIRPORT;
@@ -78,11 +79,10 @@ namespace flightgear
 {
 APTLoader::APTLoader()
   :  last_apt_id(""),
-     last_apt_elev(0.0)
-{
-  currentAirportID = 0;
-  cache = NavDataCache::instance();
-}
+     last_apt_elev(0.0),
+     currentAirportID(0),
+     cache(NavDataCache::instance())
+{ }
 
 APTLoader::~APTLoader() { }
 
@@ -242,15 +242,9 @@ void APTLoader::loadAirports()
         parseHelipadLine850(simgear::strutils::split(linesIt->str));
       } else if ( line_id == 18 ) {
         // beacon entry (ignore)
-      } else if ( line_id == 14 ) {
-        // control tower entry
-        vector<string> token(simgear::strutils::split(linesIt->str));
-
-        double lat = atof( token[1].c_str() );
-        double lon = atof( token[2].c_str() );
-        double elev = atof( token[3].c_str() );
-        tower = SGGeod::fromDegFt(lon, lat, elev + last_apt_elev);
-        cache->insertTower(currentAirportID, tower);
+      } else if ( line_id == 14 ) {  // Viewpoint/control tower
+        parseViewpointLine(aptDat, linesIt->number,
+                           simgear::strutils::split(linesIt->str));
       } else if ( line_id == 19 ) {
         // windsock entry (ignore)
       } else if ( line_id == 20 ) {
@@ -280,11 +274,11 @@ void APTLoader::loadAirports()
         // airport traffic flow (ignore)
       } else {
         std::ostringstream oss;
+        string cleanedLine = cleanLine(linesIt->str);
         oss << aptDat << ":" << linesIt->number << ": unknown row code " <<
           line_id;
-        SG_LOG( SG_GENERAL, SG_ALERT,
-                oss.str() << " (" << linesIt->str << ")" );
-        throw sg_format_exception(oss.str(), linesIt->str);
+        SG_LOG( SG_GENERAL, SG_ALERT, oss.str() << " (" << cleanedLine << ")" );
+        throw sg_format_exception(oss.str(), cleanedLine);
       }
     } // of loop over the second and subsequent apt.dat lines for the airport
 
@@ -296,7 +290,24 @@ void APTLoader::loadAirports()
 bool APTLoader::isBlankOrCommentLine(const std::string& line)
 {
   size_t pos = line.find_first_not_of(" \t");
-  return ( pos == std::string::npos || line.find("##", pos) == pos );
+  return ( pos == std::string::npos ||
+           line[pos] == '\r' ||
+           line.find("##", pos) == pos );
+}
+
+std::string APTLoader::cleanLine(const std::string& line)
+{
+  std::string res = line;
+
+  // Lines obtained from readAptDatFile() may end with \r, which can be quite
+  // confusing when printed to the terminal.
+  for (std::string::reverse_iterator it = res.rbegin();
+       it != res.rend() && *it == '\r'; /* empty */)
+  { // The beauty of C++ iterators...
+    it = std::string::reverse_iterator(res.erase( (it+1).base() ));
+  }
+
+  return res;
 }
 
 void APTLoader::throwExceptionIfStreamError(const sg_gzifstream& input_stream,
@@ -540,6 +551,22 @@ void APTLoader::parseHelipadLine850(const vector<string>& token)
                       width, 0.0, 0.0, surface_code);
 }
 
+void APTLoader::parseViewpointLine(const string& aptDat, unsigned int lineNum,
+                                   const vector<string>& token)
+{
+  if (token.size() < 5) {
+    SG_LOG( SG_GENERAL, SG_WARN,
+            aptDat << ":" << lineNum << ": invalid viewpoint line "
+            "(row code 14): at least 5 fields are required" );
+  } else {
+    double lat = atof(token[1].c_str());
+    double lon = atof(token[2].c_str());
+    double elev = atof(token[3].c_str());
+    tower = SGGeod::fromDegFt(lon, lat, elev + last_apt_elev);
+    cache->insertTower(currentAirportID, tower);
+  }
+}
+
 void APTLoader::parsePavementLine850(const vector<string>& token)
 {
   if ( token.size() >= 5 ) {

src/Airports/apt_loader.hxx

@@ -101,6 +101,8 @@ private:
 
   // Tell whether an apt.dat line is blank or a comment line
   bool isBlankOrCommentLine(const std::string& line);
+  // Return a copy of 'line' with trailing '\r' char(s) removed
+  std::string cleanLine(const std::string& line);
   void throwExceptionIfStreamError(const sg_gzifstream& input_stream,
                                    const SGPath& path);
   void parseAirportLine(unsigned int rowCode,
@@ -110,6 +112,8 @@ private:
   void parseRunwayLine850(const std::vector<std::string>& token);
   void parseWaterRunwayLine850(const std::vector<std::string>& token);
   void parseHelipadLine850(const std::vector<std::string>& token);
+  void parseViewpointLine(const std::string& aptDat, unsigned int lineNum,
+                          const std::vector<std::string>& token);
   void parsePavementLine850(const std::vector<std::string>& token);
   void parsePavementNodeLine850(int num, const std::vector<std::string>& token);
 
@@ -130,9 +134,9 @@ private:
   bool pavement;
   std::vector<FGPavementPtr> pavements;
 
-  NavDataCache* cache;
   // Not an airport identifier in the sense of the apt.dat spec!
   PositionedID currentAirportID;
+  NavDataCache* cache;
 };
 
 bool metarDataLoad(const SGPath& path);