New version : 2018.3.4

The following changes fixes a case for me where I hear the sound change levels up and down for each pause un-pause cycle.
Patch provided by daniel.c.wickstrom@gmail.com.

CMakeLists.txt

@@ -32,6 +32,7 @@ include (GenerateExportHeader)
 
 # only relevant for building shared libs but let's set it regardless
 set(CMAKE_OSX_RPATH 1)
+set(CMAKE_OSX_DEPLOYMENT_TARGET "10.9" CACHE STRING "Minimum OS X deployment version")
 
 # let's use & require C++11 - note these are only functional with CMake 3.1
 # we do manual fallbacks for CMake 3.0 in the compilers section
@@ -44,12 +45,7 @@ string(STRIP ${versionFile} SIMGEAR_VERSION)
 
 project(SimGear VERSION ${SIMGEAR_VERSION} LANGUAGES C CXX)
 
-# using 10.7 because boost requires libc++ and 10.6 doesn't include it
-# Cmake documentation says we must set this before calling project(), but
-# it only seems to be picked up setting it /after/ the call to project()
-set(CMAKE_OSX_DEPLOYMENT_TARGET "10.7")
-
-# add a dependency on the versino file
+# add a dependency on the version file
 set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS version)
 
 set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS TRUE)
@@ -126,12 +122,17 @@ option(ENABLE_RTI       "Set to ON to build SimGear with RTI support" OFF)
 option(ENABLE_GDAL      "Set to ON to build SimGear with GDAL support" OFF)
 option(ENABLE_TESTS     "Set to OFF to disable building SimGear's test applications" ON)
 option(ENABLE_SOUND     "Set to OFF to disable building SimGear's sound support" ON)
-option(USE_AEONWAVE     "Set to ON to use AeonWave instead of OpenAL" OFF)
+option(USE_AEONWAVE     "Set to ON to use AeonWave instead of OpenAL" ON)
 option(ENABLE_PKGUTIL   "Set to ON to build the sg_pkgutil application (default)" ON)
 option(ENABLE_DNS       "Set to ON to use udns library and DNS service resolver" ON)
-option(ENABLE_SIMD      "Enable SSE/SSE2 support for x86 compilers" ON)
+option(ENABLE_SIMD      "Enable SSE/SSE2 support for compilers" ON)
+option(ENABLE_SIMD_CODE	"Enable SSE/SSE2 support code for compilers" OFF)
 option(ENABLE_OPENMP    "Enable OpenMP compiler support" OFF)
 
+if (NOT ENABLE_SIMD AND ENABLE_SIMD_CODE)
+  set(ENABLE_SIMD_CODE OFF)
+endif()
+
 include (DetectArch)
 
 # until the fstream fix is applied and generally available in OSG,
@@ -167,14 +168,22 @@ endif (MSVC)
 if (MSVC AND MSVC_3RDPARTY_ROOT)
   message(STATUS "3rdparty files located in ${MSVC_3RDPARTY_ROOT}")
 
+  string(SUBSTRING ${MSVC_VERSION} 0 2 MSVC_VERSION_MAJOR)
+  string(SUBSTRING ${MSVC_VERSION} 2 2 MSVC_VERSION_MINOR)
+
   set( OSG_MSVC "msvc" )
-  if (${MSVC_VERSION} EQUAL 1900)
+  if (${MSVC_VERSION_MAJOR} EQUAL "19")
+    if (${MSVC_VERSION_MINOR} EQUAL "00")
       set( OSG_MSVC ${OSG_MSVC}140 )
-  elseif (${MSVC_VERSION} EQUAL 1800)
-      set( OSG_MSVC ${OSG_MSVC}120 )
+    else ()
+      set( OSG_MSVC ${OSG_MSVC}141 )
+    endif ()
+  elseif (${MSVC_VERSION_MAJOR} EQUAL "18")
+    set( OSG_MSVC ${OSG_MSVC}120 )
   else ()
-      message(FATAL_ERROR "Visual Studio 2013/2015 is required now")
+    message(FATAL_ERROR "Visual Studio 2013/15/17 is required")
   endif ()
+
   if (CMAKE_CL_64)
       set( OSG_MSVC ${OSG_MSVC}-64 )
       set( MSVC_3RDPARTY_DIR 3rdParty.x64 )
@@ -194,11 +203,8 @@ if (MSVC AND MSVC_3RDPARTY_ROOT)
     message(STATUS "BOOST_INCLUDEDIR is ${BOOST_INCLUDEDIR}")
   endif()
 
-  if (NOT USE_AEONWAVE)
-    set (OPENAL_INCLUDE_DIR ${MSVC_3RDPARTY_ROOT}/${MSVC_3RDPARTY_DIR}/include)
-    set (OPENAL_LIBRARY_DIR ${MSVC_3RDPARTY_ROOT}/${MSVC_3RDPARTY_DIR}/lib)
-    message(STATUS "OPENAL_INCLUDE_DIR is ${OPENAL_INCLUDE_DIR}")
-  endif()
+  set (OPENAL_INCLUDE_DIR ${MSVC_3RDPARTY_ROOT}/${MSVC_3RDPARTY_DIR}/include)
+  set (OPENAL_LIBRARY_DIR ${MSVC_3RDPARTY_ROOT}/${MSVC_3RDPARTY_DIR}/lib)
 endif (MSVC AND MSVC_3RDPARTY_ROOT)
 
 if(APPLE)
@@ -222,12 +228,19 @@ else()
 
     if (ENABLE_SOUND)
         if (USE_AEONWAVE)
-            find_package(AAX COMPONENTS aax REQUIRED)
-        else()
+            find_package(AAX)
+        endif()
+
+        if(NOT AAX_FOUND)
+            set(USE_AEONWAVE FALSE)
             find_package(OpenAL REQUIRED)
         endif()
 
-        message(STATUS "Sound support: ENABLED")
+        if(AAX_FOUND)
+            message(STATUS "Sound support: AeonWave")
+        else()
+            message(STATUS "Sound support: OpenAL")
+        endif()
     endif(ENABLE_SOUND)
 
     find_package(OpenSceneGraph 3.2.0 REQUIRED osgText osgSim osgDB osgParticle osgGA osgViewer osgUtil)
@@ -397,10 +410,15 @@ if(CMAKE_COMPILER_IS_GNUCXX)
       message(WARNING "GCC 4.4 will be required soon, please upgrade")
     endif()
 
-    if(ENABLE_SIMD)
+    if (CMAKE_BUILD_TYPE STREQUAL "Debug")
+      set(CMAKE_C_FLAGS
+        "${CMAKE_C_FLAGS} -O0 -fno-omit-frame-pointer -fno-inline")
+      set(CMAKE_CXX_FLAGS
+        "${CMAKE_CXX_FLAGS} -O0 -fno-omit-frame-pointer -fno-inline")
+    elseif (ENABLE_SIMD)
       if (X86 OR X86_64)
-        set(CMAKE_C_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse")
-        set(CMAKE_CXX_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse")
+        set(CMAKE_C_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse -ftree-vectorize -ftree-slp-vectorize")
+        set(CMAKE_CXX_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse -ftree-vectorize -ftree-slp-vectorize")
       endif()
     endif()
 
@@ -420,10 +438,15 @@ if (CLANG)
     # fix Boost compilation :(
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++")
 
-    if(ENABLE_SIMD)
+    if (CMAKE_BUILD_TYPE STREQUAL "Debug")
+      set(CMAKE_C_FLAGS
+        "${CMAKE_C_FLAGS} -O0 -fno-omit-frame-pointer -fno-inline-functions")
+      set(CMAKE_CXX_FLAGS
+        "${CMAKE_CXX_FLAGS} -O0 -fno-omit-frame-pointer -fno-inline-functions")
+    elseif (ENABLE_SIMD)
       if (X86 OR X86_64)
-        set(CMAKE_C_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse")
-        set(CMAKE_CXX_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse")
+        set(CMAKE_C_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse -ftree-vectorize -ftree-slp-vectorize")
+        set(CMAKE_CXX_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse -ftree-vectorize -ftree-slp-vectorize")
       endif()
     endif()
 endif()

CMakeModules/FindAAX.cmake

@@ -1,61 +1,74 @@
-# Locate AAX
+# Try to find AAX (AeonWave)
 # This module defines
-# AAX_LIBRARIES
-# AAX_FOUND, if false, do not try to link to AAX 
-# AAX_INCLUDE_DIR, where to find the headers
+#
+# AAX_FOUND - if false, do not try to link to AAX
+# AAX_INCLUDE_DIR - where to find the headers
+# AAX_LIBRARIES - Link these to use AAX
+#
+# Copyright (C) 2016-2018 by Erik Hofman.
+# Copyright (C) 2016-2018 by Adalin B.V.
 #
 # $AAXDIR is an environment variable that would
 # correspond to the ./configure --prefix=$AAXDIR
 # used in building AAX.
 #
-# Created by Erik Hofman.
+# This file is Public Domain (www.unlicense.org)
+# This is free and unencumbered software released into the public domain.
 
-FIND_PATH(AAX_INCLUDE_DIR aax/aax.h
-  HINTS
-  $ENV{AAXDIR}
-  $ENV{ProgramFiles}/aax
-  $ENV{ProgramFiles}/AeonWave
-  $ENV{ProgramFiles}/Adalin/AeonWave
-  ${CMAKE_SOURCE_DIR}/aax
-  PATH_SUFFIXES include
-  PATHS
-  ~/Library/Frameworks
-  /Library/Frameworks
-  /usr/local
-  /usr
-  /opt
-)
+if (AAX_LIBRARY AND AAX_INCLUDE_DIR)
+  # in cache already
+  set(AAX_FOUND TRUE)
+else()
+  find_path(AAX_INCLUDE_DIR aax/aax.h
+    HINTS
+    $ENV{AAXDIR}
+    $ENV{ProgramFiles}/aax
+    $ENV{ProgramFiles}/AeonWave
+    $ENV{ProgramFiles}/Adalin/AeonWave
+    ${CMAKE_SOURCE_DIR}/aax
+    PATH_SUFFIXES include
+    PATHS
+    ~/Library/Frameworks
+    /Library/Frameworks
+    /usr/local
+    /usr
+    /opt
+  )
 
-FIND_LIBRARY(AAX_LIBRARY 
-  NAMES AAX aax AAX32
-  HINTS
-  $ENV{AAXDIR}
-  $ENV{ProgramFiles}/AAX
-  $ENV{ProgramFiles}/AeonWave
-  $ENV{ProgramFiles}/Adalin/AeonWave
-  ${CMAKE_BUILD_DIR}/aax
-  PATH_SUFFIXES bin lib lib/${CMAKE_LIBRARY_ARCHITECTURE} lib64 libs64 libs libs/Win32 libs/Win64
-  PATHS
-  ~/Library/Frameworks
-  /Library/Frameworks
-  /usr/local
-  /usr
-  /opt
-)
+  find_library(AAX_LIBRARY 
+    NAMES AAX aax libAAX
+    HINTS
+    $ENV{AAXDIR}
+    $ENV{ProgramFiles}/AAX
+    $ENV{ProgramFiles}/AeonWave
+    $ENV{ProgramFiles}/Adalin/AeonWave
+    ${CMAKE_BUILD_DIR}/aax
+    PATH_SUFFIXES lib64 lib lib/${CMAKE_LIBRARY_ARCHITECTURE} libs64 libs libs/Win32 libs/Win64 bin
+    PATHS
+    ~/Library/Frameworks
+    /Library/Frameworks
+    /usr/local
+    /usr
+    /opt
+  )
 
-IF(AAX_LIBRARY AND AAX_INCLUDE_DIR)
-  SET(AAX_FOUND "YES")
-ELSE(AAX_LIBRARY AND AAX_INCLUDE_DIR)
-  IF(NOT AAX_INCLUDE_DIR)
-    MESSAGE(FATAL_ERROR "Unable to find the AAX library development files.")
-    SET(AAX_FOUND "NO")
-  ENDIF(NOT AAX_INCLUDE_DIR)
-  IF(NOT AAX_LIBRARY)
-    IF(SINGLE_PACKAGE)
-      SET(AAX_LIBRARY "${aax_BUILD_DIR}/aax/AAX32.dll")
-      SET(AAX_FOUND "YES")
-    ELSE(SINGLE_PACKAGE)
-    ENDIF(SINGLE_PACKAGE)
-  ENDIF(NOT AAX_LIBRARY)
-ENDIF(AAX_LIBRARY AND AAX_INCLUDE_DIR)
+  set(AAX_DEFINITIONS "")
+  if (AAX_LIBRARY AND AAX_INCLUDE_DIR)
+    set(AAX_FOUND TRUE)
+  endif()
+
+  if (AAX_FOUND)
+    if (NOT Udns_FIND_QUIETLY)
+      message(STATUS "Found AeonWave: ${AAX_LIBRARIES}")
+    endif ()
+  else ()
+    if (Udns_FIND_REQUIRED)
+      message(FATAL_ERROR "Could not find AeonWave")
+    endif ()
+  endif ()
+
+  # show the AAX_INCLUDE_DIRS and AAX_LIBRARIES variables only in the advanced view
+  mark_as_advanced(AAX_INCLUDE_DIRS AAX_LIBRARIES)
+
+endif()
 

CMakeModules/SimGearComponent.cmake

@@ -12,6 +12,11 @@ macro(simgear_component_common name includePath sourcesList sources headers)
 		set_property(GLOBAL
 			APPEND PROPERTY PUBLIC_HEADERS "${CMAKE_CURRENT_SOURCE_DIR}/${h}")
         set(fh${sourcesList} "${fh${sourcesList}}#${CMAKE_CURRENT_SOURCE_DIR}/${h}")
+
+        # also append headers to the sources list, so that IDEs find the files
+        # correctly (otherwise they are not in the project)
+        set_property(GLOBAL
+            APPEND PROPERTY ${sourcesList} "${CMAKE_CURRENT_SOURCE_DIR}/${h}")
 	endforeach()
 
     set_property(GLOBAL APPEND PROPERTY FG_GROUPS_${sourcesList}_C "${fc${sourcesList}}@")

README.OpenAL

@@ -1,20 +0,0 @@
-[This file is mirrored in both the FlightGear and SimGear packages.]
-
-You *must* have the development components of OpenAL installed on your system
-to build FlightGear!"  You can get a copy here:
-
-      http://connect.creativelabs.com/openal/default.aspx
-
-Build notes:
-
-You can download a versioned release of the openal library from 
-http://www.openal.org/downloads.html. Download the openal source,
-release 0.0.8 (dated February 11, 2006) and run:  
-tar xjvf openal-soft-1.5.304.tar.bz2
-cd openal-soft-1.5.304/
-ccmake .
-
-[ While running ccmake: press 'c' to configure, press 'c' once more, and
-  then press 'g' to generate and exit ]
-
-

README.sound

@@ -0,0 +1,39 @@
+[This file is mirrored in both the FlightGear and SimGear packages.]
+
+For Sound support FlightGear requires one of the two following packages:
+- OpenAL
+- AeonWave
+
+== OpenAL ===
+
+You *must* have the development components of OpenAL installed on your system
+to build FlightGear!"  You can get a copy here:
+
+      http://connect.creativelabs.com/openal/default.aspx
+
+Build notes:
+
+You can download a versioned release of the openal library from
+http://www.openal.org/downloads.html. Download the openal source,
+release 0.0.8 (dated February 11, 2006) and run:
+tar xjvf openal-soft-1.5.304.tar.bz2
+cd openal-soft-1.5.304/
+ccmake .
+
+[ While running ccmake: press 'c' to configure, press 'c' once more, and
+  then press 'g' to generate and exit ]
+
+
+== AeonWave ===
+
+For FlightGear AeonWave has a number of advantages over OpenAL:
+* Correct Doppler effect behavior
+* Default distance attenuation frequency filtering
+* Native support for 29 types of audio formats.
+* Native support for wav, mp3, vorbis and raw file formats.
+
+The source code of AeonWave can be found on GitHub:
+https://github.com/adalinbv
+
+Optimized binary packages are available at:
+http://www.adalin.com/

simgear/canvas/Canvas.cxx

@@ -1,4 +1,5 @@
-// The canvas for rendering with the 2d API
+///@file
+/// The canvas for rendering with the 2d API
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -38,6 +39,75 @@ namespace simgear
 {
 namespace canvas
 {
+  static int globalinstanceid = 1;
+  /**
+   * Camera Callback for moving completed canvas images to subscribed listener.
+   */
+  class CanvasImageCallback : public osg::Camera::DrawCallback {
+    public:
+    osg::Image *_rawImage;
+
+    CanvasImageCallback(osg::Image *rawImage)
+    : _min_delta_tick(1.0 / 8.0) {
+      _previousFrameTick = osg::Timer::instance()->tick();
+      _rawImage = rawImage;
+      SG_LOG(SG_GENERAL,SG_INFO,"CanvasImageCallback created. instance is " << instanceid);
+    }
+
+    virtual void operator()(osg::RenderInfo& renderInfo) const {
+      osg::Timer_t n = osg::Timer::instance()->tick();
+      double dt = osg::Timer::instance()->delta_s(_previousFrameTick, n);
+      if (dt < _min_delta_tick)
+        return;
+      _previousFrameTick = n;
+      SG_LOG(SG_GENERAL,SG_DEBUG,"CanvasImageCallback " << instanceid << ": image available for " << _subscribers.size() << " subscribers. camera is " << renderInfo.getCurrentCamera());
+
+      bool hasSubscribers = false;
+      {
+        OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_lock);
+        hasSubscribers = !_subscribers.empty();
+      }
+      if (hasSubscribers) {
+        //Make sure image can be overwritten by next frame while it is still returned to the client
+        osg::Image* image = new osg::Image(*_rawImage, osg::CopyOp::DEEP_COPY_ALL);
+        {
+          OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_lock);
+          while (!_subscribers.empty()) {
+            try {
+              CanvasImageReadyListener *subs = _subscribers.back();
+              if (subs){
+                subs->imageReady(image);
+              }else{
+                SG_LOG(SG_GENERAL,SG_WARN,"CanvasImageCallback subscriber null");
+              }
+            } catch (...) { }
+            _subscribers.pop_back();
+          }
+        }
+      }
+    }
+
+    void subscribe(CanvasImageReadyListener * subscriber) {
+      OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_lock);
+      _subscribers.push_back(subscriber);
+    }
+
+    void unsubscribe(CanvasImageReadyListener * subscriber) {
+      OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_lock);
+      _subscribers.remove(subscriber);
+    }
+
+    int getSubscriberCount() {
+      return _subscribers.size();
+    }
+
+    private:
+      mutable list<CanvasImageReadyListener*> _subscribers;
+      mutable OpenThreads::Mutex _lock;
+      mutable double _previousFrameTick;
+      double _min_delta_tick;
+      int instanceid = globalinstanceid++;
+  };
 
   //----------------------------------------------------------------------------
   Canvas::CullCallback::CullCallback(const CanvasWeakPtr& canvas):
@@ -247,6 +317,21 @@ namespace canvas
 
       osg::Camera* camera = _texture.getCamera();
 
+      string canvasname = _node->getStringValue("name");
+      int renderToImage = _node->getBoolValue("render-to-image");
+
+      if (renderToImage){
+        CanvasImageCallback *_screenshotCallback = dynamic_cast<CanvasImageCallback*> (camera->getFinalDrawCallback());
+        if (!_screenshotCallback) {
+          // no draw callback yet
+          osg::Image* shot = new osg::Image();
+          shot->allocateImage(getSizeX(), getSizeY(), 24, GL_RGB, GL_UNSIGNED_BYTE);
+          camera->attach(osg::Camera::COLOR_BUFFER, shot);
+          camera->setFinalDrawCallback(new CanvasImageCallback(shot));
+          SG_LOG(SG_GENERAL,SG_INFO,"CanvasImage: attached image and draw callback to camera " << camera << " for canvas " << canvasname << ". Ready for subscriber now.");
+        }
+      }
+
       // TODO Allow custom render order? For now just keep in order with
       //      property tree.
       camera->setRenderOrder(osg::Camera::PRE_RENDER, _node->getIndex());
@@ -346,6 +431,41 @@ namespace canvas
     }
   }
 
+  int Canvas::subscribe(CanvasImageReadyListener * subscriber) {
+    osg::Camera* camera = _texture.getCamera();
+    const string canvasname = _node->getStringValue("name");
+
+    SG_LOG(SG_GENERAL,SG_DEBUG,"CanvasImage: subscribe to canvas " << canvasname.c_str() << ", camera ="<< camera);
+
+    if (!_node->getBoolValue("render-to-image")) {
+      SG_LOG(SG_GENERAL,SG_INFO,"CanvasImage: Setting render-to-image");
+      _node->addChild("render-to-image", 0)->setBoolValue(1);
+      setStatusFlags(STATUS_DIRTY, true);
+    }
+
+    CanvasImageCallback *_screenshotCallback = dynamic_cast<CanvasImageCallback*> (camera->getFinalDrawCallback());
+    if (_screenshotCallback) {
+      // Camera ready for subscriber. Otherwise, draw callback is created by canvas thread later.
+      SG_LOG(SG_GENERAL,SG_DEBUG,"CanvasImage: adding subscriber to camera draw callback");
+      _screenshotCallback->subscribe(subscriber);
+      // TODO: check: Is this the correct way to ensure the canvas will be available?
+      enableRendering(true);
+      return 1;
+    }
+    return 0;
+  }
+
+  int Canvas::unsubscribe(CanvasImageReadyListener * subscriber) {
+    osg::Camera* camera = _texture.getCamera();
+    SG_LOG(SG_GENERAL,SG_DEBUG,"CanvasImage: unsubscribe");
+    CanvasImageCallback *cb = dynamic_cast<CanvasImageCallback*> (camera->getFinalDrawCallback());
+    if (cb) {
+      SG_LOG(SG_GENERAL,SG_DEBUG,"CanvasImage: unsubscribe from camera " << camera);
+      cb->unsubscribe(subscriber);
+    }
+    return 0;
+  }
+
   //----------------------------------------------------------------------------
   bool Canvas::addEventListener( const std::string& type,
                                  const EventListener& cb )

simgear/canvas/Canvas.hxx

@@ -44,6 +44,17 @@ namespace canvas
   class CanvasMgr;
   class MouseEvent;
 
+  /**
+   * A listener interested in completed canvas drawing.
+   */
+  class CanvasImageReadyListener {
+  public:
+    virtual void imageReady(osg::ref_ptr<osg::Image>) = 0;
+    virtual ~CanvasImageReadyListener()
+    {
+    }
+  };
+
   /**
    * Canvas to draw onto (to an off-screen render target).
    */
@@ -71,18 +82,18 @@ namespace canvas
         public osg::NodeCallback
       {
         public:
-          CullCallback(const CanvasWeakPtr& canvas);
+          explicit CullCallback(const CanvasWeakPtr& canvas);
 
         private:
           CanvasWeakPtr _canvas;
 
-          virtual void operator()(osg::Node* node, osg::NodeVisitor* nv);
+          void operator()(osg::Node* node, osg::NodeVisitor* nv) override;
       };
       typedef osg::ref_ptr<CullCallback> CullCallbackPtr;
 
-      Canvas(SGPropertyNode* node);
+      explicit Canvas(SGPropertyNode* node);
       virtual ~Canvas();
-      virtual void onDestroy();
+      void onDestroy() override;
 
       void setCanvasMgr(CanvasMgr* canvas_mgr);
       CanvasMgr* getCanvasMgr() const;
@@ -160,7 +171,12 @@ namespace canvas
        */
       void enableRendering(bool force = false);
 
-      void update(double delta_time_sec);
+      void update(double delta_time_sec) override;
+
+      osg::Camera* getCamera();
+      int subscribe(CanvasImageReadyListener * subscriber);
+      int unsubscribe(CanvasImageReadyListener * subscriber);
+      int getSubscriberCount();
 
       bool addEventListener(const std::string& type, const EventListener& cb);
       bool dispatchEvent(const EventPtr& event);
@@ -184,11 +200,9 @@ namespace canvas
       bool propagateEvent( EventPtr const& event,
                            EventPropagationPath const& path );
 
-      virtual void childAdded( SGPropertyNode * parent,
-                               SGPropertyNode * child );
-      virtual void childRemoved( SGPropertyNode * parent,
-                                 SGPropertyNode * child );
-      virtual void valueChanged (SGPropertyNode * node);
+      void childAdded(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void childRemoved(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void valueChanged(SGPropertyNode * node) override;
 
       osg::Texture2D* getTexture() const;
 
@@ -254,8 +268,8 @@ namespace canvas
 
       static SystemAdapterPtr _system_adapter;
 
-      Canvas(const Canvas&); // = delete;
-      Canvas& operator=(const Canvas&); // = delete;
+      Canvas(const Canvas&) = delete;
+      Canvas& operator=(const Canvas&) = delete;
   };
 
 } // namespace canvas

simgear/canvas/CanvasEvent.cxx

@@ -1,4 +1,5 @@
-// Canvas Event for event model similar to DOM Level 3 Event Model
+///@file
+/// Canvas Event for event model similar to DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -125,10 +126,10 @@ namespace canvas
   //----------------------------------------------------------------------------
   std::string Event::typeToStr(int type)
   {
-    TypeMap const& type_map = getTypeMap();
+    auto const& map_by_id = getTypeMap().by<id>();
 
-    TypeMap::map_by<id>::const_iterator it = type_map.by<id>().find(type);
-    if( it == type_map.by<id>().end() )
+    auto it = map_by_id.find(type);
+    if( it == map_by_id.end() )
       return "unknown";
     return it->second;
   }

simgear/canvas/CanvasEvent.hxx

@@ -65,6 +65,11 @@ namespace canvas
       // of the actual event instances.
       virtual ~Event();
 
+      /**
+       * Clone event and set to the given type (Same type if not specified)
+       */
+      virtual Event* clone(int type = 0) const = 0;
+
       /**
        * Get whether this events support bubbling
        */
@@ -110,7 +115,14 @@ namespace canvas
        */
       bool defaultPrevented() const;
 
+      /**
+       * Register a new type string or get the id of an existing type string
+       *
+       * @param type    Type string
+       * @return Id of the given @a type
+       */
       static int getOrRegisterType(const std::string& type);
+
       static int strToType(const std::string& type);
       static std::string typeToStr(int type);
 

simgear/canvas/CanvasEventManager.cxx

@@ -1,4 +1,5 @@
-// Manage event handling inside a Canvas similar to the DOM Level 3 Event Model
+///@file
+/// Manage event handling inside a Canvas similar to the DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -17,9 +18,11 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
 
 #include <simgear_config.h>
+
 #include "CanvasEventManager.hxx"
-#include <simgear/canvas/events/MouseEvent.hxx>
-#include <simgear/canvas/elements/CanvasElement.hxx>
+#include "elements/CanvasElement.hxx"
+#include "events/MouseEvent.hxx"
+
 #include <cmath>
 
 namespace simgear
@@ -114,6 +117,8 @@ namespace canvas
         return handled;
       }
       case Event::DRAG:
+      case Event::DRAG_START:
+      case Event::DRAG_END:
         if( !_last_mouse_down.valid() )
           return false;
         else

simgear/canvas/CanvasEventManager.hxx

@@ -1,4 +1,5 @@
-// Manage event handling inside a Canvas similar to the DOM Level 3 Event Model
+///@file
+/// Manage event handling inside a Canvas similar to the DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/CanvasEventTypes.hxx

@@ -1,4 +1,5 @@
-// Mapping between canvas gui Event types and their names
+///@file
+/// Mapping between canvas gui Event types and their names
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -25,6 +26,8 @@ ENUM_MAPPING(MOUSE_UP,    "mouseup",    MouseEvent)
 ENUM_MAPPING(CLICK,       "click",      MouseEvent)
 ENUM_MAPPING(DBL_CLICK,   "dblclick",   MouseEvent)
 ENUM_MAPPING(DRAG,        "drag",       MouseEvent)
+ENUM_MAPPING(DRAG_START,  "dragstart",  MouseEvent)
+ENUM_MAPPING(DRAG_END,    "dragend",    MouseEvent)
 ENUM_MAPPING(WHEEL,       "wheel",      MouseEvent)
 ENUM_MAPPING(MOUSE_MOVE,  "mousemove",  MouseEvent)
 ENUM_MAPPING(MOUSE_OVER,  "mouseover",  MouseEvent)

simgear/canvas/CanvasEventVisitor.cxx

@@ -1,5 +1,6 @@
-// Visitor for traversing a canvas element hierarchy similar to the traversal
-// of the DOM Level 3 Event Model
+///@file
+/// Visitor for traversing a canvas element hierarchy similar to the traversal
+/// of the DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -18,9 +19,10 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
 
 #include <simgear_config.h>
+
 #include "CanvasEvent.hxx"
 #include "CanvasEventVisitor.hxx"
-#include <simgear/canvas/elements/CanvasElement.hxx>
+#include "elements/CanvasElement.hxx"
 
 namespace simgear
 {

simgear/canvas/CanvasEventVisitor.hxx

@@ -1,5 +1,6 @@
-// Visitor for traversing a canvas element hierarchy similar to the traversal
-// of the DOM Level 3 Event Model
+///@file
+/// Visitor for traversing a canvas element hierarchy similar to the traversal
+/// of the DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/CanvasMgr.cxx

@@ -1,4 +1,5 @@
-// Canvas with 2D rendering API
+///@file
+/// Canvas with 2D rendering API
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -17,12 +18,11 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
 
 #include <simgear_config.h>
+
 #include "CanvasMgr.hxx"
 #include "Canvas.hxx"
 #include "CanvasEventManager.hxx"
 
-#include <boost/bind.hpp>
-
 namespace simgear
 {
 namespace canvas

simgear/canvas/CanvasMgr.hxx

@@ -1,4 +1,5 @@
-// Canvas with 2D rendering API
+///@file
+/// Canvas with 2D rendering API
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -61,7 +62,7 @@ namespace canvas
 
     protected:
 
-      virtual void elementCreated(PropertyBasedElementPtr element);
+      void elementCreated(PropertyBasedElementPtr element) override;
   };
 
 } // namespace canvas

simgear/canvas/CanvasObjectPlacement.cxx

@@ -1,4 +1,5 @@
-// Canvas placement for placing a canvas texture onto osg objects.
+///@file
+/// Canvas placement for placing a canvas texture onto osg objects
 //
 // It also provides a SGPickCallback for passing mouse events to the canvas and
 // manages emissive lighting of the placed canvas.

simgear/canvas/CanvasObjectPlacement.hxx

@@ -1,5 +1,5 @@
 ///@file
-/// Placement for putting a canvas texture onto OpenSceneGraph objects.
+/// Placement for putting a canvas texture onto OpenSceneGraph objects
 ///
 /// It also provides a SGPickCallback for passing mouse events to the canvas and
 /// manages emissive lighting of the placed canvas.
@@ -60,7 +60,7 @@ namespace canvas
        */
       void setCaptureEvents(bool enable);
 
-      virtual bool childChanged(SGPropertyNode* child);
+      bool childChanged(SGPropertyNode* child) override;
 
     protected:
       typedef SGSharedPtr<SGPickCallback> PickCallbackPtr;

simgear/canvas/CanvasPlacement.cxx

@@ -1,4 +1,5 @@
-// Base class for canvas placements
+///@file
+/// Base class for canvas placements
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/CanvasPlacement.hxx

@@ -1,4 +1,5 @@
-// Base class for canvas placements
+///@file
+/// Base class for canvas placements
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -40,9 +41,8 @@ namespace canvas
     protected:
       SGPropertyNode_ptr _node;
 
-    private:
-      Placement(const Placement&) /* = delete */;
-      Placement& operator=(const Placement&) /* = delete */;
+      Placement(const Placement&) = delete;
+      Placement& operator=(const Placement&) = delete;
   };
 
 } // namespace canvas

simgear/canvas/CanvasSystemAdapter.hxx

@@ -1,4 +1,5 @@
-// Adapter for using the canvas with different applications
+///@file
+/// Adapter for using the canvas with different applications
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -29,6 +30,10 @@ namespace HTTP { class Client; }
 namespace canvas
 {
 
+  /**
+   * Provides access to different required systems of the application to the
+   * Canvas
+   */
   class SystemAdapter
   {
     public:

simgear/canvas/CanvasWindow.cxx

@@ -1,4 +1,5 @@
-// Window for placing a Canvas onto it (for dialogs, menus, etc.)
+///@file
+/// Window for placing a Canvas onto it (for dialogs, menus, etc.)
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -212,6 +213,19 @@ namespace canvas
       _resize_left = getRegion().l() + offset.x();
   }
 
+  //----------------------------------------------------------------------------
+  bool Window::handleEvent(const EventPtr& event)
+  {
+    if( auto mouse_event = dynamic_cast<MouseEvent*>(event.get()) )
+    {
+      mouse_event->local_pos =
+      mouse_event->client_pos =
+        mouse_event->screen_pos - toOsg(getPosition());
+    }
+
+    return Image::handleEvent(event);
+  }
+
   //----------------------------------------------------------------------------
   void Window::parseDecorationBorder(const std::string& str)
   {

simgear/canvas/CanvasWindow.hxx

@@ -1,4 +1,5 @@
-// Window for placing a Canvas onto it (for dialogs, menus, etc.)
+///@file
+/// Window for placing a Canvas onto it (for dialogs, menus, etc.)
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -96,6 +97,8 @@ namespace canvas
       void handleResize( uint8_t mode,
                          const osg::Vec2f& offset = osg::Vec2f() );
 
+      bool handleEvent(const EventPtr& event) override;
+
     protected:
 
       enum Attributes

simgear/canvas/ODGauge.cxx

@@ -1,4 +1,5 @@
-// Owner Drawn Gauge helper class
+///@file
+/// Owner Drawn Gauge helper class
 //
 // Written by Harald JOHNSEN, started May 2005.
 //
@@ -6,9 +7,9 @@
 //
 // Ported to OSG by Tim Moore - Jun 2007
 //
-// Heavily modified to be usable for the 2d Canvas by Thomas Geymayer - April 2012
-// Supports now multisampling/mipmapping, usage of the stencil buffer and placing
-// the texture in the scene by certain filter criteria
+// Heavily modified to be usable for the 2d Canvas by Thomas Geymayer - April
+// 2012 Supports now multisampling/mipmapping, usage of the stencil buffer and
+// placing the texture in the scene by certain filter criteria.
 //
 // This library is free software; you can redistribute it and/or
 // modify it under the terms of the GNU Library General Public

simgear/canvas/ODGauge.hxx

@@ -1,4 +1,5 @@
-// Owner Drawn Gauge helper class
+///@file
+/// Owner Drawn Gauge helper class
 //
 // Written by Harald JOHNSEN, started May 2005.
 //
@@ -6,9 +7,9 @@
 //
 // Ported to OSG by Tim Moore - Jun 2007
 //
-// Heavily modified to be usable for the 2d Canvas by Thomas Geymayer - April 2012
-// Supports now multisampling/mipmapping, usage of the stencil buffer and placing
-// the texture in the scene by certain filter criteria
+// Heavily modified to be usable for the 2d Canvas by Thomas Geymayer - April
+// 2012 Supports now multisampling/mipmapping, usage of the stencil buffer and
+// placing the texture in the scene by certain filter criteria.
 //
 // This library is free software; you can redistribute it and/or
 // modify it under the terms of the GNU Library General Public

simgear/canvas/VGInitOperation.cxx

@@ -1,4 +1,5 @@
-// osg::Operation to initialize the OpenVG context used for path rendering
+///@file
+/// osg::Operation to initialize the OpenVG context used for path rendering
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/VGInitOperation.hxx

@@ -1,4 +1,5 @@
-// osg::Operation to initialize the OpenVG context used for path rendering
+///@file
+/// osg::Operation to initialize the OpenVG context used for path rendering
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/canvas_fwd.hxx

@@ -1,4 +1,5 @@
-// Canvas forward declarations
+///@file
+/// Canvas forward declarations
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/elements/CanvasElement.cxx

@@ -1,4 +1,5 @@
-// Interface for 2D Canvas element
+///@file
+/// Interface for 2D Canvas element
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -271,8 +272,7 @@ namespace canvas
   //----------------------------------------------------------------------------
   void Element::setFocus()
   {
-    CanvasPtr canvas = _canvas.lock();
-    if( canvas )
+    if( auto canvas = _canvas.lock() )
       canvas->setFocusElement(this);
   }
 

simgear/canvas/elements/CanvasElement.hxx

@@ -95,7 +95,7 @@ namespace canvas
        *
        */
       virtual ~Element() = 0;
-      virtual void onDestroy();
+      void onDestroy() override;
 
       ElementPtr getParent() const;
       CanvasWeakPtr getCanvas() const;
@@ -105,7 +105,7 @@ namespace canvas
        *
        * @param dt  Frame time in seconds
        */
-      virtual void update(double dt);
+      void update(double dt) override;
 
       bool addEventListener(const std::string& type, const EventListener& cb);
       virtual void clearEventListener();
@@ -154,11 +154,9 @@ namespace canvas
        */
       osg::Vec2f posToLocal(const osg::Vec2f& pos) const;
 
-      virtual void childAdded( SGPropertyNode * parent,
-                               SGPropertyNode * child );
-      virtual void childRemoved( SGPropertyNode * parent,
-                                 SGPropertyNode * child );
-      virtual void valueChanged(SGPropertyNode * child);
+      void childAdded(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void childRemoved(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void valueChanged(SGPropertyNode* child) override;
 
       virtual bool setStyle( const SGPropertyNode* child,
                              const StyleInfo* style_info = 0 );
@@ -597,7 +595,7 @@ namespace canvas
 
       osg::ref_ptr<osg::Drawable> _drawable;
 
-      Element(const Element&);// = delete
+      Element(const Element&) = delete;
 
       template<class Derived>
       static Derived& derived_cast(Element& el)

simgear/canvas/elements/CanvasGroup.cxx

@@ -1,4 +1,5 @@
-// A group of 2D Canvas elements
+///@file
+/// A group of 2D Canvas elements
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -168,6 +169,7 @@ namespace canvas
     if( !_scene_group.valid() )
       return warnSceneGroupExpired("clearEventListener");
 
+    // TODO should this be recursive?
     for(size_t i = 0; i < _scene_group->getNumChildren(); ++i)
       getChildByIndex(i)->clearEventListener();
   }

simgear/canvas/elements/CanvasGroup.hxx

@@ -1,4 +1,5 @@
-// A group of 2D Canvas elements
+///@file
+/// A group of 2D Canvas elements
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -86,14 +87,15 @@ namespace canvas
        */
       ElementPtr getElementById(const std::string& id);
 
-      virtual void clearEventListener();
+      void clearEventListener() override;
 
-      virtual bool traverse(EventVisitor& visitor);
+      bool traverse(EventVisitor& visitor) override;
 
-      virtual bool setStyle( const SGPropertyNode* child,
-                             const StyleInfo* style_info = 0 );
+      bool setStyle( const SGPropertyNode* child,
+                     const StyleInfo* style_info = 0 ) override;
 
-      virtual osg::BoundingBox getTransformedBounds(const osg::Matrix& m) const;
+      osg::BoundingBox
+      getTransformedBounds(const osg::Matrix& m) const override;
 
     protected:
 
@@ -105,11 +107,11 @@ namespace canvas
        */
       virtual ElementFactory getChildFactory(const std::string& type) const;
 
-      virtual void updateImpl(double dt);
+      void updateImpl(double dt) override;
 
-      virtual void childAdded(SGPropertyNode * child);
-      virtual void childRemoved(SGPropertyNode * child);
-      virtual void childChanged(SGPropertyNode * child);
+      void childAdded(SGPropertyNode * child) override;
+      void childRemoved(SGPropertyNode * child) override;
+      void childChanged(SGPropertyNode * child) override;
 
       void handleZIndexChanged(ElementPtr child, int z_index = 0);
 

simgear/canvas/elements/CanvasImage.cxx

@@ -1,4 +1,5 @@
-// An image on the Canvas
+///@file
+/// An image on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/elements/CanvasImage.hxx

@@ -1,4 +1,5 @@
-// An image on the Canvas
+///@file
+/// An image on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -53,7 +54,7 @@ namespace canvas
              ElementWeakPtr parent = 0 );
       virtual ~Image();
 
-      virtual void valueChanged(SGPropertyNode* child);
+      void valueChanged(SGPropertyNode* child) override;
 
       void setSrcCanvas(CanvasPtr canvas);
       CanvasWeakPtr getSrcCanvas() const;
@@ -93,7 +94,7 @@ namespace canvas
 
       const SGRect<float>& getRegion() const;
 
-      bool handleEvent(const EventPtr& event);
+      bool handleEvent(const EventPtr& event) override;
 
       /**
        *
@@ -108,9 +109,9 @@ namespace canvas
         SRC_CANVAS     = DEST_SIZE << 1
       };
 
-      virtual void updateImpl(double dt);
+      void updateImpl(double dt) override;
 
-      virtual void childChanged(SGPropertyNode * child);
+      void childChanged(SGPropertyNode * child) override;
 
       void setupDefaultDimensions();
       SGRect<int> getTextureDimensions() const;

simgear/canvas/elements/CanvasMap.cxx

@@ -1,5 +1,6 @@
-// A group of 2D Canvas elements which get automatically transformed according
-// to the map parameters.
+///@file
+/// A group of 2D Canvas elements which get automatically transformed according
+/// to the map parameters.
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -96,17 +97,26 @@ namespace canvas
           || (!geo_node->isDirty() && !_projection_dirty) )
         continue;
 
-      GeoCoord lat = parseGeoCoord(geo_node->getLat());
-      if( lat.type != GeoCoord::LATITUDE )
-        continue;
-
-      GeoCoord lon = parseGeoCoord(geo_node->getLon());
-      if( lon.type != GeoCoord::LONGITUDE )
-        continue;
-
-      Projection::ScreenPosition pos =
-        _projection->worldToScreen(lat.value, lon.value);
-
+      double latD = -9999.0, lonD = -9999.0;
+      if (geo_node->isDirty()) {
+        GeoCoord lat = parseGeoCoord(geo_node->getLat());
+        if( lat.type != GeoCoord::LATITUDE )
+          continue;
+        
+        GeoCoord lon = parseGeoCoord(geo_node->getLon());
+        if( lon.type != GeoCoord::LONGITUDE )
+          continue;
+        
+        // save the parsed values so we can re-use them if only projection
+        // is changed (very common case for moving vehicle)
+        latD = lat.value;
+        lonD = lon.value;
+        geo_node->setCachedLatLon(std::make_pair(latD, lonD));
+      } else {
+        std::tie(latD, lonD) = geo_node->getCachedLatLon();
+      }
+      
+      Projection::ScreenPosition pos = _projection->worldToScreen(latD, lonD);
       geo_node->setScreenPos(pos.x, pos.y);
 
 //      geo_node->print();
@@ -199,10 +209,18 @@ namespace canvas
       _projection = std::make_shared<SansonFlamsteedProjection>();
 
     _projection->setWorldPosition(_node->getDoubleValue(REF_LAT),
-                                  _node->getDoubleValue(REF_LON) );
-    _projection->setOrientation(_node->getFloatValue(HDG));
-    _projection->setScreenRange(_node->getDoubleValue(SCREEN_RANGE));
-    _projection->setRange(_node->getDoubleValue(RANGE));
+                                  _node->getDoubleValue(REF_LON));
+
+    // Only set existing properties to prevent using 0 instead of default values
+
+    if( auto heading = _node->getChild(HDG) )
+      _projection->setOrientation(heading->getFloatValue());
+
+    if( auto screen_range = _node->getChild(SCREEN_RANGE) )
+      _projection->setScreenRange(screen_range->getDoubleValue());
+
+    if( auto range = _node->getChild(RANGE) )
+      _projection->setRange(range->getDoubleValue());
 
     _projection_dirty = true;
   }

simgear/canvas/elements/CanvasMap.hxx

@@ -1,5 +1,6 @@
-// A group of 2D Canvas elements which get automatically transformed according
-// to the map parameters.
+///@file
+/// A group of 2D Canvas elements which get automatically transformed according
+/// to the map parameters.
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -46,16 +47,14 @@ namespace canvas
       virtual ~Map();
 
     protected:
-      virtual void updateImpl(double dt);
+      void updateImpl(double dt) override;
 
       void updateProjection(SGPropertyNode* type_node);
 
-      virtual void childAdded( SGPropertyNode* parent,
-                               SGPropertyNode* child );
-      virtual void childRemoved( SGPropertyNode* parent,
-                                 SGPropertyNode* child );
-      virtual void valueChanged(SGPropertyNode* child);
-      virtual void childChanged(SGPropertyNode* child);
+      void childAdded(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void childRemoved(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void valueChanged(SGPropertyNode* child) override;
+      void childChanged(SGPropertyNode* child) override;
 
       using GeoNodes =
         std::unordered_map<SGPropertyNode*, std::shared_ptr<GeoNodePair>>;

simgear/canvas/elements/CanvasPath.cxx

@@ -1,4 +1,5 @@
-// An OpenVG path on the Canvas
+///@file
+/// An OpenVG path on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -230,9 +231,12 @@ namespace canvas
           vgDestroyPaint(_paint_fill);
       }
 
-      virtual const char* className() const { return "PathDrawable"; }
-      virtual osg::Object* cloneType() const { return new PathDrawable(_path_element); }
-      virtual osg::Object* clone(const osg::CopyOp&) const { return new PathDrawable(_path_element); }
+      const char* className() const override
+      { return "PathDrawable"; }
+      osg::Object* cloneType() const override
+      { return new PathDrawable(_path_element); }
+      osg::Object* clone(const osg::CopyOp&) const override
+      { return new PathDrawable(_path_element); }
 
       /**
        * Replace the current path segments with the new ones
@@ -383,7 +387,7 @@ namespace canvas
       /**
        * Draw callback
        */
-      virtual void drawImplementation(osg::RenderInfo& renderInfo) const
+      void drawImplementation(osg::RenderInfo& renderInfo) const override
       {
         if( _attributes_dirty & PATH )
           return;
@@ -555,13 +559,13 @@ namespace canvas
       /**
        * Compute the bounding box
        */
-      virtual osg::BoundingBox
+      osg::BoundingBox
 #if OSG_VERSION_LESS_THAN(3,3,2)
       computeBound()
 #else
       computeBoundingBox()
 #endif
-      const
+      const override
       {
         if( _path == VG_INVALID_HANDLE || (_attributes_dirty & PATH) )
           return osg::BoundingBox();
@@ -667,7 +671,7 @@ namespace canvas
       struct PathUpdateCallback:
         public osg::Drawable::UpdateCallback
       {
-        virtual void update(osg::NodeVisitor*, osg::Drawable* drawable)
+        void update(osg::NodeVisitor*, osg::Drawable* drawable) override
         {
           static_cast<PathDrawable*>(drawable)->update();
         }
@@ -859,6 +863,9 @@ namespace canvas
   //----------------------------------------------------------------------------
   void Path::childChanged(SGPropertyNode* child)
   {
+    if( child->getParent() != _node )
+      return;
+      
     const std::string& name = child->getNameString();
     const std::string &prName = child->getParent()->getNameString();
 
@@ -888,16 +895,15 @@ namespace canvas
       return;
     }
 
-    if( child->getParent() != _node )
-      return;
-
     if( name == "cmd" )
       _attributes_dirty |= CMDS;
     else if( name == "coord" )
       _attributes_dirty |= COORDS;
     else if ( name == "svg")
+    {
       _hasSVG = true;
       _attributes_dirty |= SVG;
+    }
   }
 
   //----------------------------------------------------------------------------

simgear/canvas/elements/CanvasPath.hxx

@@ -1,4 +1,5 @@
-// An OpenVG path on the Canvas
+///@file
+/// An OpenVG path on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -40,7 +41,8 @@ namespace canvas
             ElementWeakPtr parent = 0 );
       virtual ~Path();
 
-      virtual osg::BoundingBox getTransformedBounds(const osg::Matrix& m) const;
+      osg::BoundingBox
+      getTransformedBounds(const osg::Matrix& m) const override;
 
       /** Add a segment with the given command and coordinates */
       Path& addSegment(uint8_t cmd, std::initializer_list<float> coords = {});
@@ -86,10 +88,10 @@ namespace canvas
       bool _hasRect : 1;
       SGRectf _rect;
 
-      virtual void updateImpl(double dt);
+      void updateImpl(double dt) override;
 
-      virtual void childRemoved(SGPropertyNode * child);
-      virtual void childChanged(SGPropertyNode * child);
+      void childRemoved(SGPropertyNode * child) override;
+      void childChanged(SGPropertyNode * child) override;
 
       void parseRectToVGPath();
   };

simgear/canvas/elements/CanvasText.cxx

@@ -1,4 +1,5 @@
-// A text on the Canvas
+///@file
+/// A text on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -17,6 +18,7 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
 
 #include <simgear_config.h>
+
 #include "CanvasText.hxx"
 #include <simgear/canvas/Canvas.hxx>
 #include <simgear/canvas/CanvasSystemAdapter.hxx>
@@ -56,21 +58,20 @@ namespace canvas
 
       SGVec2i sizeForWidth(int w) const;
 
-      virtual osg::BoundingBox
+      osg::BoundingBox
 #if OSG_VERSION_LESS_THAN(3,3,2)
       computeBound()
 #else
       computeBoundingBox()
 #endif
-      const;
+      const override;
 
     protected:
-
       friend class TextLine;
 
       canvas::Text *_text_element;
 
-      virtual void computePositions(unsigned int contextID) const;
+      void computePositions(unsigned int contextID) const override;
   };
 
   class TextLine

simgear/canvas/elements/CanvasText.hxx

@@ -1,4 +1,5 @@
-// A text on the Canvas
+///@file
+/// A text on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/elements/map/geo_node_pair.hxx

@@ -67,7 +67,8 @@ namespace canvas
       {
         _node_lat = node;
         _status &= ~LAT_MISSING;
-
+        _xNode.reset();
+          
         if( node == _node_lon )
         {
           _node_lon = 0;
@@ -79,7 +80,8 @@ namespace canvas
       {
         _node_lon = node;
         _status &= ~LON_MISSING;
-
+        _yNode.reset();
+          
         if( node == _node_lat )
         {
           _node_lat = 0;
@@ -97,19 +99,34 @@ namespace canvas
         return _node_lon ? _node_lon->getStringValue() : "";
       }
 
+      void setCachedLatLon(const std::pair<double, double>& latLon)
+      { _cachedLatLon = latLon; }
+      
+      std::pair<double, double> getCachedLatLon()
+      { return _cachedLatLon; }
+      
       void setTargetName(const std::string& name)
       {
         _target_name = name;
+        _xNode.reset();
+        _yNode.reset();
       }
 
       void setScreenPos(float x, float y)
       {
         assert( isComplete() );
-        SGPropertyNode *parent = _node_lat->getParent();
-        parent->getChild(_target_name, _node_lat->getIndex(), true)
-              ->setDoubleValue(x);
-        parent->getChild(_target_name, _node_lon->getIndex(), true)
-              ->setDoubleValue(y);
+        if (!_xNode) {
+          SGPropertyNode *parent = _node_lat->getParent();
+          _xNode = parent->getChild(_target_name, _node_lat->getIndex(), true);
+        }
+          
+        if (!_yNode) {
+          SGPropertyNode *parent = _node_lat->getParent();
+          _yNode = parent->getChild(_target_name, _node_lon->getIndex(), true);
+        }
+          
+        _xNode->setDoubleValue(x);
+        _yNode->setDoubleValue(y);
       }
 
       void print()
@@ -125,6 +142,9 @@ namespace canvas
       SGPropertyNode *_node_lat,
                      *_node_lon;
       std::string   _target_name;
+      SGPropertyNode_ptr _xNode,
+          _yNode;
+      std::pair<double, double> _cachedLatLon;
 
   };
 

simgear/canvas/elements/map/projection.hxx

@@ -1,4 +1,5 @@
-// Geographic projections for Canvas map element
+///@file
+/// Geographic projections for Canvas map element
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/events/CustomEvent.cxx

@@ -1,4 +1,5 @@
-// Canvas user defined event
+///@file
+/// Canvas user defined event
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -46,6 +47,14 @@ namespace canvas
 //    assert( type_map.find(type_id) != type_map.end() );
   }
 
+  //----------------------------------------------------------------------------
+  CustomEvent* CustomEvent::clone(int type) const
+  {
+    auto event = new CustomEvent(*this);
+    event->type = type;
+    return event;
+  }
+
   //----------------------------------------------------------------------------
   void CustomEvent::setDetail(StringMap const& data)
   {

simgear/canvas/events/CustomEvent.hxx

@@ -38,6 +38,7 @@ namespace canvas
     public:
 
       /**
+       * @brief Construct a user defined event from a type string
        *
        * @param type_str    Event type name (if name does not exist yet it will
        *                    be registered as new event type)
@@ -49,6 +50,10 @@ namespace canvas
                    StringMap const& data = StringMap() );
 
       /**
+       * @brief Construct a user defined event from a (previously registered)
+       *        type id
+       *
+       * @see getOrRegisterType()
        *
        * @param type_id     Event type id
        * @param bubbles     If this event should take part in the bubbling phase
@@ -58,6 +63,8 @@ namespace canvas
                    bool bubbles = false,
                    StringMap const& data = StringMap() );
 
+      CustomEvent* clone(int type = 0) const override;
+
       /**
        * Set user data
        */
@@ -74,7 +81,7 @@ namespace canvas
        * @see #bubbles
        * @see CustomEvent()
        */
-      virtual bool canBubble() const { return bubbles; }
+      bool canBubble() const override { return bubbles; }
 
       StringMap detail; //!< User data map
       bool bubbles;     //!< Whether the event supports bubbling

simgear/canvas/events/DeviceEvent.cxx

@@ -1,4 +1,5 @@
-// Input device event
+///@file
+/// Input device event
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/events/KeyboardEvent.cxx

@@ -1,4 +1,5 @@
-// Keyboard event
+///@file
+/// Keyboard event
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -69,6 +70,14 @@ namespace canvas
 //      // TODO what to do with wrong event type?
   }
 
+  //----------------------------------------------------------------------------
+  KeyboardEvent* KeyboardEvent::clone(int type) const
+  {
+    auto event = new KeyboardEvent(*this);
+    event->type = type;
+    return event;
+  }
+
   //----------------------------------------------------------------------------
   void KeyboardEvent::setKey(uint32_t key)
   {

simgear/canvas/events/KeyboardEvent.hxx

@@ -45,6 +45,7 @@ namespace canvas
 
       KeyboardEvent();
       KeyboardEvent(const osgGA::GUIEventAdapter& ea);
+      KeyboardEvent* clone(int type = 0) const override;
 
       void setKey(uint32_t key);
       void setUnmodifiedKey(uint32_t key);

simgear/canvas/events/MouseEvent.cxx

@@ -1,4 +1,5 @@
-// Mouse event
+///@file
+/// Mouse event
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -48,6 +49,14 @@ namespace canvas
       button += 1;
   }
 
+  //----------------------------------------------------------------------------
+  MouseEvent* MouseEvent::clone(int type) const
+  {
+    auto event = new MouseEvent(*this);
+    event->type = type;
+    return event;
+  }
+
   //----------------------------------------------------------------------------
   bool MouseEvent::canBubble() const
   {

simgear/canvas/events/MouseEvent.hxx

@@ -36,8 +36,9 @@ namespace canvas
     public:
       MouseEvent();
       MouseEvent(const osgGA::GUIEventAdapter& ea);
+      MouseEvent* clone(int type = 0) const override;
 
-      virtual bool canBubble() const;
+      bool canBubble() const override;
 
       osg::Vec2f getScreenPos() const { return screen_pos; }
       osg::Vec2f getClientPos() const { return client_pos; }

simgear/canvas/events/input_event_demo.cxx

@@ -1,4 +1,5 @@
-// Keyboard event demo. Press some keys and get some info...
+///@file
+/// Keyboard event demo. Press some keys and get some info...
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/debug/CMakeLists.txt

@@ -1,7 +1,7 @@
 
 include (SimGearComponent)
 
-set(HEADERS debug_types.h logstream.hxx BufferedLogCallback.hxx)
+set(HEADERS debug_types.h logstream.hxx BufferedLogCallback.hxx OsgIoCapture.hxx)
 set(SOURCES logstream.cxx BufferedLogCallback.cxx)
 
-simgear_component(debug debug "${SOURCES}" "${HEADERS}")
+simgear_component(debug debug "${SOURCES}" "${HEADERS}")

simgear/debug/OsgIoCapture.hxx

@@ -0,0 +1,45 @@
+#include <cstring>
+
+#include <osg/Notify>
+
+using namespace osg;
+
+
+/**
+ * merge OSG output into our logging system, so it gets recorded to file,
+ * and so we can display a GUI console with renderer issues, especially
+ * shader compilation warnings and errors.
+ */
+class NotifyLogger : public osg::NotifyHandler
+{
+public:
+    // note this callback will be invoked by OSG from multiple threads.
+    // fortunately our Simgear logging implementation already handles
+    // that internally, so we simply pass the message on.
+    virtual void notify(osg::NotifySeverity severity, const char* message) {
+        // Detect whether a osg::Reference derived object is deleted with a non-zero
+        // reference count. In this case trigger a segfault to get a stack trace.
+        if (strstr(message, "the final reference count was")) {
+            // as this is going to segfault ignore the translation of severity and always output the message.
+            SG_LOG(SG_GL, SG_ALERT, message);
+            int* trigger_segfault = 0;
+            *trigger_segfault     = 0;
+            return;
+        }
+        SG_LOG(SG_GL, translateSeverity(severity), message);
+    }
+
+private:
+    sgDebugPriority translateSeverity(osg::NotifySeverity severity) {
+        switch (severity) {
+            case osg::ALWAYS:
+            case osg::FATAL:      return SG_ALERT;
+            case osg::WARN:       return SG_WARN;
+            case osg::NOTICE:
+            case osg::INFO:       return SG_INFO;
+            case osg::DEBUG_FP:
+            case osg::DEBUG_INFO: return SG_DEBUG;
+            default:              return SG_ALERT;
+        }
+    }
+};

simgear/debug/debug_types.h

@@ -34,7 +34,8 @@ typedef enum {
     SG_GUI         = 0x00800000,
     SG_TERRASYNC   = 0x01000000,
     SG_PARTICLES   = 0x02000000,
-    SG_UNDEFD      = 0x04000000, // For range checking
+    SG_HEADLESS    = 0x04000000,
+    SG_UNDEFD      = 0x08000000, // For range checking
 
     SG_ALL         = 0xFFFFFFFF
 } sgDebugClass;

simgear/debug/logstream.cxx

@@ -100,6 +100,7 @@ const char* LogCallback::debugClassToString(sgDebugClass c)
         case SG_GUI:        return "gui";
         case SG_TERRASYNC:  return "terrasync";
         case SG_PARTICLES:  return "particles";
+        case SG_HEADLESS:   return "headless";
         default:            return "unknown";
     }
 }
@@ -335,9 +336,7 @@ public:
 
     ~LogStreamPrivate()
     {
-        for (simgear::LogCallback* cb : m_callbacks) {
-            delete cb;
-        }
+        removeCallbacks();
     }
 
     SGMutex m_lock;
@@ -363,6 +362,9 @@ public:
 #endif
     bool m_developerMode = false;
 
+    // test suite mode.
+    bool m_testMode = false;
+
     void startLog()
     {
         SGGuard<SGMutex> g(m_lock);
@@ -443,6 +445,16 @@ public:
         }
     }
 
+    void removeCallbacks()
+    {
+        PauseThread pause(this);
+        for (simgear::LogCallback* cb : m_callbacks) {
+            delete cb;
+        }
+        m_callbacks.clear();
+        m_consoleCallbacks.clear();
+    }
+
     void setLogLevels( sgDebugClass c, sgDebugPriority p )
     {
         PauseThread pause(this);
@@ -455,6 +467,9 @@ public:
 
     bool would_log( sgDebugClass c, sgDebugPriority p ) const
     {
+        // Testing mode, so always log.
+        if (m_testMode) return true;
+
         p = translatePriority(p);
         if (p >= SG_INFO) return true;
         return ((c & m_logClass) != 0 && p >= m_logPriority);
@@ -711,6 +726,13 @@ void logstream::requestConsole()
 #endif
 }
 
+void
+logstream::setTestingMode( bool testMode )
+{
+    d->m_testMode = testMode;
+    if (testMode) d->removeCallbacks();
+}
+
 
 namespace simgear
 {

simgear/debug/logstream.hxx

@@ -158,12 +158,22 @@ public:
      
     void removeCallback(simgear::LogCallback* cb);
 
+    void removeCallbacks();
+
     /**
      * optionally record all entries and submit them to new log callbacks that
      * are added. This allows simplified logging configuration, but still including
      * early startup information in all logs.
      */
     void setStartupLoggingEnabled(bool enabled);
+
+    /**
+     * Set up the logstream for running in test mode.  For example the callbacks
+     * will be unregistered and the behaviour of the would_log() function
+     * sanitized.
+     */
+    void setTestingMode(bool testMode);
+
 private:
     // constructor
     logstream();

simgear/io/HTTPClient.cxx

@@ -242,8 +242,13 @@ void Client::makeRequest(const Request_ptr& r)
     if( r->isComplete() )
       return;
 
+    if (r->url().empty()) {
+        r->setFailure(EINVAL, "no URL specified on request");
+        return;
+    }
+
     if( r->url().find("://") == std::string::npos ) {
-        r->setFailure(EINVAL, "malformed URL");
+        r->setFailure(EINVAL, "malformed URL: '" + r->url() + "'");
         return;
     }
 
@@ -467,12 +472,26 @@ size_t Client::requestReadCallback(char *ptr, size_t size, size_t nmemb, void *u
   return actualBytes;
 }
 
+bool isRedirectStatus(int code)
+{
+    return ((code >= 300) && (code < 400));
+}
+    
 size_t Client::requestHeaderCallback(char *rawBuffer, size_t size, size_t nitems, void *userdata)
 {
   size_t byteSize = size * nitems;
   Request* req = static_cast<Request*>(userdata);
   std::string h = strutils::simplify(std::string(rawBuffer, byteSize));
 
+  if (req->readyState() >= HTTP::Request::HEADERS_RECEIVED) {
+      // this can happen with chunked transfers (secondary chunks)
+      // or redirects
+      if (isRedirectStatus(req->responseCode())) {
+          req->responseStart(h);
+          return byteSize;
+      }
+  }
+    
   if (req->readyState() == HTTP::Request::OPENED) {
     req->responseStart(h);
     return byteSize;

simgear/io/HTTPRepository.cxx

@@ -75,6 +75,25 @@ namespace simgear
 
     typedef SGSharedPtr<HTTPRepoGetRequest> RepoRequestPtr;
 
+    std::string innerResultCodeAsString(HTTPRepository::ResultCode code)
+    {
+        switch (code) {
+        case HTTPRepository::REPO_NO_ERROR:     return "no error";
+        case HTTPRepository::REPO_ERROR_NOT_FOUND:  return "not found";
+        case HTTPRepository::REPO_ERROR_SOCKET:     return "socket error";
+        case HTTPRepository::SVN_ERROR_XML:        return "malformed XML";
+        case HTTPRepository::SVN_ERROR_TXDELTA:        return "malformed XML";
+        case HTTPRepository::REPO_ERROR_IO:         return "I/O error";
+        case HTTPRepository::REPO_ERROR_CHECKSUM:         return "checksum verification error";
+        case HTTPRepository::REPO_ERROR_FILE_NOT_FOUND: return "file not found";
+        case HTTPRepository::REPO_ERROR_HTTP:   return "HTTP-level error";
+        case HTTPRepository::REPO_ERROR_CANCELLED:   return "cancelled";
+        case HTTPRepository::REPO_PARTIAL_UPDATE:   return "partial update (incomplete)";
+        }
+
+        return "Unknown response code";
+    }
+
 class HTTPRepoPrivate
 {
 public:
@@ -163,17 +182,11 @@ class HTTPDirectory
         ChildInfo(Type ty, const std::string & nameData, const std::string & hashData) :
             type(ty),
             name(nameData),
-            hash(hashData),
-            sizeInBytes(0)
+            hash(hashData)
         {
         }
 
-        ChildInfo(const ChildInfo& other) :
-            type(other.type),
-            name(other.name),
-            hash(other.hash),
-            sizeInBytes(other.sizeInBytes)
-        { }
+		ChildInfo(const ChildInfo& other) = default;
 
         void setSize(const std::string & sizeData)
         {
@@ -187,7 +200,8 @@ class HTTPDirectory
 
         Type type;
         std::string name, hash;
-        size_t sizeInBytes;
+		size_t sizeInBytes = 0;
+		SGPath path; // absolute path on disk
     };
 
     typedef std::vector<ChildInfo> ChildInfoList;
@@ -254,46 +268,45 @@ public:
             return;
         }
 
-        string_list indexNames = indexChildren();
-        const_string_list_iterator nameIt = indexNames.begin();
-        for (; nameIt != indexNames.end(); ++nameIt) {
-            SGPath p(absolutePath());
-            p.append(*nameIt);
-            if (p.exists()) {
-                continue; // only copy if the file is missing entirely
-            }
+		char* buf = nullptr;
+		size_t bufSize = 0;
 
-            ChildInfoList::iterator c = findIndexChild(*nameIt);
-            if (c->type == ChildInfo::DirectoryType) {
-                continue; // only care about files
-            }
+		for (const auto& child : children) {
+			if (child.type != ChildInfo::FileType)
+				continue;
 
-            SGPath cp = _repository->installedCopyPath;
-            cp.append(relativePath());
-            cp.append(*nameIt);
-            if (!cp.exists()) {
-                continue;
-            }
+			if (child.path.exists())
+				continue;
 
-            SG_LOG(SG_TERRASYNC, SG_BULK, "new child, copying existing file" << cp << p);
+			SGPath cp = _repository->installedCopyPath;
+			cp.append(relativePath());
+			cp.append(child.name);
+			if (!cp.exists()) {
+				continue;
+			}
 
-            SGBinaryFile src(cp);
-            SGBinaryFile dst(p);
-            src.open(SG_IO_IN);
-            dst.open(SG_IO_OUT);
+			SGBinaryFile src(cp);
+			SGBinaryFile dst(child.path);
+			src.open(SG_IO_IN);
+			dst.open(SG_IO_OUT);
 
-            char* buf = (char*) malloc(cp.sizeInBytes());
-            if (!buf) {
-                continue;
-            }
+			if (bufSize < cp.sizeInBytes()) {
+				bufSize = cp.sizeInBytes();
+				free(buf);
+				buf = (char*) malloc(bufSize);
+				if (!buf) {
+					continue;
+				}
+			}
 
-            src.read(buf, cp.sizeInBytes());
-            dst.write(buf, cp.sizeInBytes());
-            src.close();
-            dst.close();
+			src.read(buf, cp.sizeInBytes());
+			dst.write(buf, cp.sizeInBytes());
+			src.close();
+			dst.close();
 
-            free(buf);
-        }
+		}
+
+		free(buf);
     }
 
     void updateChildrenBasedOnHash()
@@ -302,38 +315,40 @@ public:
 
         copyInstalledChildren();
 
-        string_list indexNames = indexChildren(),
-            toBeUpdated, orphans;
+		string_list toBeUpdated, orphans,
+			indexNames = indexChildren();
         simgear::Dir d(absolutePath());
         PathList fsChildren = d.children(0);
-        PathList::const_iterator it = fsChildren.begin();
 
+        for (const auto& child : fsChildren) {
+			const auto& fileName = child.file();
+			if ((fileName == ".dirindex") || (fileName == ".hashes")) {
+				continue;
+			}
 
-        for (; it != fsChildren.end(); ++it) {
-            ChildInfo info(it->isDir() ? ChildInfo::DirectoryType : ChildInfo::FileType,
-                           it->file(), "");
+            ChildInfo info(child.isDir() ? ChildInfo::DirectoryType : ChildInfo::FileType, 
+				fileName, "");
+			info.path = child;
             std::string hash = hashForChild(info);
 
-            ChildInfoList::iterator c = findIndexChild(it->file());
+            ChildInfoList::iterator c = findIndexChild(fileName);
             if (c == children.end()) {
-                SG_LOG(SG_TERRASYNC, SG_DEBUG, "is orphan '" << it->file() << "'" );
-
-				orphans.push_back(it->file());
+                orphans.push_back(fileName);
             } else if (c->hash != hash) {
-                SG_LOG(SG_TERRASYNC, SG_DEBUG, "hash mismatch'" << it->file() );
+#if 0
+                SG_LOG(SG_TERRASYNC, SG_DEBUG, "hash mismatch'" << fileName);
                 // file exists, but hash mismatch, schedule update
                 if (!hash.empty()) {
-                    SG_LOG(SG_TERRASYNC, SG_DEBUG, "file exists but hash is wrong for:" << it->file() );
+                    SG_LOG(SG_TERRASYNC, SG_DEBUG, "file exists but hash is wrong for:" << fileName);
                     SG_LOG(SG_TERRASYNC, SG_DEBUG, "on disk:" << hash << " vs in info:" << c->hash);
                 }
-
-                toBeUpdated.push_back(it->file() );
+#endif
+                toBeUpdated.push_back(fileName);
             } else {
                 // file exists and hash is valid. If it's a directory,
                 // perform a recursive check.
-                SG_LOG(SG_TERRASYNC, SG_DEBUG, "file exists hash is good:" << it->file() );
                 if (c->type == ChildInfo::DirectoryType) {
-                    HTTPDirectory* childDir = childDirectory(it->file());
+                    HTTPDirectory* childDir = childDirectory(fileName);
                     childDir->updateChildrenBasedOnHash();
                 }
             }
@@ -341,7 +356,7 @@ public:
             // remove existing file system children from the index list,
             // so we can detect new children
             // https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Erase-Remove
-            indexNames.erase(std::remove(indexNames.begin(), indexNames.end(), it->file()), indexNames.end());
+            indexNames.erase(std::remove(indexNames.begin(), indexNames.end(), fileName), indexNames.end());
         } // of real children iteration
 
         // all remaining names in indexChilden are new children
@@ -386,7 +401,6 @@ public:
                 continue;
             }
 
-            SG_LOG(SG_TERRASYNC,SG_DEBUG, "scheduling update for " << *it );
             if (cit->type == ChildInfo::FileType) {
                 _repository->updateFile(this, *it, cit->sizeInBytes);
             } else {
@@ -411,19 +425,16 @@ public:
     void didUpdateFile(const std::string& file, const std::string& hash, size_t sz)
     {
         // check hash matches what we expected
-        ChildInfoList::iterator it = findIndexChild(file);
+        auto it = findIndexChild(file);
         if (it == children.end()) {
             SG_LOG(SG_TERRASYNC, SG_WARN, "updated file but not found in dir:" << _relativePath << " " << file);
         } else {
-            SGPath fpath(absolutePath());
-            fpath.append(file);
-
             if (it->hash != hash) {
                 // we don't erase the file on a hash mismatch, becuase if we're syncing during the
                 // middle of a server-side update, the downloaded file may actually become valid.
                 _repository->failedToUpdateChild(_relativePath, HTTPRepository::REPO_ERROR_CHECKSUM);
             } else {
-                _repository->updatedFileContents(fpath, hash);
+                _repository->updatedFileContents(it->path, hash);
                 _repository->totalDownloaded += sz;
             } // of hash matches
         } // of found in child list
@@ -481,11 +492,13 @@ private:
 
             if( typeData == "version" ) {
                 if( tokens.size() < 2 ) {
-                    SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: missing version number in line '" << line << "'" );
+                    SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: missing version number in line '" << line << "'"
+                           << "\n\tparsing:" << p.utf8Str());
                     break;
                 }
                 if( tokens[1] != "1" ) {
-                    SG_LOG(SG_TERRASYNC, SG_WARN, "invalid .dirindex file: wrong version number '" << tokens[1] << "' (expected 1)" );
+                    SG_LOG(SG_TERRASYNC, SG_WARN, "invalid .dirindex file: wrong version number '" << tokens[1] << "' (expected 1)"
+                           << "\n\tparsing:" << p.utf8Str());
                     break;
                 }
                 continue; // version is good, continue
@@ -496,29 +509,32 @@ private:
             }
 
             if( typeData == "time" && tokens.size() > 1 ) {
-                SG_LOG(SG_TERRASYNC, SG_INFO, ".dirindex at '" << p.str() << "' timestamp: " << tokens[1] );
+               // SG_LOG(SG_TERRASYNC, SG_INFO, ".dirindex at '" << p.str() << "' timestamp: " << tokens[1] );
                 continue;
             }
 
             if( tokens.size() < 3 ) {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: not enough tokens in line '" << line << "' (ignoring line)" );
+                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: not enough tokens in line '" << line << "' (ignoring line)"
+                       << "\n\tparsing:" << p.utf8Str());
                 continue;
             }
 
             if (typeData != "f" && typeData != "d" ) {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: invalid type in line '" << line << "', expected 'd' or 'f', (ignoring line)" );
+                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: invalid type in line '" << line << "', expected 'd' or 'f', (ignoring line)"
+                       << "\n\tparsing:" << p.utf8Str());
                 continue;
             }
 
             // security: prevent writing outside the repository via ../../.. filenames
             // (valid filenames never contain / - subdirectories have their own .dirindex)
             if ((tokens[1] == "..") || (tokens[1].find_first_of("/\\") != std::string::npos)) {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: invalid filename in line '" << line << "', (ignoring line)" );
+                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: invalid filename in line '" << line << "', (ignoring line)"
+                       << "\n\tparsing:" << p.utf8Str());
                 continue;
             }
 
-            children.push_back(ChildInfo(typeData == "f" ? ChildInfo::FileType : ChildInfo::DirectoryType, tokens[1], tokens[2]));
-
+            children.emplace_back(ChildInfo(typeData == "f" ? ChildInfo::FileType : ChildInfo::DirectoryType, tokens[1], tokens[2]));
+			children.back().path = absolutePath() / tokens[1];
             if (tokens.size() > 3) {
                 children.back().setSize(tokens[3]);
             }
@@ -550,8 +566,7 @@ private:
 
     std::string hashForChild(const ChildInfo& child) const
     {
-        SGPath p(absolutePath());
-        p.append(child.name);
+		SGPath p(child.path);
         if (child.type == ChildInfo::DirectoryType) {
             p.append(".dirindex");
         }
@@ -656,6 +671,11 @@ void HTTPRepository::setInstalledCopyPath(const SGPath& copyPath)
     _d->installedCopyPath = copyPath;
 }
 
+std::string HTTPRepository::resultCodeAsString(ResultCode code)
+{
+    return innerResultCodeAsString(code);
+}
+
 HTTPRepository::ResultCode
 HTTPRepository::failure() const
 {
@@ -668,7 +688,7 @@ HTTPRepository::failure() const
 
     void HTTPRepoGetRequest::cancel()
     {
-        _directory->repository()->http->cancelRequest(this, "Reposiotry cancelled");
+        _directory->repository()->http->cancelRequest(this, "Repository cancelled");
         _directory = 0;
     }
 
@@ -690,7 +710,7 @@ HTTPRepository::failure() const
                 file.reset(new SGBinaryFile(pathInRepo));
                 if (!file->open(SG_IO_OUT)) {
                   SG_LOG(SG_TERRASYNC, SG_WARN, "unable to create file " << pathInRepo);
-                  _directory->repository()->http->cancelRequest(this, "Unable to create output file");
+                    _directory->repository()->http->cancelRequest(this, "Unable to create output file:" + pathInRepo.utf8Str());
                 }
 
                 sha1_init(&hashContext);
@@ -706,12 +726,12 @@ HTTPRepository::failure() const
             if (responseCode() == 200) {
                 std::string hash = strutils::encodeHex(sha1_result(&hashContext), HASH_LENGTH);
                 _directory->didUpdateFile(fileName, hash, contentSize());
-                SG_LOG(SG_TERRASYNC, SG_DEBUG, "got file " << fileName << " in " << _directory->absolutePath());
             } else if (responseCode() == 404) {
                 SG_LOG(SG_TERRASYNC, SG_WARN, "terrasync file not found on server: " << fileName << " for " << _directory->absolutePath());
                 _directory->didFailToUpdateFile(fileName, HTTPRepository::REPO_ERROR_FILE_NOT_FOUND);
             } else {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "terrasync file download error on server: " << fileName << " for " << _directory->absolutePath() << ": " << responseCode() );
+                SG_LOG(SG_TERRASYNC, SG_WARN, "terrasync file download error on server: " << fileName << " for " << _directory->absolutePath() <<
+                       "\n\tserver responded: " << responseCode() << "/" << responseReason());
                 _directory->didFailToUpdateFile(fileName, HTTPRepository::REPO_ERROR_HTTP);
             }
 
@@ -720,13 +740,22 @@ HTTPRepository::failure() const
 
         virtual void onFail()
         {
+            HTTPRepository::ResultCode code = HTTPRepository::REPO_ERROR_SOCKET;
+            if (responseCode() == -1) {
+                code = HTTPRepository::REPO_ERROR_CANCELLED;
+            }
+
+			if (file) {
+				file->close();
+			}
+
             file.reset();
             if (pathInRepo.exists()) {
                 pathInRepo.remove();
             }
 
             if (_directory) {
-                _directory->didFailToUpdateFile(fileName, HTTPRepository::REPO_ERROR_SOCKET);
+                _directory->didFailToUpdateFile(fileName, code);
                 _directory->repository()->finishedRequest(this);
             }
         }
@@ -791,7 +820,7 @@ HTTPRepository::failure() const
 
                     // dir index data has changed, so write to disk and update
                     // the hash accordingly
-                    sg_ofstream of(pathInRepo(), std::ios::trunc | std::ios::out);
+                    sg_ofstream of(pathInRepo(), std::ios::trunc | std::ios::out | std::ios::binary);
                     if (!of.is_open()) {
                         throw sg_io_exception("Failed to open directory index file for writing", pathInRepo());
                     }
@@ -970,11 +999,11 @@ HTTPRepository::failure() const
 
         SGPath cachePath = basePath;
         cachePath.append(".hashes");
-        sg_ofstream stream(cachePath, std::ios::out | std::ios::trunc);
+        sg_ofstream stream(cachePath, std::ios::out | std::ios::trunc | std::ios::binary);
         HashCache::const_iterator it;
         for (it = hashes.begin(); it != hashes.end(); ++it) {
-            stream << it->filePath << ":" << it->modTime << ":"
-            << it->lengthBytes << ":" << it->hashHex << "\n";
+            stream << it->filePath << "*" << it->modTime << "*"
+            << it->lengthBytes << "*" << it->hashHex << "\n";
         }
         stream.close();
         hashCacheDirty = false;
@@ -998,7 +1027,7 @@ HTTPRepository::failure() const
             if( line.empty() || line[0] == '#' )
                 continue;
 
-            string_list tokens = simgear::strutils::split( line, ":" );
+			string_list tokens = simgear::strutils::split(line, "*");
             if( tokens.size() < 4 ) {
                 SG_LOG(SG_TERRASYNC, SG_WARN, "invalid entry in '" << cachePath << "': '" << line << "' (ignoring line)");
                 continue;
@@ -1057,7 +1086,7 @@ HTTPRepository::failure() const
 		} else {
 			// we encounter this code path when deleting an orphaned directory
 		}
-		
+
 		Dir dir(absPath);
 		bool result = dir.remove(true);
 
@@ -1121,13 +1150,15 @@ HTTPRepository::failure() const
             RequestVector copyOfActive(activeRequests);
             RequestVector::iterator rq;
             for (rq = copyOfActive.begin(); rq != copyOfActive.end(); ++rq) {
-                //SG_LOG(SG_TERRASYNC, SG_DEBUG, "cancelling request for:" << (*rq)->url());
-                http->cancelRequest(*rq, "Repository updated failed");
+                http->cancelRequest(*rq, "Repository updated failed due to checksum error");
             }
 
-
             SG_LOG(SG_TERRASYNC, SG_WARN, "failed to update repository:" << baseUrl
-                   << ", possibly modified during sync");
+                   << "\n\tchecksum failure for: " << relativePath
+                   << "\n\tthis typically indicates the remote repository is corrupt or was being updated during the sync");
+        } else if (fileStatus == HTTPRepository::REPO_ERROR_CANCELLED) {
+            // if we were cancelled, don't report or log
+            return;
         }
 
         Failure f;
@@ -1135,7 +1166,8 @@ HTTPRepository::failure() const
         f.error = fileStatus;
         failures.push_back(f);
 
-        SG_LOG(SG_TERRASYNC, SG_WARN, "failed to update entry:" << relativePath << " code:" << fileStatus);
+        SG_LOG(SG_TERRASYNC, SG_WARN, "failed to update entry:" << relativePath << " status/code: "
+               << innerResultCodeAsString(fileStatus) << "/" << fileStatus);
     }
 
 } // of namespace simgear

simgear/io/HTTPRepository.hxx

@@ -42,6 +42,7 @@ public:
         REPO_ERROR_CHECKSUM,
         REPO_ERROR_FILE_NOT_FOUND,
         REPO_ERROR_HTTP,
+        REPO_ERROR_CANCELLED,
         REPO_PARTIAL_UPDATE
     };
 
@@ -70,6 +71,9 @@ public:
      * repository. When a file is missing it will be copied from this tree.
      */
     void setInstalledCopyPath(const SGPath& copyPath);
+    
+    static std::string resultCodeAsString(ResultCode code);
+
 private:
     bool isBare() const;
 

simgear/io/HTTPRequest.cxx

@@ -328,6 +328,16 @@ unsigned int Request::responseLength() const
   return _responseLength;
 }
 
+//------------------------------------------------------------------------------
+void Request::setSuccess(int code)
+{
+    _responseStatus = code;
+    _responseReason.clear();
+    if( !isComplete() ) {
+        setReadyState(DONE);
+    }
+}
+    
 //------------------------------------------------------------------------------
 void Request::setFailure(int code, const std::string& reason)
 {

simgear/io/HTTPRequest.hxx

@@ -224,7 +224,7 @@ protected:
     virtual void onAlways();
 
     void setFailure(int code, const std::string& reason);
-
+    void setSuccess(int code);
 private:
     friend class Client;
     friend class Connection;

simgear/io/http_repo_sync.cxx

@@ -5,8 +5,6 @@
 #include <signal.h>
 
 #include <iostream>
-#include <boost/foreach.hpp>
-
 
 #include <simgear/io/sg_file.hxx>
 #include <simgear/io/HTTPClient.hxx>

simgear/io/sg_file.hxx

@@ -62,7 +62,7 @@ public:
     SGFile( int existingFd );
 
     /** Destructor */
-    ~SGFile();
+    virtual ~SGFile();
 
     // open the file based on specified direction
     bool open( const SGProtocolDir dir );

simgear/io/test_HTTP.cxx

@@ -273,7 +273,23 @@ public:
             d << "\r\n"; // final CRLF to terminate the headers
             d << contentStr;
             push(d.str().c_str());
-
+        } else if (path == "/test_redirect") {
+            string contentStr("<html>See <a href=\"wibble\">Here</a></html>");
+            stringstream d;
+            d << "HTTP/1.1 " << 302 << " " << "Found" << "\r\n";
+            d << "Location:" << " http://localhost:2000/was_redirected" << "\r\n";
+            d << "Content-Length:" << contentStr.size() << "\r\n";
+            d << "\r\n"; // final CRLF to terminate the headers
+            d << contentStr;
+            push(d.str().c_str());
+        } else if (path == "/was_redirected") {
+            string contentStr(BODY1);
+            stringstream d;
+            d << "HTTP/1.1 " << 200 << " " << reasonForCode(200) << "\r\n";
+            d << "Content-Length:" << contentStr.size() << "\r\n";
+            d << "\r\n"; // final CRLF to terminate the headers
+            d << contentStr;
+            push(d.str().c_str());
         } else {
           TestServerChannel::processRequestHeaders();
         }
@@ -773,6 +789,24 @@ cout << "testing proxy close" << endl;
         SG_CHECK_EQUAL(tr2->bodyData, string(BODY1));
         SG_CHECK_EQUAL(tr2->responseBytesReceived(), strlen(BODY1));
     }
+    
+    {
+        cout << "redirect test" << endl;
+        // redirect test
+        testServer.disconnectAll();
+        cl.clearAllConnections();
+        
+        TestRequest* tr = new TestRequest("http://localhost:2000/test_redirect");
+        HTTP::Request_ptr own(tr);
+        cl.makeRequest(tr);
+        
+        waitForComplete(&cl, tr);
+        SG_CHECK_EQUAL(tr->responseCode(), 200);
+        SG_CHECK_EQUAL(tr->responseReason(), string("OK"));
+        SG_CHECK_EQUAL(tr->responseLength(), strlen(BODY1));
+        SG_CHECK_EQUAL(tr->responseBytesReceived(), strlen(BODY1));
+        SG_CHECK_EQUAL(tr->bodyData, string(BODY1));
+    }
 
     cout << "all tests passed ok" << endl;
     return EXIT_SUCCESS;

simgear/io/test_HTTP.hxx

@@ -30,7 +30,6 @@ public:
 
     virtual ~TestServerChannel()
     {
-        std::cerr << "dtor test server channel" << std::endl;
     }
 
     virtual void collectIncomingData(const char* s, int n)
@@ -139,8 +138,8 @@ public:
 
     void sendErrorResponse(int code, bool close, std::string content)
     {
-        std::cerr << "sending error " << code << " for " << path << std::endl;
-        std::cerr << "\tcontent:" << content << std::endl;
+     //   std::cerr << "sending error " << code << " for " << path << std::endl;
+       // std::cerr << "\tcontent:" << content << std::endl;
 
         std::stringstream headerData;
         headerData << "HTTP/1.1 " << code << " " << reasonForCode(code) << "\r\n";
@@ -168,7 +167,6 @@ public:
 
     virtual void handleClose (void)
     {
-        std::cerr << "channel close" << std::endl;
         NetBufferChannel::handleClose();
     }
 

simgear/io/test_repository.cxx

@@ -309,7 +309,8 @@ std::string test_computeHashForPath(const SGPath& p)
         return std::string();
     sha1nfo info;
     sha1_init(&info);
-    char* buf = static_cast<char*>(alloca(1024 * 1024));
+    char* buf = static_cast<char*>(malloc(1024 * 1024));
+	assert(buf);
     size_t readLen;
 
     SGBinaryFile f(p);
@@ -319,6 +320,9 @@ std::string test_computeHashForPath(const SGPath& p)
         sha1_write(&info, buf, readLen);
     }
 
+	f.close();
+	free(buf);
+
     std::string hashBytes((char*) sha1_result(&info), HASH_LENGTH);
     return strutils::encodeHex(hashBytes);
 }
@@ -433,6 +437,34 @@ void testBasicClone(HTTP::Client* cl)
     std::cout << "Passed test: basic clone and update" << std::endl;
 }
 
+void testUpdateNoChanges(HTTP::Client* cl)
+{
+	std::unique_ptr<HTTPRepository> repo;
+	SGPath p(simgear::Dir::current().path());
+	p.append("http_repo_basic"); // same as before
+
+	global_repo->clearRequestCounts();
+
+	repo.reset(new HTTPRepository(p, cl));
+	repo->setBaseUrl("http://localhost:2000/repo");
+	repo->update();
+
+	waitForUpdateComplete(cl, repo.get());
+
+	verifyFileState(p, "fileA");
+	verifyFileState(p, "dirC/subdirA/subsubA/fileCAAA");
+
+	verifyRequestCount("dirA", 0);
+	verifyRequestCount("dirB", 0);
+	verifyRequestCount("dirB/subdirA", 0);
+	verifyRequestCount("dirB/subdirA/fileBAA", 0);
+	verifyRequestCount("dirC", 0);
+	verifyRequestCount("dirC/fileCA", 0);
+
+	std::cout << "Passed test:no changes update" << std::endl;
+
+}
+
 void testModifyLocalFiles(HTTP::Client* cl)
 {
     std::unique_ptr<HTTPRepository> repo;
@@ -469,10 +501,6 @@ void testModifyLocalFiles(HTTP::Client* cl)
     std::cout << "Passed test: identify and fix locally modified files" << std::endl;
 }
 
-void testNoChangesUpdate()
-{
-
-}
 
 void testMergeExistingFileWithoutDownload(HTTP::Client* cl)
 {
@@ -725,7 +753,7 @@ void testCopyInstalledChildren(HTTP::Client* cl)
     verifyRequestCount("dirJ/fileJC", 1);
     verifyRequestCount("dirJ/fileJD", 1);
 
-    std::cout << "Copy installed children" << std::endl;
+    std::cout << "passed Copy installed children" << std::endl;
 }
 
 int main(int argc, char* argv[])
@@ -751,6 +779,7 @@ int main(int argc, char* argv[])
     global_repo->defineFile("dirC/subdirA/subsubA/fileCAAA");
 
     testBasicClone(&cl);
+	testUpdateNoChanges(&cl);
 
     testModifyLocalFiles(&cl);
 

simgear/io/test_untar.cxx

@@ -11,6 +11,7 @@
 
 #include <simgear/misc/test_macros.hxx>
 #include <simgear/misc/sg_path.hxx>
+#include <simgear/misc/sg_dir.hxx>
 #include <simgear/io/sg_file.hxx>
 
 
@@ -31,7 +32,7 @@ void testTarGz()
     uint8_t* buf = (uint8_t*) alloca(8192);
     size_t bufSize = f.read((char*) buf, 8192);
 
-    SG_VERIFY(TarExtractor::isTarData(buf, bufSize));
+    SG_VERIFY(ArchiveExtractor::determineType(buf, bufSize) == ArchiveExtractor::TarData);
 
     f.close();
 }
@@ -44,18 +45,146 @@ void testPlainTar()
     SGBinaryFile f(p);
     f.open(SG_IO_IN);
 
-    uint8_t* buf = (uint8_t*) alloca(8192);
-    size_t bufSize = f.read((char*) buf, 8192);
+	uint8_t* buf = (uint8_t*)alloca(8192);
+	size_t bufSize = f.read((char*) buf, 8192);
 
-    SG_VERIFY(TarExtractor::isTarData(buf, bufSize));
+	SG_VERIFY(ArchiveExtractor::determineType(buf, bufSize) == ArchiveExtractor::TarData);
 
     f.close();
 }
 
+void testExtractStreamed()
+{
+	SGPath p = SGPath(SRC_DIR);
+	p.append("test.tar.gz");
+
+	SGBinaryFile f(p);
+	f.open(SG_IO_IN);
+
+	SGPath extractDir = simgear::Dir::current().path() / "test_extract_streamed";
+	simgear::Dir pd(extractDir);
+	pd.removeChildren();
+
+	ArchiveExtractor ex(extractDir);
+
+	uint8_t* buf = (uint8_t*) alloca(128);
+	while (!f.eof()) {
+		size_t bufSize = f.read((char*) buf, 128);
+		ex.extractBytes(buf, bufSize);
+	}
+
+	ex.flush();
+	SG_VERIFY(ex.isAtEndOfArchive());
+	SG_VERIFY(ex.hasError() == false);
+
+	SG_VERIFY((extractDir / "testDir/hello.c").exists());
+	SG_VERIFY((extractDir / "testDir/foo.txt").exists());
+}
+
+void testExtractLocalFile()
+{
+
+}
+
+void testFilterTar()
+{
+    SGPath p = SGPath(SRC_DIR);
+    p.append("badTar.tgz");
+    
+    SGBinaryFile f(p);
+    f.open(SG_IO_IN);
+    
+    SGPath extractDir = simgear::Dir::current().path() / "test_filter_tar";
+    simgear::Dir pd(extractDir);
+    pd.removeChildren();
+    
+    ArchiveExtractor ex(extractDir);
+    
+    uint8_t* buf = (uint8_t*) alloca(128);
+    while (!f.eof()) {
+        size_t bufSize = f.read((char*) buf, 128);
+        ex.extractBytes(buf, bufSize);
+    }
+    
+    ex.flush();
+    SG_VERIFY(ex.isAtEndOfArchive());
+    SG_VERIFY(ex.hasError() == false);
+    
+    SG_VERIFY((extractDir / "tarWithBadContent/regular-file.txt").exists());
+    SG_VERIFY(!(extractDir / "tarWithBadContent/symbolic-linked.png").exists());
+    SG_VERIFY((extractDir / "tarWithBadContent/screenshot.png").exists());
+    SG_VERIFY((extractDir / "tarWithBadContent/dirOne/subDirA").exists());
+    SG_VERIFY(!(extractDir / "tarWithBadContent/dirOne/subDirA/linked.txt").exists());
+
+
+}
+
+void testExtractZip()
+{
+	SGPath p = SGPath(SRC_DIR);
+	p.append("zippy.zip");
+
+	SGBinaryFile f(p);
+	f.open(SG_IO_IN);
+
+	SGPath extractDir = simgear::Dir::current().path() / "test_extract_zip";
+	simgear::Dir pd(extractDir);
+	pd.removeChildren();
+
+	ArchiveExtractor ex(extractDir);
+
+	uint8_t* buf = (uint8_t*)alloca(128);
+	while (!f.eof()) {
+		size_t bufSize = f.read((char*)buf, 128);
+		ex.extractBytes(buf, bufSize);
+	}
+
+	ex.flush();
+	SG_VERIFY(ex.isAtEndOfArchive());
+	SG_VERIFY(ex.hasError() == false);
+
+	SG_VERIFY((extractDir / "zippy/dirA/hello.c").exists());
+	SG_VERIFY((extractDir / "zippy/bar.xml").exists());
+	SG_VERIFY((extractDir / "zippy/long-named.json").exists());
+}
+
+void testPAXAttributes()
+{
+    SGPath p = SGPath(SRC_DIR);
+    p.append("pax-extended.tar");
+    
+    SGBinaryFile f(p);
+    f.open(SG_IO_IN);
+    
+    SGPath extractDir = simgear::Dir::current().path() / "test_pax_extended";
+    simgear::Dir pd(extractDir);
+    pd.removeChildren();
+    
+    ArchiveExtractor ex(extractDir);
+    
+    uint8_t* buf = (uint8_t*) alloca(128);
+    while (!f.eof()) {
+        size_t bufSize = f.read((char*) buf, 128);
+        ex.extractBytes(buf, bufSize);
+    }
+    
+    ex.flush();
+    SG_VERIFY(ex.isAtEndOfArchive());
+    SG_VERIFY(ex.hasError() == false);
+
+}
+
 int main(int ac, char ** av)
 {
     testTarGz();
     testPlainTar();
-
+    testFilterTar();
+	testExtractStreamed();
+	testExtractZip();
+   
+    // disabled to avoiding checking in large PAX archive
+    // testPAXAttributes();
+    
+	std::cout << "all tests passed" << std::endl;
     return 0;
 }

simgear/io/untar.cxx

@@ -31,12 +31,80 @@
 #include <simgear/sg_inlines.h>
 #include <simgear/io/sg_file.hxx>
 #include <simgear/misc/sg_dir.hxx>
-
+#include <simgear/io/iostreams/sgstream.hxx>
 #include <simgear/debug/logstream.hxx>
+#include <simgear/package/unzip.h>
+#include <simgear/structure/exception.hxx>
 
 namespace simgear
 {
 
+	class ArchiveExtractorPrivate
+	{
+	public:
+		ArchiveExtractorPrivate(ArchiveExtractor* o) :
+			outer(o)
+		{
+			assert(outer);
+		}
+
+		typedef enum {
+			INVALID = 0,
+			READING_HEADER,
+			READING_FILE,
+			READING_PADDING,
+            READING_PAX_GLOBAL_ATTRIBUTES,
+            READING_PAX_FILE_ATTRIBUTES,
+			PRE_END_OF_ARCHVE,
+			END_OF_ARCHIVE,
+			ERROR_STATE, ///< states above this are error conditions
+			BAD_ARCHIVE,
+			BAD_DATA,
+			FILTER_STOPPED
+		} State;
+
+		State state = INVALID;
+		ArchiveExtractor* outer = nullptr;
+
+		virtual void extractBytes(const uint8_t* bytes, size_t count) = 0;
+
+		virtual void flush() = 0;
+
+		SGPath extractRootPath()
+		{
+			return outer->_rootPath;
+		}
+
+		ArchiveExtractor::PathResult filterPath(std::string& pathToExtract)
+		{
+			return outer->filterPath(pathToExtract);
+		}
+
+
+		bool isSafePath(const std::string& p) const
+		{
+			if (p.empty()) {
+				return false;
+			}
+
+			// reject absolute paths
+			if (p.at(0) == '/') {
+				return false;
+			}
+
+			// reject paths containing '..'
+			size_t doubleDot = p.find("..");
+			if (doubleDot != std::string::npos) {
+				return false;
+			}
+
+			// on POSIX could use realpath to sanity check
+			return true;
+		}
+	};
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
 const int ZLIB_DECOMPRESS_BUFFER_SIZE = 32 * 1024;
 const int ZLIB_INFLATE_WINDOW_BITS = MAX_WBITS;
 const int ZLIB_DECODE_GZIP_HEADER = 16;
@@ -81,24 +149,14 @@ typedef struct
 #define FIFOTYPE '6'            /* FIFO special */
 #define CONTTYPE '7'            /* reserved */
 
-class TarExtractorPrivate
+	const char PAX_GLOBAL_HEADER = 'g';
+	const char PAX_FILE_ATTRIBUTES = 'x';
+
+
+class TarExtractorPrivate : public ArchiveExtractorPrivate
 {
 public:
-    typedef enum {
-        INVALID = 0,
-        READING_HEADER,
-        READING_FILE,
-        READING_PADDING,
-        PRE_END_OF_ARCHVE,
-        END_OF_ARCHIVE,
-        ERROR_STATE, ///< states above this are error conditions
-        BAD_ARCHIVE,
-        BAD_DATA,
-        FILTER_STOPPED
-    } State;
-
-    SGPath path;
-    State state;
+    
     union {
         UstarHeaderBlock header;
         uint8_t headerBytes[TAR_HEADER_BLOCK_SIZE];
@@ -109,17 +167,22 @@ public:
     size_t currentFileSize;
     z_stream zlibStream;
     uint8_t* zlibOutput;
-    bool haveInitedZLib;
-    bool uncompressedData; // set if reading a plain .tar (not tar.gz)
+    bool haveInitedZLib = false;
+    bool uncompressedData = false; // set if reading a plain .tar (not tar.gz)
     uint8_t* headerPtr;
-    TarExtractor* outer;
     bool skipCurrentEntry = false;
+    std::string paxAttributes;
+    std::string paxPathName;
     
-    TarExtractorPrivate(TarExtractor* o) :
-        haveInitedZLib(false),
-        uncompressedData(false),
-        outer(o)
+    TarExtractorPrivate(ArchiveExtractor* o) :
+		ArchiveExtractorPrivate(o)
     {
+		memset(&zlibStream, 0, sizeof(z_stream));
+		zlibOutput = (unsigned char*)malloc(ZLIB_DECOMPRESS_BUFFER_SIZE);
+		zlibStream.zalloc = Z_NULL;
+		zlibStream.zfree = Z_NULL;
+		zlibStream.avail_out = ZLIB_DECOMPRESS_BUFFER_SIZE;
+		zlibStream.next_out = zlibOutput;
     }
 
     ~TarExtractorPrivate()
@@ -127,6 +190,17 @@ public:
         free(zlibOutput);
     }
 
+    void readPaddingIfRequired()
+    {
+        size_t pad = currentFileSize % TAR_HEADER_BLOCK_SIZE;
+        if (pad) {
+            bytesRemaining = TAR_HEADER_BLOCK_SIZE - pad;
+            setState(READING_PADDING);
+        } else {
+            setState(READING_HEADER);
+        }
+    }
+    
     void checkEndOfState()
     {
         if (bytesRemaining > 0) {
@@ -138,13 +212,7 @@ public:
                 currentFile->close();
                 currentFile.reset();
             }
-            size_t pad = currentFileSize % TAR_HEADER_BLOCK_SIZE;
-            if (pad) {
-                bytesRemaining = TAR_HEADER_BLOCK_SIZE - pad;
-                setState(READING_PADDING);
-            } else {
-                setState(READING_HEADER);
-            }
+            readPaddingIfRequired();
         } else if (state == READING_HEADER) {
             processHeader();
         } else if (state == PRE_END_OF_ARCHVE) {
@@ -153,6 +221,12 @@ public:
             } else {
                 // what does the spec say here?
             }
+        } else if (state == READING_PAX_GLOBAL_ATTRIBUTES) {
+            parsePAXAttributes(true);
+            readPaddingIfRequired();
+        } else if (state == READING_PAX_FILE_ATTRIBUTES) {
+            parsePAXAttributes(false);
+            readPaddingIfRequired();
         } else if (state == READING_PADDING) {
             setState(READING_HEADER);
         }
@@ -168,6 +242,77 @@ public:
         state = newState;
     }
 
+	void extractBytes(const uint8_t* bytes, size_t count) override
+	{
+		zlibStream.next_in = (uint8_t*) bytes;
+		zlibStream.avail_in = count;
+
+		if (!haveInitedZLib) {
+			// now we have data, see if we're dealing with GZ-compressed data or not
+			if ((bytes[0] == 0x1f) && (bytes[1] == 0x8b)) {
+				// GZIP identification bytes
+				if (inflateInit2(&zlibStream, ZLIB_INFLATE_WINDOW_BITS | ZLIB_DECODE_GZIP_HEADER) != Z_OK) {
+					SG_LOG(SG_IO, SG_WARN, "inflateInit2 failed");
+					state = TarExtractorPrivate::BAD_DATA;
+					return;
+				}
+			} else {
+				UstarHeaderBlock* header = (UstarHeaderBlock*)bytes;
+				if (strncmp(header->magic, TMAGIC, TMAGLEN) != 0) {
+					SG_LOG(SG_IO, SG_WARN, "didn't find tar magic in header");
+					state = TarExtractorPrivate::BAD_DATA;
+					return;
+				}
+
+				uncompressedData = true;
+			}
+
+			haveInitedZLib = true;
+			setState(TarExtractorPrivate::READING_HEADER);
+		} // of init on first-bytes case
+
+		if (uncompressedData) {
+			processBytes((const char*) bytes, count);
+		} else {
+			size_t writtenSize;
+			// loop, running zlib() inflate and sending output bytes to
+			// our request body handler. Keep calling inflate until no bytes are
+			// written, and ZLIB has consumed all available input
+			do {
+				zlibStream.next_out = zlibOutput;
+				zlibStream.avail_out = ZLIB_DECOMPRESS_BUFFER_SIZE;
+				int result = inflate(&zlibStream, Z_NO_FLUSH);
+				if (result == Z_OK || result == Z_STREAM_END) {
+					// nothing to do
+
+				}
+				else if (result == Z_BUF_ERROR) {
+					// transient error, fall through
+				}
+				else {
+					//  _error = result;
+					SG_LOG(SG_IO, SG_WARN, "Permanent ZLib error:" << zlibStream.msg);
+					state = TarExtractorPrivate::BAD_DATA;
+					return;
+				}
+
+				writtenSize = ZLIB_DECOMPRESS_BUFFER_SIZE - zlibStream.avail_out;
+				if (writtenSize > 0) {
+					processBytes((const char*) zlibOutput, writtenSize);
+				}
+
+				if (result == Z_STREAM_END) {
+					break;
+				}
+			} while ((zlibStream.avail_in > 0) || (writtenSize > 0));
+		} // of Zlib-compressed data
+	}
+
+	void flush() override
+	{
+		// no-op for tar files, we process everything greedily
+	}
+
     void processHeader()
     {
         if (headerIsAllZeros()) {
@@ -180,28 +325,32 @@ public:
         }
 
         if (strncmp(header.magic, TMAGIC, TMAGLEN) != 0) {
-            SG_LOG(SG_IO, SG_WARN, "magic is wrong");
+            SG_LOG(SG_IO, SG_WARN, "Untar: magic is wrong");
             state = BAD_ARCHIVE;
             return;
         }
 
         skipCurrentEntry = false;
         std::string tarPath = std::string(header.prefix) + std::string(header.fileName);
-
+        if (!paxPathName.empty()) {
+            tarPath = paxPathName;
+            paxPathName.clear(); // clear for next file
+        }
+        
         if (!isSafePath(tarPath)) {
             SG_LOG(SG_IO, SG_WARN, "bad tar path:" << tarPath);
             skipCurrentEntry = true;
         }
 
-        auto result = outer->filterPath(tarPath);
-        if (result == TarExtractor::Stop) {
+        auto result = filterPath(tarPath);
+        if (result == ArchiveExtractor::Stop) {
             setState(FILTER_STOPPED);
             return;
-        } else if (result == TarExtractor::Skipped) {
+        } else if (result == ArchiveExtractor::Skipped) {
             skipCurrentEntry = true;
         }
         
-        SGPath p = path / tarPath;
+        SGPath p = extractRootPath() / tarPath;
         if (header.typeflag == DIRTYPE) {
             if (!skipCurrentEntry) {
                 Dir dir(p);
@@ -216,6 +365,20 @@ public:
                 currentFile->open(SG_IO_OUT);
             }
             setState(READING_FILE);
+        } else if (header.typeflag == PAX_GLOBAL_HEADER) {
+            setState(READING_PAX_GLOBAL_ATTRIBUTES);
+            currentFileSize = ::strtol(header.size, NULL, 8);
+            bytesRemaining = currentFileSize;
+            paxAttributes.clear();
+        } else if (header.typeflag == PAX_FILE_ATTRIBUTES) {
+            setState(READING_PAX_FILE_ATTRIBUTES);
+            currentFileSize = ::strtol(header.size, NULL, 8);
+            bytesRemaining = currentFileSize;
+            paxAttributes.clear();
+        } else if ((header.typeflag == SYMTYPE) || (header.typeflag == LNKTYPE)) {
+            SG_LOG(SG_IO, SG_WARN, "Tarball contains a link or symlink, will be skipped:" << tarPath);
+            skipCurrentEntry = true;
+            setState(READING_HEADER);
         } else {
             SG_LOG(SG_IO, SG_WARN, "Unsupported tar file type:" << header.typeflag);
             state = BAD_ARCHIVE;
@@ -240,6 +403,9 @@ public:
             headerPtr += curBytes;
         } else if (state == READING_PADDING) {
             bytesRemaining -= curBytes;
+        } else if ((state == READING_PAX_FILE_ATTRIBUTES) || (state == READING_PAX_GLOBAL_ATTRIBUTES)) {
+            bytesRemaining -= curBytes;
+            paxAttributes.append(bytes, curBytes);
         }
 
         checkEndOfState();
@@ -261,132 +427,283 @@ public:
         return true;
     }
 
-    bool isSafePath(const std::string& p) const
+    // https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.3.0/com.ibm.zos.v2r3.bpxa500/paxex.htm#paxex
+    void parsePAXAttributes(bool areGlobal)
     {
-        if (p.empty()) {
-            return false;
+        auto lineStart = 0;
+        for (;;) {
+            auto firstSpace = paxAttributes.find(' ', lineStart);
+            auto firstEq = paxAttributes.find('=', lineStart);
+            if ((firstEq == std::string::npos) || (firstSpace == std::string::npos)) {
+                SG_LOG(SG_IO, SG_WARN, "Malfroemd PAX attributes in tarfile");
+                break;
+            }
+            
+            uint32_t lengthBytes = std::stoul(paxAttributes.substr(lineStart, firstSpace));
+            uint32_t dataBytes = lengthBytes - (firstEq + 1);
+            std::string name = paxAttributes.substr(firstSpace+1, firstEq - (firstSpace + 1));
+            
+            // dataBytes - 1 here to trim off the trailing newline
+            std::string data = paxAttributes.substr(firstEq+1, dataBytes - 1);
+            
+            processPAXAttribute(areGlobal, name, data);
+            
+            lineStart += lengthBytes;
         }
-
-        // reject absolute paths
-        if (p.at(0) == '/') {
-            return false;
+    }
+    
+    void processPAXAttribute(bool isGlobalAttr, const std::string& attrName, const std::string& data)
+    {
+        if (!isGlobalAttr && (attrName == "path")) {
+            // data is UTF-8 encoded path name
+            paxPathName = data;
         }
-
-        // reject paths containing '..'
-        size_t doubleDot = p.find("..");
-        if (doubleDot != std::string::npos) {
-            return false;
-        }
-
-        // on POSIX could use realpath to sanity check
-        return true;
     }
 };
 
-TarExtractor::TarExtractor(const SGPath& rootPath) :
-    d(new TarExtractorPrivate(this))
-{
+///////////////////////////////////////////////////////////////////////////////
 
-    d->path = rootPath;
-    d->state = TarExtractorPrivate::INVALID;
-
-    memset(&d->zlibStream, 0, sizeof(z_stream));
-    d->zlibOutput = (unsigned char*) malloc(ZLIB_DECOMPRESS_BUFFER_SIZE);
-    d->zlibStream.zalloc = Z_NULL;
-    d->zlibStream.zfree = Z_NULL;
-
-    d->zlibStream.avail_out = ZLIB_DECOMPRESS_BUFFER_SIZE;
-    d->zlibStream.next_out = d->zlibOutput;
+extern "C" {
+	void fill_memory_filefunc(zlib_filefunc_def*);
 }
 
-TarExtractor::~TarExtractor()
+class ZipExtractorPrivate : public ArchiveExtractorPrivate
+{
+public:
+	std::string m_buffer;
+
+	ZipExtractorPrivate(ArchiveExtractor* outer) :
+		ArchiveExtractorPrivate(outer)
+	{
+
+	}
+
+	~ZipExtractorPrivate()
+	{
+
+	}
+
+	void extractBytes(const uint8_t* bytes, size_t count) override
+	{
+		// becuase the .zip central directory is at the end of the file,
+		// we have no choice but to simply buffer bytes here until flush()
+		// is called
+		m_buffer.append((const char*) bytes, count);
+	}
+
+	void flush() override
+	{
+		zlib_filefunc_def memoryAccessFuncs;
+		fill_memory_filefunc(&memoryAccessFuncs);
+
+		char bufferName[128];
+#if defined(SG_WINDOWS)
+        ::snprintf(bufferName, 128, "%p+%llx", m_buffer.data(), m_buffer.size());
+#else
+		::snprintf(bufferName, 128, "%p+%lx", m_buffer.data(), m_buffer.size());
+#endif
+		unzFile zip = unzOpen2(bufferName, &memoryAccessFuncs);
+
+		const size_t BUFFER_SIZE = 32 * 1024;
+		void* buf = malloc(BUFFER_SIZE);
+
+		try {
+			int result = unzGoToFirstFile(zip);
+			if (result != UNZ_OK) {
+				throw sg_exception("failed to go to first file in archive");
+			}
+
+			while (true) {
+				extractCurrentFile(zip, (char*)buf, BUFFER_SIZE);
+				if (state == FILTER_STOPPED) {
+					break;
+				}
+
+				result = unzGoToNextFile(zip);
+				if (result == UNZ_END_OF_LIST_OF_FILE) {
+					break;
+				}
+				else if (result != UNZ_OK) {
+					throw sg_io_exception("failed to go to next file in the archive");
+				}
+			}
+			state = END_OF_ARCHIVE;
+		}
+		catch (sg_exception&) {
+			state = BAD_ARCHIVE;
+		}
+
+		free(buf);
+		unzClose(zip);
+	}
+
+	void extractCurrentFile(unzFile zip, char* buffer, size_t bufferSize)
+	{
+		unz_file_info fileInfo;
+		unzGetCurrentFileInfo(zip, &fileInfo,
+			buffer, bufferSize,
+			NULL, 0,  /* extra field */
+			NULL, 0 /* comment field */);
+
+		std::string name(buffer);
+		if (!isSafePath(name)) {
+			throw sg_format_exception("Bad zip path", name);
+		}
+
+		auto filterResult = filterPath(name);
+		if (filterResult == ArchiveExtractor::Stop) {
+			state = FILTER_STOPPED;
+			return;
+		}
+		else if (filterResult == ArchiveExtractor::Skipped) {
+			return;
+		}
+
+		if (fileInfo.uncompressed_size == 0) {
+			// assume it's a directory for now
+			// since we create parent directories when extracting
+			// a path, we're done here
+			return;
+		}
+
+		int result = unzOpenCurrentFile(zip);
+		if (result != UNZ_OK) {
+			throw sg_io_exception("opening current zip file failed", sg_location(name));
+		}
+
+		sg_ofstream outFile;
+		bool eof = false;
+		SGPath path = extractRootPath() / name;
+
+		// create enclosing directory heirarchy as required
+		Dir parentDir(path.dir());
+		if (!parentDir.exists()) {
+			bool ok = parentDir.create(0755);
+			if (!ok) {
+				throw sg_io_exception("failed to create directory heirarchy for extraction", path);
+			}
+		}
+
+		outFile.open(path, std::ios::binary | std::ios::trunc | std::ios::out);
+		if (outFile.fail()) {
+			throw sg_io_exception("failed to open output file for writing", path);
+		}
+
+		while (!eof) {
+			int bytes = unzReadCurrentFile(zip, buffer, bufferSize);
+			if (bytes < 0) {
+				throw sg_io_exception("unzip failure reading curent archive", sg_location(name));
+			}
+			else if (bytes == 0) {
+				eof = true;
+			}
+			else {
+				outFile.write(buffer, bytes);
+			}
+		}
+
+		outFile.close();
+		unzCloseCurrentFile(zip);
+	}
+};
+
+//////////////////////////////////////////////////////////////////////////////
+
+ArchiveExtractor::ArchiveExtractor(const SGPath& rootPath) :
+	_rootPath(rootPath)
+{
+}
+
+ArchiveExtractor::~ArchiveExtractor()
 {
 
 }
 
-void TarExtractor::extractBytes(const char* bytes, size_t count)
+void ArchiveExtractor::extractBytes(const uint8_t* bytes, size_t count)
 {
-    if (d->state >= TarExtractorPrivate::ERROR_STATE) {
+	if (!d) {
+		_prebuffer.append((char*) bytes, count);
+		auto r = determineType((uint8_t*) _prebuffer.data(), _prebuffer.size());
+		if (r == InsufficientData) {
+			return;
+		}
+
+		if (r == TarData) {
+			d.reset(new TarExtractorPrivate(this));
+		}
+		else if (r == ZipData) {
+			d.reset(new ZipExtractorPrivate(this));
+		}
+		else {
+			SG_LOG(SG_IO, SG_ALERT, "Invcalid archive type");
+			_invalidDataType = true;
+			return;
+		}
+
+		// if hit here, we created the extractor. Feed the prefbuffer
+		// bytes through it
+		d->extractBytes((uint8_t*) _prebuffer.data(), _prebuffer.size());
+		_prebuffer.clear();
+		return;
+	}
+
+    if (d->state >= ArchiveExtractorPrivate::ERROR_STATE) {
         return;
     }
 
-    d->zlibStream.next_in = (uint8_t*) bytes;
-    d->zlibStream.avail_in = count;
-
-    if (!d->haveInitedZLib) {
-        // now we have data, see if we're dealing with GZ-compressed data or not
-        uint8_t* ubytes = (uint8_t*) bytes;
-        if ((ubytes[0] == 0x1f) && (ubytes[1] == 0x8b)) {
-            // GZIP identification bytes
-            if (inflateInit2(&d->zlibStream, ZLIB_INFLATE_WINDOW_BITS | ZLIB_DECODE_GZIP_HEADER) != Z_OK) {
-                SG_LOG(SG_IO, SG_WARN, "inflateInit2 failed");
-                d->state = TarExtractorPrivate::BAD_DATA;
-                return;
-            }
-        } else {
-            UstarHeaderBlock* header = (UstarHeaderBlock*) bytes;
-            if (strncmp(header->magic, TMAGIC, TMAGLEN) != 0) {
-                SG_LOG(SG_IO, SG_WARN, "didn't find tar magic in header");
-                d->state = TarExtractorPrivate::BAD_DATA;
-                return;
-            }
-
-            d->uncompressedData = true;
-        }
-
-        d->haveInitedZLib = true;
-        d->setState(TarExtractorPrivate::READING_HEADER);
-    } // of init on first-bytes case
-
-    if (d->uncompressedData) {
-        d->processBytes(bytes, count);
-    } else {
-        size_t writtenSize;
-        // loop, running zlib() inflate and sending output bytes to
-        // our request body handler. Keep calling inflate until no bytes are
-        // written, and ZLIB has consumed all available input
-        do {
-            d->zlibStream.next_out = d->zlibOutput;
-            d->zlibStream.avail_out = ZLIB_DECOMPRESS_BUFFER_SIZE;
-            int result = inflate(&d->zlibStream, Z_NO_FLUSH);
-            if (result == Z_OK || result == Z_STREAM_END) {
-                // nothing to do
-
-            } else if (result == Z_BUF_ERROR) {
-                // transient error, fall through
-            } else {
-                //  _error = result;
-                SG_LOG(SG_IO, SG_WARN, "Permanent ZLib error:" << d->zlibStream.msg);
-                d->state = TarExtractorPrivate::BAD_DATA;
-                return;
-            }
-
-            writtenSize = ZLIB_DECOMPRESS_BUFFER_SIZE - d->zlibStream.avail_out;
-            if (writtenSize > 0) {
-                d->processBytes((const char*) d->zlibOutput, writtenSize);
-            }
-
-            if (result == Z_STREAM_END) {
-                break;
-            }
-        } while ((d->zlibStream.avail_in > 0) || (writtenSize > 0));
-    } // of Zlib-compressed data
+	d->extractBytes(bytes, count);
 }
 
-bool TarExtractor::isAtEndOfArchive() const
+void ArchiveExtractor::flush()
 {
-    return (d->state == TarExtractorPrivate::END_OF_ARCHIVE);
+	if (!d)
+		return;
+
+	d->flush();
 }
 
-bool TarExtractor::hasError() const
+bool ArchiveExtractor::isAtEndOfArchive() const
 {
-    return (d->state >= TarExtractorPrivate::ERROR_STATE);
+	if (!d)
+		return false;
+
+    return (d->state == ArchiveExtractorPrivate::END_OF_ARCHIVE);
 }
 
-bool TarExtractor::isTarData(const uint8_t* bytes, size_t count)
+bool ArchiveExtractor::hasError() const
+{
+	if (_invalidDataType)
+		return true;
+
+	if (!d)
+		return false;
+
+    return (d->state >= ArchiveExtractorPrivate::ERROR_STATE);
+}
+
+ArchiveExtractor::DetermineResult ArchiveExtractor::determineType(const uint8_t* bytes, size_t count)
+{
+	// check for ZIP
+	if (count < 4) {
+		return InsufficientData;
+	}
+
+	if (memcmp(bytes, "PK\x03\x04", 4) == 0) {
+		return ZipData;
+	}
+
+	auto r = isTarData(bytes, count);
+	if ((r == TarData) || (r == InsufficientData))
+		return r;
+
+	return Invalid;
+}
+
+
+ArchiveExtractor::DetermineResult ArchiveExtractor::isTarData(const uint8_t* bytes, size_t count)
 {
     if (count < 2) {
-        return false;
+        return InsufficientData;
     }
 
     UstarHeaderBlock* header = 0;
@@ -404,21 +721,20 @@ bool TarExtractor::isTarData(const uint8_t* bytes, size_t count)
 
         if (inflateInit2(&z, ZLIB_INFLATE_WINDOW_BITS | ZLIB_DECODE_GZIP_HEADER) != Z_OK) {
             inflateEnd(&z);
-            return false;
+            return Invalid;
         }
 
         int result = inflate(&z, Z_SYNC_FLUSH);
         if (result != Z_OK) {
             SG_LOG(SG_IO, SG_WARN, "inflate failed:" << result);
             inflateEnd(&z);
-            return false; // not tar data
+            return Invalid; // not tar data
         }
 
         size_t written = 4096 - z.avail_out;
         if (written < TAR_HEADER_BLOCK_SIZE) {
-            SG_LOG(SG_IO, SG_WARN, "insufficient data for header");
             inflateEnd(&z);
-            return false;
+            return InsufficientData;
         }
 
         header = reinterpret_cast<UstarHeaderBlock*>(zlibOutput);
@@ -426,22 +742,25 @@ bool TarExtractor::isTarData(const uint8_t* bytes, size_t count)
     } else {
         // uncompressed tar
         if (count < TAR_HEADER_BLOCK_SIZE) {
-            SG_LOG(SG_IO, SG_WARN, "insufficient data for header");
-            return false;
+            return InsufficientData;
         }
 
         header = (UstarHeaderBlock*) bytes;
     }
 
     if (strncmp(header->magic, TMAGIC, TMAGLEN) != 0) {
-        SG_LOG(SG_IO, SG_WARN, "not a tar file");
-        return false;
+        return Invalid;
     }
 
-    return true;
+    return TarData;
 }
 
-auto TarExtractor::filterPath(std::string& pathToExtract)
+void ArchiveExtractor::extractLocalFile(const SGPath& archiveFile)
+{
+
+}
+
+auto ArchiveExtractor::filterPath(std::string& pathToExtract)
   -> PathResult
 {
     SG_UNUSED(pathToExtract);

simgear/io/untar.hxx

@@ -21,40 +21,68 @@
 #include <memory>
 
 #include <cstdlib>
-#include <stdint.h> // for uint8_t
+#include <cstdint>
+
 #include <simgear/misc/sg_path.hxx>
 
 namespace simgear
 {
 
-class TarExtractorPrivate;
+class ArchiveExtractorPrivate;
 
-class TarExtractor
+class ArchiveExtractor
 {
 public:
-    TarExtractor(const SGPath& rootPath);
-	~TarExtractor();
+	ArchiveExtractor(const SGPath& rootPath);
+	~ArchiveExtractor();
 
-    static bool isTarData(const uint8_t* bytes, size_t count);
+	enum DetermineResult
+	{
+		Invalid,
+		InsufficientData,
+		TarData,
+		ZipData
+	};
 
-    void extractBytes(const char* bytes, size_t count);
+	static DetermineResult determineType(const uint8_t* bytes, size_t count);
+
+	/**
+	 * @brief API to extract a local zip or tar.gz 
+	 */
+	void extractLocalFile(const SGPath& archiveFile);
+
+	/**
+	 * @brief API to extract from memory - this can be called multiple
+	 * times for streamking from a network socket etc
+	 */
+    void extractBytes(const uint8_t* bytes, size_t count);
+
+	void flush();
 
     bool isAtEndOfArchive() const;
 
     bool hasError() const;
 
+	enum PathResult {
+		Accepted,
+		Skipped,
+		Modified,
+		Stop
+	};
+
 protected:
-    enum PathResult {
-      Accepted,
-      Skipped,
-      Modified,
-      Stop
-    };
+
 
     virtual PathResult filterPath(std::string& pathToExtract);
 private:
-    friend class TarExtractorPrivate;
-    std::unique_ptr<TarExtractorPrivate> d;
+	static DetermineResult isTarData(const uint8_t* bytes, size_t count);
+
+    friend class ArchiveExtractorPrivate;
+    std::unique_ptr<ArchiveExtractorPrivate> d;
+
+	SGPath _rootPath;
+	std::string _prebuffer; // store bytes before type is determined
+	bool _invalidDataType = false;
 };
 
 } // of namespace simgear

simgear/math/SGMatrix.hxx

@@ -142,10 +142,10 @@ public:
   T (&sg(void))[4][4]
   { return _data.ptr(); }
   /// Readonly raw storage interface
-  const simd4x4_t<T,4> (&simd4x4(void) const)
+  const simd4x4_t<T,4> &simd4x4(void) const
   { return _data; }
   /// Readonly raw storage interface
-  simd4x4_t<T,4> (&simd4x4(void))
+  simd4x4_t<T,4> &simd4x4(void)
   { return _data; }
 
 

simgear/math/SGVec2.hxx

@@ -87,10 +87,10 @@ public:
   /// Access raw data
   T (&data(void))[2]
   { return _data.ptr(); }
-  const simd4_t<T,2> (&simd2(void) const)
+  const simd4_t<T,2> &simd2(void) const
   { return _data; }
   /// Readonly raw storage interface
-  simd4_t<T,2> (&simd2(void))
+  simd4_t<T,2> &simd2(void)
   { return _data; }
 
   /// Inplace addition

simgear/math/SGVec3.hxx

@@ -106,10 +106,10 @@ public:
   T (&data(void))[3]
   { return _data.ptr(); }
   /// Readonly raw storage interface
-  const simd4_t<T,3> (&simd3(void) const)
+  const simd4_t<T,3> &simd3(void) const
   { return _data; }
   /// Readonly raw storage interface
-  simd4_t<T,3> (&simd3(void))
+  simd4_t<T,3> &simd3(void)
   { return _data; }
 
   /// Inplace addition

simgear/math/SGVec4.hxx

@@ -99,10 +99,10 @@ public:
   T (&data(void))[4]
   { return _data.ptr(); }
   /// Readonly raw storage interface
-  const simd4_t<T,4> (&simd4(void) const)
+  const simd4_t<T,4> &simd4(void) const
   { return _data; }
   /// Readonly raw storage interface
-  simd4_t<T,4> (&simd4(void))
+  simd4_t<T,4> &simd4(void)
   { return _data; }
 
   /// Inplace addition

simgear/math/simd.hxx

@@ -309,7 +309,7 @@ inline simd4_t<T,N> operator*(simd4_t<T,N> v, T f) {
     return v;
 }
 
-#ifdef ENABLE_SIMD
+#ifdef ENABLE_SIMD_CODE
 
 # ifdef __SSE__
 namespace simd4
@@ -352,10 +352,10 @@ public:
     simd4_t(const simd4_t<float,2>& v) { simd4 = v.v4(); }
     simd4_t(const __m128& v) { simd4 = v; }
 
-    inline const __m128 (&v4(void) const) {
+    inline const __m128 &v4(void) const {
         return simd4;
     }
-    inline __m128 (&v4(void)) {
+    inline __m128 &v4(void) {
         return simd4;
     }
 
@@ -1120,11 +1120,11 @@ public:
     simd4_t(const simd4_t<int,2>& v) { simd4 = v.v4(); }
     simd4_t(const __m128i& v) { simd4 = v; }
 
-    inline __m128i (&v4(void)) {
+    inline __m128i &v4(void) {
         return simd4;
     }
 
-    inline const __m128i (&v4(void) const) {
+    inline const __m128i &v4(void) const {
         return simd4;
     }
 
@@ -1305,7 +1305,7 @@ inline simd4_t<int,N> max(simd4_t<int,N> v1, const simd4_t<int,N>& v2) {
 
 # endif
 
-#endif /* ENABLE_SIMD */
+#endif /* ENABLE_SIMD_CODE */
 
 #endif /* __SIMD_H__ */
 

simgear/math/simd4x4.hxx

@@ -289,7 +289,7 @@ inline simd4x4_t<T,N> operator*(const simd4x4_t<T,N>& m1, const simd4x4_t<T,N>&
 }
 
 
-#ifdef ENABLE_SIMD
+#ifdef ENABLE_SIMD_CODE
 
 # ifdef __SSE__
 template<>
@@ -1191,7 +1191,7 @@ inline simd4_t<int,3> transform<int>(const simd4x4_t<int,4>& m, const simd4_t<in
 } /* namespace simd4x */
 # endif
 
-#endif /* ENABLE_SIMD */
+#endif /* ENABLE_SIMD_CODE */
 
 #endif /* __SIMD4X4_H__ */
 

simgear/misc/ResourceManager.cxx

@@ -17,14 +17,23 @@
 //
 // $Id$
 
+#include <assert.h>
+#include <algorithm>
+
 #include <simgear_config.h>
 
 #include <simgear/misc/ResourceManager.hxx>
+#include <simgear/debug/logstream.hxx>
 
 namespace simgear
 {
     
-static ResourceManager* static_manager = NULL;
+static ResourceManager* static_manager = nullptr;
+
+ResourceProvider::~ResourceProvider()
+{
+    // pin to this compilation unit
+}
 
 ResourceManager::ResourceManager()
 {
@@ -40,13 +49,20 @@ ResourceManager* ResourceManager::instance()
     return static_manager;
 }    
 
+ResourceManager::~ResourceManager()
+{
+    assert(this == static_manager);
+    static_manager = nullptr;
+    std::for_each(_providers.begin(), _providers.end(), 
+        [](ResourceProvider* p) { delete p; });
+}
 /**
  * trivial provider using a fixed base path
  */
 class BasePathProvider : public ResourceProvider
 {
 public:
-    BasePathProvider(const SGPath& aBase, int aPriority) :
+    BasePathProvider(const SGPath& aBase, ResourceManager::Priority aPriority) :
         ResourceProvider(aPriority),
         _base(aBase)
     {
@@ -69,6 +85,8 @@ void ResourceManager::addBasePath(const SGPath& aPath, Priority aPriority)
 
 void ResourceManager::addProvider(ResourceProvider* aProvider)
 {
+    assert(aProvider);
+
     ProviderVec::iterator it = _providers.begin();
     for (; it != _providers.end(); ++it) {
       if (aProvider->priority() > (*it)->priority()) {
@@ -81,6 +99,16 @@ void ResourceManager::addProvider(ResourceProvider* aProvider)
     _providers.push_back(aProvider);
 }
 
+void ResourceManager::removeProvider(ResourceProvider* aProvider)
+{
+    assert(aProvider);
+    auto it = std::find(_providers.begin(), _providers.end(), aProvider);
+    if (it == _providers.end()) {
+        SG_LOG(SG_GENERAL, SG_DEV_ALERT, "unknown provider doing remove");
+        return;
+    }
+}
+
 SGPath ResourceManager::findPath(const std::string& aResource, SGPath aContext)
 {
     if (!aContext.isNull()) {
@@ -90,9 +118,8 @@ SGPath ResourceManager::findPath(const std::string& aResource, SGPath aContext)
         }
     }
     
-    ProviderVec::iterator it = _providers.begin();
-    for (; it != _providers.end(); ++it) {
-      SGPath path = (*it)->resolve(aResource, aContext);
+    for (auto provider : _providers) {
+      SGPath path = provider->resolve(aResource, aContext);
       if (!path.isNull()) {
         return path;
       }

simgear/misc/ResourceManager.hxx

@@ -36,6 +36,8 @@ class ResourceProvider;
 class ResourceManager
 {
 public:
+    ~ResourceManager();
+
     typedef enum {
       PRIORITY_DEFAULT = 0,
       PRIORITY_FALLBACK = -100,
@@ -55,6 +57,8 @@ public:
      */
     void addProvider(ResourceProvider* aProvider);
     
+    void removeProvider(ResourceProvider* aProvider);
+
     /**
      * given a resource name (or path), find the appropriate real resource
      * path.
@@ -75,17 +79,19 @@ class ResourceProvider
 public:
     virtual SGPath resolve(const std::string& aResource, SGPath& aContext) const = 0;
     
-    virtual int priority() const
+    virtual ~ResourceProvider();
+
+    virtual ResourceManager::Priority priority() const
     {
       return _priority;
     }
     
 protected:
-    ResourceProvider(int aPriority) :
+    ResourceProvider(ResourceManager::Priority aPriority) :
       _priority(aPriority)
     {}
     
-    int _priority;
+    ResourceManager::Priority _priority = ResourceManager::PRIORITY_DEFAULT;
 };
     
 } // of simgear namespace

simgear/misc/path_test.cxx

@@ -160,7 +160,7 @@ void test_path_dir()
 	SG_VERIFY(sub2.isFile());
 	SG_CHECK_EQUAL(sub2.sizeInBytes(), 250);
 
-    SGPath sub3 = p / "subß" / "file𝕽";
+    SGPath sub3 = p / "subß" / u8"file𝕽";
     sub3.create_dir(0755);
 
     {
@@ -174,7 +174,7 @@ void test_path_dir()
     sub3.set_cached(false);
     SG_VERIFY(sub3.exists());
     SG_CHECK_EQUAL(sub3.sizeInBytes(), 100);
-    SG_CHECK_EQUAL(sub3.file(), "file𝕽");
+    SG_CHECK_EQUAL(sub3.file(), u8"file𝕽");
 
 	simgear::Dir subD(p / "subA");
 	simgear::PathList dirChildren = subD.children(simgear::Dir::TYPE_DIR | simgear::Dir::NO_DOT_OR_DOTDOT);
@@ -188,7 +188,7 @@ void test_path_dir()
     simgear::Dir subS(sub3.dirPath());
     fileChildren = subS.children(simgear::Dir::TYPE_FILE | simgear::Dir::NO_DOT_OR_DOTDOT);
     SG_CHECK_EQUAL(fileChildren.size(), 1);
-    SG_CHECK_EQUAL(fileChildren[0], subS.path() / "file𝕽");
+    SG_CHECK_EQUAL(fileChildren[0], subS.path() / u8"file𝕽");
 
 }
 

simgear/misc/sg_path.cxx

@@ -107,7 +107,7 @@ static SGPath pathForKnownFolder(REFKNOWNFOLDERID folderId, const SGPath& def)
             // system call will allocate dynamic memory... which we must release when done
             wchar_t* localFolder = 0;
 
-            if (pSHGetKnownFolderPath(folderId, KF_FLAG_DEFAULT_PATH, NULL, &localFolder) == S_OK) {
+			if (pSHGetKnownFolderPath(folderId, KF_FLAG_DONT_VERIFY, NULL, &localFolder) == S_OK) {
                 SGPath folder_path = SGPath(localFolder, def.getPermissionChecker());
                 // release dynamic memory
                 CoTaskMemFree(static_cast<void*>(localFolder));

simgear/misc/stdint.hxx

@@ -12,65 +12,50 @@
 // $Id$
 
 
-//////////////////////////////////////////////////////////////////////
-//
-//  There are many sick systems out there:
-//
-//  check for sizeof(float) and sizeof(double)
-//  if sizeof(float) != 4 this code must be patched
-//  if sizeof(double) != 8 this code must be patched
-//
-//  Those are comments I fetched out of glibc source:
-//  - s390 is big-endian
-//  - Sparc is big-endian, but v9 supports endian conversion
-//    on loads/stores and GCC supports such a mode.  Be prepared.
-//  - The MIPS architecture has selectable endianness.
-//  - x86_64 is little-endian.
-//  - CRIS is little-endian.
-//  - m68k is big-endian.
-//  - Alpha is little-endian.
-//  - PowerPC can be little or big endian.
-//  - SH is bi-endian but with a big-endian FPU.
-//  - hppa1.1 big-endian.
-//  - ARM is (usually) little-endian but with a big-endian FPU.
-//
-//////////////////////////////////////////////////////////////////////
+#include <cstdint>
+#include <cstdlib> // for _byteswap_foo on Win32
 
-
-#ifdef _MSC_VER
-typedef signed char      int8_t;
-typedef signed short     int16_t;
-typedef signed int       int32_t;
-typedef signed __int64   int64_t;
-typedef unsigned char    uint8_t;
-typedef unsigned short   uint16_t;
-typedef unsigned int     uint32_t;
-typedef unsigned __int64 uint64_t;
-
-typedef int ssize_t;
-#elif defined(sgi) || defined(__sun)
-# include <sys/types.h>
-#else
-# include <stdint.h>
+#if defined(_MSC_VER)
+using ssize_t = int64_t; // this is a POSIX type, not a C one
 #endif
 
-
 inline uint16_t sg_bswap_16(uint16_t x) {
+#if defined(__llvm__) || \
+ (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) && !defined(__ICC)
+    return __builtin_bswap16(x);
+#elif defined(_MSC_VER) && !defined(_DEBUG)
+   return _byteswap_ushort(x);
+#else
     x = (x >> 8) | (x << 8);
     return x;
+#endif
 }
 
 inline uint32_t sg_bswap_32(uint32_t x) {
+#if defined(__llvm__) || \
+ (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)) && !defined(__ICC)
+    return __builtin_bswap32(x);
+#elif defined(_MSC_VER) && !defined(_DEBUG)
+   return _byteswap_ulong(x);
+#else
     x = ((x >>  8) & 0x00FF00FFL) | ((x <<  8) & 0xFF00FF00L);
     x = (x >> 16) | (x << 16);
     return x;
+#endif
 }
 
 inline uint64_t sg_bswap_64(uint64_t x) {
+#if defined(__llvm__) || \
+ (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)) && !defined(__ICC)
+   return __builtin_bswap64(x);
+#elif defined(_MSC_VER) && !defined(_DEBUG)
+   return _byteswap_uint64(x);
+#else
     x = ((x >>  8) & 0x00FF00FF00FF00FFLL) | ((x <<  8) & 0xFF00FF00FF00FF00LL);
     x = ((x >> 16) & 0x0000FFFF0000FFFFLL) | ((x << 16) & 0xFFFF0000FFFF0000LL);
     x = (x >> 32) | (x << 32);
     return x;
+#endif
 }
 
 

simgear/misc/strutils.cxx

@@ -37,7 +37,7 @@
 #include <simgear/package/md5.h>
 #include <simgear/compiler.h>   // SG_WINDOWS
 #include <simgear/structure/exception.hxx>
-
+#include <simgear/math/SGGeod.hxx>
 
 #if defined(SG_WINDOWS)
 	#include <windows.h>
@@ -1133,6 +1133,306 @@ bool matchPropPathToTemplate(const std::string& path, const std::string& templat
 
     // unreachable
 }
+ 
+bool parseStringAsLatLonValue(const std::string& s, double& degrees)
+{
+    try {
+            string ss = simplify(s);
+            auto spacePos = ss.find_first_of(" *");
+        
+            if (spacePos == std::string::npos) {
+                degrees = std::stod(ss);
+            } else {
+                degrees = std::stod(ss.substr(0, spacePos));
+                
+                double minutes = 0.0, seconds = 0.0;
+                
+                // check for minutes marker
+                auto quotePos = ss.find('\'');
+                if (quotePos == std::string::npos) {
+                    const auto minutesStr = ss.substr(spacePos+1);
+                    if (!minutesStr.empty()) {
+                        minutes = std::stod(minutesStr);
+                    }
+                } else {
+                    minutes = std::stod(ss.substr(spacePos+1, quotePos - spacePos));
+                    const auto secondsStr = ss.substr(quotePos+1);
+                    if (!secondsStr.empty()) {
+                        seconds = std::stod(secondsStr);
+                    }
+                }
+                
+                if ((seconds < 0.0) || (minutes < 0.0)) {
+                    // don't allow sign information in minutes or seconds
+                    return false;
+                }
+                
+                double offset = (minutes / 60.0) + (seconds / 3600.0);
+                degrees += (degrees >= 0.0) ? offset : -offset;
+            }
+        
+            // since we simplified, any trailing N/S/E/W must be the last char
+            const char lastChar = ::toupper(ss.back());
+            if ((lastChar == 'W') || (lastChar == 'S')) {
+                degrees = -degrees;
+            }
+    } catch (std::exception&) {
+        // std::stdo can throw
+        return false;
+    }
+    
+    return true;
+}
+        
+namespace {
+    bool isLatString(const std::string &s)
+    {
+        const char lastChar = ::toupper(s.back());
+        return (lastChar == 'N') || (lastChar == 'S');
+    }
+    
+    bool isLonString(const std::string &s)
+    {
+        const char lastChar = ::toupper(s.back());
+        return (lastChar == 'E') || (lastChar == 'W');
+    }
+} // of anonymous namespace
+        
+bool parseStringAsGeod(const std::string& s, SGGeod* result, bool assumeLonLatOrder)
+{
+    if (s.empty())
+        return false;
+    
+    const auto commaPos = s.find(',');
+    if (commaPos == string::npos) {
+        return false;
+    }
+    
+    auto termA = simplify(s.substr(0, commaPos)),
+        termB = simplify(s.substr(commaPos+1));
+    double valueA, valueB;
+    if (!parseStringAsLatLonValue(termA, valueA) || !parseStringAsLatLonValue(termB, valueB)) {
+        return false;
+    }
+    
+    if (result) {
+        // explicit ordering
+        if (isLatString(termA) && isLonString(termB)) {
+            *result = SGGeod::fromDeg(valueB, valueA);
+        } else if (isLonString(termA) && isLatString(termB)) {
+            *result = SGGeod::fromDeg(valueA, valueB);
+        } else {
+            // implicit ordering
+            // SGGeod wants longitude, latitude
+            *result = assumeLonLatOrder ? SGGeod::fromDeg(valueA, valueB)
+                                        : SGGeod::fromDeg(valueB, valueA);
+        }
+    }
+    
+    return true;
+}
+        
+namespace {
+    const char* static_degreeSymbols[] = {
+        "*",
+        " ",
+        "\xB0",     // Latin-1 B0 codepoint
+        "\xC2\xB0"  // UTF-8 equivalent
+    };
+} // of anonymous namespace
+        
+std::string formatLatLonValueAsString(double deg, LatLonFormat format,
+                                      char c,
+                                      DegreeSymbol degreeSymbol)
+{
+    double min, sec;
+    const int sign = deg < 0.0 ? -1 : 1;
+    deg = fabs(deg);
+    char buf[128];
+    const char* degSym = static_degreeSymbols[static_cast<int>(degreeSymbol)];
+    
+    switch (format) {
+    case LatLonFormat::DECIMAL_DEGREES:
+        ::snprintf(buf, sizeof(buf), "%3.6f%c", deg, c);
+        break;
+        
+    case LatLonFormat::DEGREES_MINUTES:
+        // d mm.mmm' (DMM format) -- uses a round-off factor tailored to the
+        // required precision of the minutes field (three decimal places),
+        // preventing minute values of 60.
+        min = (deg - int(deg)) * 60.0;
+        if (min >= 59.9995) {
+            min -= 60.0;
+            deg += 1.0;
+        }
+        snprintf(buf, sizeof(buf), "%d%s%06.3f'%c", int(deg), degSym, fabs(min), c);
+        break;
+        
+    case LatLonFormat::DEGREES_MINUTES_SECONDS:
+        // d mm'ss.s" (DMS format) -- uses a round-off factor tailored to the
+        // required precision of the seconds field (one decimal place),
+        // preventing second values of 60.
+        min = (deg - int(deg)) * 60.0;
+        sec = (min - int(min)) * 60.0;
+        if (sec >= 59.95) {
+            sec -= 60.0;
+            min += 1.0;
+            if (min >= 60.0) {
+                min -= 60.0;
+                deg += 1.0;
+            }
+        }
+        ::snprintf(buf, sizeof(buf), "%d%s%02d'%04.1f\"%c", int(deg), degSym,
+                   int(min), fabs(sec), c);
+        break;
+            
+    case LatLonFormat::SIGNED_DECIMAL_DEGREES:
+        // d.dddddd' (signed DDD format).
+        ::snprintf(buf, sizeof(buf), "%3.6f", sign*deg);
+        break;
+    
+    case LatLonFormat::SIGNED_DEGREES_MINUTES:
+        // d mm.mmm' (signed DMM format).
+        min = (deg - int(deg)) * 60.0;
+        if (min >= 59.9995) {
+            min -= 60.0;
+            deg += 1.0;
+        }
+        if (sign == 1) {
+            snprintf(buf, sizeof(buf), "%d%s%06.3f'", int(deg), degSym, fabs(min));
+        } else {
+            snprintf(buf, sizeof(buf), "-%d%s%06.3f'", int(deg), degSym, fabs(min));
+        }
+        break;
+        
+            
+    case LatLonFormat::SIGNED_DEGREES_MINUTES_SECONDS:
+        // d mm'ss.s" (signed DMS format).
+        min = (deg - int(deg)) * 60.0;
+        sec = (min - int(min)) * 60.0;
+        if (sec >= 59.95) {
+            sec -= 60.0;
+            min += 1.0;
+            if (min >= 60.0) {
+                min -= 60.0;
+                deg += 1.0;
+            }
+        }
+        if (sign == 1) {
+            snprintf(buf, sizeof(buf), "%d%s%02d'%04.1f\"", int(deg), degSym, int(min), fabs(sec));
+        } else {
+            snprintf(buf, sizeof(buf), "-%d%s%02d'%04.1f\"", int(deg), degSym, int(min), fabs(sec));
+        }
+        break;
+            
+    case LatLonFormat::ZERO_PAD_DECIMAL_DEGRESS:
+        // dd.dddddd X, ddd.dddddd X (zero padded DDD format).
+        if (c == 'N' || c == 'S') {
+            snprintf(buf, sizeof(buf), "%09.6f%c", deg, c);
+        } else {
+            snprintf(buf, sizeof(buf), "%010.6f%c", deg, c);
+        }
+        break;
+            
+    case LatLonFormat::ZERO_PAD_DEGREES_MINUTES:
+        // dd mm.mmm' X, ddd mm.mmm' X (zero padded DMM format).
+        min = (deg - int(deg)) * 60.0;
+        if (min >= 59.9995) {
+            min -= 60.0;
+            deg += 1.0;
+        }
+        if (c == 'N' || c == 'S') {
+            snprintf(buf, sizeof(buf), "%02d%s%06.3f'%c", int(deg), degSym, fabs(min), c);
+        } else {
+            snprintf(buf, sizeof(buf), "%03d%s%06.3f'%c", int(deg), degSym, fabs(min), c);
+        }
+        break;
+            
+    case LatLonFormat::ZERO_PAD_DEGREES_MINUTES_SECONDS:
+        // dd mm'ss.s" X, dd mm'ss.s" X (zero padded DMS format).
+        min = (deg - int(deg)) * 60.0;
+        sec = (min - int(min)) * 60.0;
+        if (sec >= 59.95) {
+            sec -= 60.0;
+            min += 1.0;
+            if (min >= 60.0) {
+                min -= 60.0;
+                deg += 1.0;
+            }
+        }
+        if (c == 'N' || c == 'S') {
+            snprintf(buf, sizeof(buf), "%02d%s%02d'%04.1f\"%c", int(deg), degSym, int(min), fabs(sec), c);
+        } else {
+            snprintf(buf, sizeof(buf), "%03d%s%02d'%04.1f\"%c", int(deg), degSym, int(min), fabs(sec), c);
+        }
+        break;
+            
+    case LatLonFormat::TRINITY_HOUSE:
+        // dd* mm'.mmm X, ddd* mm'.mmm X (Trinity House Navigation standard).
+        min = (deg - int(deg)) * 60.0;
+        if (min >= 59.9995) {
+            min -= 60.0;
+            deg += 1.0;
+        }
+        if (c == 'N' || c == 'S') {
+            snprintf(buf, sizeof(buf), "%02d* %02d'.%03d%c", int(deg), int(min), int(SGMisc<double>::round((min-int(min))*1000)), c);
+        } else {
+            snprintf(buf, sizeof(buf), "%03d* %02d'.%03d%c", int(deg), int(min), int(SGMisc<double>::round((min-int(min))*1000)), c);
+        }
+        break;
+            
+    case LatLonFormat::DECIMAL_DEGREES_SYMBOL:
+        ::snprintf(buf, sizeof(buf), "%3.6f%s%c", deg, degSym, c);
+        break;
+            
+    case LatLonFormat::ICAO_ROUTE_DEGREES:
+    {
+        min = (deg - int(deg)) * 60.0;
+        if (min >= 59.9995) {
+            min -= 60.0;
+            deg += 1.0;
+        }
+        
+        if (static_cast<int>(min) == 0) {
+            // 7-digit mode
+            if (c == 'N' || c == 'S') {
+                snprintf(buf, sizeof(buf), "%02d%c", int(deg), c);
+            } else {
+                snprintf(buf, sizeof(buf), "%03d%c", int(deg), c);
+            }
+        } else {
+            // 11-digit mode
+            if (c == 'N' || c == 'S') {
+                snprintf(buf, sizeof(buf), "%02d%02d%c", int(deg), int(min), c);
+            } else {
+                snprintf(buf, sizeof(buf), "%03d%02d%c", int(deg), int(min), c);
+            }
+        }
+        break;
+    }
+            
+    default:
+        break;
+    }
+ 
+    return std::string(buf);
+}
+
+std::string formatGeodAsString(const SGGeod& geod, LatLonFormat format,
+                               DegreeSymbol degreeSymbol)
+{
+    const char ns = (geod.getLatitudeDeg() > 0.0) ? 'N' : 'S';
+    const char ew = (geod.getLongitudeDeg() > 0.0) ? 'E' : 'W';
+    
+    // no comma seperator
+    if (format == LatLonFormat::ICAO_ROUTE_DEGREES) {
+        return formatLatLonValueAsString(geod.getLatitudeDeg(), format, ns, degreeSymbol) +
+            formatLatLonValueAsString(geod.getLongitudeDeg(), format, ew, degreeSymbol);
+    }
+    
+    return formatLatLonValueAsString(geod.getLatitudeDeg(), format, ns, degreeSymbol) + ","
+        + formatLatLonValueAsString(geod.getLongitudeDeg(), format, ew, degreeSymbol);
+}
 
 } // end namespace strutils
 

simgear/misc/strutils.hxx

@@ -36,6 +36,9 @@
 
 typedef std::vector < std::string > string_list;
 
+// forward decls
+class SGGeod;
+
 namespace simgear {
   namespace strutils {
 
@@ -72,10 +75,10 @@ namespace simgear {
 	std::string strip( const std::string& s );
 
 	/**
-         * Return a new string with any trailing \r and \n characters removed.
+         * Return a new string with any trailing \\r and \\n characters removed.
          * Typically useful to clean a CR-terminated line obtained from
-         * std::getline() which, upon reading CRLF (\r\n), discards the Line
-         * Feed character (\n) but leaves the Carriage Return (\r) in the
+         * std::getline() which, upon reading CRLF (\\r\\n), discards the Line
+         * Feed character (\\n) but leaves the Carriage Return (\\r) in the
          * string.
          * @param s Input string
          * @return The cleaned string
@@ -83,7 +86,7 @@ namespace simgear {
         std::string stripTrailingNewlines(const std::string& s);
 
 	/**
-         * Strip any trailing \r and \n characters from a string.
+         * Strip any trailing \\r and \\n characters from a string.
          * Should have slightly less overhead than stripTrailingNewlines().
          * @param s Input string (modified in-place)
          */
@@ -112,11 +115,12 @@ namespace simgear {
 	 * Produces a result similar to the perl and python functions of the
 	 * same name.
 	 * 
-	 * @param s The string to split into words,
-	 * @param sep Word delimiters.  If not specified then any whitespace is a separator,
-	 * @param maxsplit If given, splits at no more than maxsplit places,
-	 * resulting in at most maxsplit+1 words.
-	 * @return Array of words.
+	 * @param s         The string to split into words
+	 * @param sep       Word delimiters. If not specified then any whitespace is
+	 *                  a separator
+	 * @param maxsplit  If given, splits at no more than maxsplit places,
+	 *                  resulting in at most maxsplit+1 words
+	 * @return Array of words
 	 */
 	string_list
 	split( const std::string& s,
@@ -124,11 +128,11 @@ namespace simgear {
 	       int maxsplit = 0 );
 
       /**
-       * split a string on any of several characters. Commonly used to deal
+       * Split a string on any of several characters. Commonly used to deal
        * with strings containing whitespace, newlines. To parse CSS style
-       * string, use with '\n\t ,' as the seperator list.
+       * string, use with '\\n\\t ,' as the separator list.
        *
-       * Note consecutive seperators will not produce empty entries in the
+       * @note Consecutive separators will not produce empty entries in the
        * the result, i.e splitting 'a,b,,c,d' with a ',' will produce a result
        * with four entries, not five.
        */
@@ -226,14 +230,22 @@ namespace simgear {
     bool to_bool(const std::string& s);
 
     /**
-     * Like strcmp(), but for dotted versions strings NN.NN.NN
-     * any number of terms are supported.
-     * @return 0 if versions match, -ve number if v1 is lower, +ve if v1
-     * is greater
-     * @param maxComponents is the maximum number of components to look at.
-     * This can be used to ignore (say) the patch level by setting it to 2
+     * Compare dotted versions strings NN.NN.NN (analogous to strcmp())
+     *
+     * @note Any number of terms are supported.
+     *
+     * @param v1            First version
+     * @param v2            Second version
+     * @param maxComponents The maximum number of components to look at. This
+     *                      can be used to ignore (say) the patch level by
+     *                      setting it to 2
+     * @return 0 if versions match,
+     *         -ve number if @a v1 is lower,
+     *         +ve if @a v1 is greater
      */
-    int compare_versions(const std::string& v1, const std::string& v2, int maxComponents = 0);
+    int compare_versions( const std::string& v1,
+                          const std::string& v2,
+                          int maxComponents = 0 );
 
     /**
      * Convert a string to upper case.
@@ -345,6 +357,68 @@ namespace simgear {
        *      /views[0]/view[4]/fig, /views[0]/view[1000]/flight
        */
       bool matchPropPathToTemplate(const std::string& path, const std::string& templatePath);
+      
+      bool parseStringAsLatLonValue(const std::string& s, double& result);
+      
+      /**
+       * Attempt to parse a string as a latitude,longitude input. Returns true
+       * or false based on success, and returns the SGGeod by pointer. Leading,
+       * trailing and internal white-space is skipped / ignored.
+       *
+       * Supported formats:
+       *   <signed decimal degrees latitude>,<signed decimal degress longitude>
+       *   <unsigned decimal degrees>[NS],<unsigned decimal degrees>[EW]
+       *   <degrees>*<decimal minutes>'[NS],<degrees>*<decimal minutes>'[EW]
+       *
+       * Latitude and longitude are parsed seperately so the formats for each
+       * do not need to agree. Latitude is assumed to precede longitude
+       * unless assumeLonLatOrder = true
+       *
+       * When NSEW characters are used, the order can be swapped and will be
+       * fixed correctly (longitude then latitude).
+       */
+      bool parseStringAsGeod(const std::string& string,
+                             SGGeod* result = nullptr,
+                             bool assumeLonLatOrder = false);
+      
+      // enum values here correspond to existing lon-lat format codes inside
+      // FlightGear (property: /sim/lon-lat-format )
+      // Don't re-order, just add new ones, or things may break
+      enum class LatLonFormat
+      {
+          DECIMAL_DEGREES = 0,          ///< 88.4N,4.54W,
+          DEGREES_MINUTES,             ///< 88 24.6'N, 4 30.5'W
+          DEGREES_MINUTES_SECONDS,
+          SIGNED_DECIMAL_DEGREES,          ///< 88.4,-4.54
+          SIGNED_DEGREES_MINUTES,
+          SIGNED_DEGREES_MINUTES_SECONDS,
+          ZERO_PAD_DECIMAL_DEGRESS,
+          ZERO_PAD_DEGREES_MINUTES,
+          ZERO_PAD_DEGREES_MINUTES_SECONDS,
+          TRINITY_HOUSE,            ///< dd* mm'.mmm X, ddd* mm'.mmm X (Trinity House Navigation standard).
+          DECIMAL_DEGREES_SYMBOL,        ///< 88.4*N,4.54*W
+          ICAO_ROUTE_DEGREES,              ///< 52N045W or 5212N04512W - precision auto-selected
+      };
+      
+      enum class DegreeSymbol
+      {
+          ASTERISK = 0,
+          SPACE,
+          LATIN1_DEGREE,
+          UTF8_DEGREE
+      };
+      
+      std::string formatLatLonValueAsString(double deg,
+                                            LatLonFormat format, char c,
+                                            DegreeSymbol degreeSymbol = DegreeSymbol::ASTERISK);
+
+      /**
+       * Format an SGGeod as a string according to the provided rule.
+       * if the SGGeod is invalid (default constructed), will return an empty string
+       */
+      std::string formatGeodAsString(const SGGeod& geod,
+                                     LatLonFormat format = LatLonFormat::DECIMAL_DEGREES,
+                                     DegreeSymbol degreeSymbol = DegreeSymbol::ASTERISK);
   } // end namespace strutils
 } // end namespace simgear
 

simgear/misc/strutils_test.cxx

@@ -20,6 +20,7 @@
 #include <simgear/misc/strutils.hxx>
 #include <simgear/structure/exception.hxx>
 #include <simgear/constants.h>
+#include <simgear/math/SGGeod.hxx>
 
 using std::string;
 using std::vector;
@@ -619,6 +620,101 @@ void test_utf8Convert()
     SG_VERIFY(a == aRoundTrip);
 }
 
+void test_parseGeod()
+{
+    SGGeod a;
+    SG_VERIFY(strutils::parseStringAsGeod("56.12,-3.0", &a));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -3.0);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), 56.12);
+    
+    SG_VERIFY(strutils::parseStringAsGeod("56.12345678s,3.12345678w", &a));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -3.12345678);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), -56.12345678);
+    
+    
+    // trailing degrees
+    SG_VERIFY(strutils::parseStringAsGeod("56.12*,-3.0*", &a));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -3.0);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), 56.12);
+    
+    // embedded whitepace, DMS notation, NSEW notation
+    SG_VERIFY(strutils::parseStringAsGeod("\t40 30'50\"S,  12 34'56\"W ", &a));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -12.58222222);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), -40.5138888);
+    
+    // embedded whitepace, DMS notation, NSEW notation, degrees symbol
+    SG_VERIFY(strutils::parseStringAsGeod("\t40*30'50\"S,  12*34'56\"W ", &a));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -12.58222222);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), -40.5138888);
+    
+    // signed degrees-minutes
+    SG_VERIFY(strutils::parseStringAsGeod("-45 27.89,-12 34.56", &a));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -12.576);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), -45.464833333);
+    
+    SG_VERIFY(strutils::parseStringAsGeod("") == false);
+    SG_VERIFY(strutils::parseStringAsGeod("aaaaaaaa") == false);
+    
+    // ordering tests
+    
+    // normal default order, but explicitly pass as lon,lat
+    // (should work)
+    SG_VERIFY(strutils::parseStringAsGeod("3.12345678w, 56.12345678s", &a));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -3.12345678);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), -56.12345678);
+    
+
+    // different default order
+    // also some embedded whitespace for fun
+    SG_VERIFY(strutils::parseStringAsGeod(" -12 34.56,\n-45 27.89 ", &a, true));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -12.576);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), -45.464833333);
+    
+    
+    // differnet default order, but still set explicitly so should
+    // use the lat,lon order
+    SG_VERIFY(strutils::parseStringAsGeod("\t40 30'50\"S,  12 34'56\"W ", &a, true));
+    SG_CHECK_EQUAL_EP(a.getLongitudeDeg(), -12.58222222);
+    SG_CHECK_EQUAL_EP(a.getLatitudeDeg(), -40.5138888);
+    
+    
+    // malformed inputs
+    
+    SG_VERIFY(strutils::parseStringAsGeod("12.345,", &a, true) == false);
+    double d;
+    SG_VERIFY(strutils::parseStringAsLatLonValue("", d) == false);
+}
+
+void test_formatGeod()
+{
+    SGGeod a = SGGeod::fromDeg(-3.46, 55.45);
+    SG_CHECK_EQUAL(strutils::formatGeodAsString(a, strutils::LatLonFormat::SIGNED_DECIMAL_DEGREES), "55.450000,-3.460000");
+    SG_CHECK_EQUAL(strutils::formatGeodAsString(a, strutils::LatLonFormat::DEGREES_MINUTES_SECONDS),
+                   "55*27'00.0\"N,3*27'36.0\"W");
+
+    
+    SG_CHECK_EQUAL(strutils::formatGeodAsString(a, strutils::LatLonFormat::ICAO_ROUTE_DEGREES),
+                   "5527N00327W");
+    SGGeod shortA = SGGeod::fromDeg(106, -34);
+    SG_CHECK_EQUAL(strutils::formatGeodAsString(shortA, strutils::LatLonFormat::ICAO_ROUTE_DEGREES),
+                   "34S106E");
+    
+    
+    const auto s = strutils::formatGeodAsString(a,
+                                                strutils::LatLonFormat::ZERO_PAD_DEGREES_MINUTES,
+                                                strutils::DegreeSymbol::LATIN1_DEGREE);
+    SG_CHECK_EQUAL(s, "55\xB0" "27.000'N,003\xB0" "27.600'W");
+    
+    // Jakarta, if you care
+    SGGeod b = SGGeod::fromDeg(106.8278, -6.1568);
+    const auto s2 = strutils::formatGeodAsString(b,
+                                                strutils::LatLonFormat::DECIMAL_DEGREES_SYMBOL,
+                                                strutils::DegreeSymbol::UTF8_DEGREE);
+    SG_CHECK_EQUAL(s2, "6.156800\xC2\xB0S,106.827800\xC2\xB0" "E");
+    
+    
+}
+
 int main(int argc, char* argv[])
 {
     test_strip();
@@ -639,6 +735,8 @@ int main(int argc, char* argv[])
     test_propPathMatch();
     test_readTime();
     test_utf8Convert();
+    test_parseGeod();
+    test_formatGeod();
     
     return EXIT_SUCCESS;
 }

simgear/nasal/lib.c

@@ -302,15 +302,18 @@ static char* dosprintf(char* f, ...)
     char* buf;
     va_list va;
     int olen, len = 16;
+    va_start(va, f);
     while(1) {
         buf = naAlloc(len);
-        va_start(va, f);
-        olen = vsnprintf(buf, len, f, va);
+        va_list vaCopy;
+        va_copy(vaCopy, va);
+        olen = vsnprintf(buf, len, f, vaCopy);
         if(olen >= 0 && olen < len) {
             va_end(va);
+            va_end(vaCopy);
             return buf;
         }
-        va_end(va);
+        va_end(vaCopy);
         naFree(buf);
         len *= 2;
     }

simgear/package/Catalog.cxx

@@ -81,17 +81,27 @@ bool checkVersion(const std::string& aVersion, SGPropertyNode_ptr props)
     }
     return false;
 }
+    
+SGPropertyNode_ptr alternateForVersion(const std::string& aVersion, SGPropertyNode_ptr props)
+{
+    for (auto v : props->getChildren("alternate-version")) {
+        for (auto versionSpec : v->getChildren("version")) {
+            if (checkVersionString(aVersion, versionSpec->getStringValue())) {
+                return v;
+            }
+        }
+    }
+    
+    return {};
+}
 
 std::string redirectUrlForVersion(const std::string& aVersion, SGPropertyNode_ptr props)
 {
-    BOOST_FOREACH(SGPropertyNode* v, props->getChildren("alternate-version")) {
-        std::string s(v->getStringValue("version"));
-        if (checkVersionString(aVersion, s)) {
-            return  v->getStringValue("url");;
-        }
-    }
+    auto node = alternateForVersion(aVersion, props);
+    if (node)
+        return node->getStringValue("url");;
 
-    return std::string();
+    return {};
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -108,12 +118,12 @@ public:
     }
 
 protected:
-    virtual void gotBodyData(const char* s, int n)
+    void gotBodyData(const char* s, int n) override
     {
         m_buffer += std::string(s, n);
     }
 
-    virtual void onDone()
+    void onDone() override
     {
         if (responseCode() != 200) {
             Delegate::StatusCode code = Delegate::FAIL_DOWNLOAD;
@@ -139,24 +149,27 @@ protected:
 
         std::string ver(m_owner->root()->applicationVersion());
         if (!checkVersion(ver, props)) {
-            SG_LOG(SG_GENERAL, SG_WARN, "downloaded catalog " << m_owner->url() << ", version required " << ver);
-
             // check for a version redirect entry
-            std::string url = redirectUrlForVersion(ver, props);
-            if (!url.empty()) {
-                SG_LOG(SG_GENERAL, SG_WARN, "redirecting from " << m_owner->url() <<
-                       " to \n\t" << url);
-
-                // update the URL and kick off a new request
-                m_owner->setUrl(url);
-                Downloader* dl = new Downloader(m_owner, url);
-                m_owner->root()->makeHTTPRequest(dl);
+            auto alt = alternateForVersion(ver, props);
+            if (alt) {
+                SG_LOG(SG_GENERAL, SG_WARN, "have alternate version of package at:"
+                       << alt->getStringValue("url"));
+                m_owner->processAlternate(alt);
             } else {
+                SG_LOG(SG_GENERAL, SG_WARN, "downloaded catalog " << m_owner->url()
+                       << ", but app version " << ver << " is not comaptible");
                 m_owner->refreshComplete(Delegate::FAIL_VERSION);
             }
 
             return;
         } // of version check failed
+        
+        // validate what we downloaded, in case it's now corrupted
+        // (i.e someone uploaded bad XML data)
+        if (!m_owner->validatePackages()) {
+            m_owner->refreshComplete(Delegate::FAIL_VALIDATION);
+            return;
+        }
 
         // cache the catalog data, now we have a valid install root
         Dir d(m_owner->installRoot());
@@ -169,7 +182,13 @@ protected:
         m_owner->writeTimestamp();
         m_owner->refreshComplete(Delegate::STATUS_REFRESHED);
     }
-
+    
+    void onFail() override
+    {
+        // network level failure
+        SG_LOG(SG_GENERAL, SG_WARN, "catalog network failure for:" << m_owner->url());
+        m_owner->refreshComplete(Delegate::FAIL_DOWNLOAD);
+    }
 private:
 
     CatalogRef m_owner;
@@ -215,7 +234,8 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath)
 
     bool versionCheckOk = checkVersion(aRoot->applicationVersion(), props);
     if (!versionCheckOk) {
-        SG_LOG(SG_GENERAL, SG_INFO, "catalog at:" << aPath << " failed version check: need" << aRoot->applicationVersion());
+        SG_LOG(SG_GENERAL, SG_INFO, "catalog at:" << aPath << " failed version check: app version: "
+               << aRoot->applicationVersion());
         // keep the catalog but mark it as needing an update
     } else {
         SG_LOG(SG_GENERAL, SG_DEBUG, "creating catalog from:" << aPath);
@@ -226,7 +246,9 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath)
     c->parseProps(props);
     c->parseTimestamp();
 
-    if (versionCheckOk) {
+    if (!c->validatePackages()) {
+        c->changeStatus(Delegate::FAIL_VALIDATION);
+    } else if (versionCheckOk) {
         // parsed XML ok, mark status as valid
         c->changeStatus(Delegate::STATUS_SUCCESS);
     } else {
@@ -235,25 +257,44 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath)
 
     return c;
 }
+    
+bool Catalog::validatePackages() const
+{
+    for (auto pack : packages()) {
+        if (!pack->validate()) {
+            SG_LOG(SG_GENERAL, SG_WARN, "Catalog " << id() << " failed validation due to invalid package:" << pack->id());
+            return false;
+        }
+    }
+    
+    return true;
+}
 
 bool Catalog::uninstall()
 {
     bool ok;
     bool atLeastOneFailure = false;
 
-    BOOST_FOREACH(PackageRef p, installedPackages()) {
-        ok = p->existingInstall()->uninstall();
-        if (!ok) {
-            SG_LOG(SG_GENERAL, SG_WARN, "uninstall of package " <<
-                p->id() << " failed");
-            // continue trying other packages, bailing out here
-            // gains us nothing
-            atLeastOneFailure = true;
+    try {
+        // clean uninstall of each airacft / package in turn. This is
+        // slightly overkill since we then nuke the entire catalog
+        // directory anyway
+        for (PackageRef p : installedPackages()) {
+            ok = p->existingInstall()->uninstall();
+            if (!ok) {
+                SG_LOG(SG_GENERAL, SG_WARN, "uninstall of package " <<
+                    p->id() << " failed");
+                // continue trying other packages, bailing out here
+                // gains us nothing
+                atLeastOneFailure = true;
+            }
         }
+    } catch (sg_exception& e) {
+        SG_LOG(SG_GENERAL, SG_WARN, "uninstall of catalog failed " << e.getMessage() << ", will clean-up directory");
+        atLeastOneFailure = true;
     }
 
-    Dir d(m_installRoot);
-    ok = d.remove(true /* recursive */);
+    ok = removeDirectory();
     if (!ok) {
         atLeastOneFailure = true;
     }
@@ -263,6 +304,15 @@ bool Catalog::uninstall()
     return ok;
 }
 
+bool Catalog::removeDirectory()
+{
+    Dir d(m_installRoot);
+    if (!m_installRoot.exists())
+        return true;
+    
+    return d.remove(true /* recursive */);
+}
+    
 PackageList const&
 Catalog::packages() const
 {
@@ -540,6 +590,64 @@ Delegate::StatusCode Catalog::status() const
     return m_status;
 }
 
+bool Catalog::isEnabled() const
+{
+    switch (m_status) {
+    case Delegate::STATUS_SUCCESS:
+    case Delegate::STATUS_REFRESHED:
+    case Delegate::STATUS_IN_PROGRESS:
+    // this is important so we can use Catalog aircraft in offline mode
+    case Delegate::FAIL_DOWNLOAD:
+        return true;
+    default:
+        return false;
+    }
+}
+    
+void Catalog::processAlternate(SGPropertyNode_ptr alt)
+{
+    std::string altId;
+    const auto idPtr = alt->getStringValue("id");
+    if (idPtr) {
+        altId = std::string(idPtr);
+    }
+    
+    std::string altUrl;
+    if (alt->getStringValue("url")) {
+        altUrl = std::string(alt->getStringValue("url"));
+    }
+    
+    CatalogRef existing;
+    if (!altId.empty()) {
+        existing = root()->getCatalogById(altId);
+    } else {
+        existing = root()->getCatalogByUrl(altUrl);
+    }
+    
+    if (existing && (existing != this)) {
+        // we already have the alternate, so just go quiet here
+        changeStatus(Delegate::FAIL_VERSION);
+        return;
+    }
+
+    // we have an alternate ID, and it's differnt from our ID, so let's
+    // define a new catalog
+    if (!altId.empty()) {
+        SG_LOG(SG_GENERAL, SG_INFO, "Adding new catalog:" << altId << " as version alternate for " << id());
+        // new catalog being added
+        createFromUrl(root(), altUrl);
+
+        // and we can go idle now
+        changeStatus(Delegate::FAIL_VERSION);
+        return;
+    }
+    
+    SG_LOG(SG_GENERAL, SG_INFO, "Migrating catalog " << id() << " to new URL:" << altUrl);
+    setUrl(altUrl);
+    Downloader* dl = new Downloader(this, altUrl);
+    root()->makeHTTPRequest(dl);
+}
+    
 } // of namespace pkg
 
 } // of namespace simgear

simgear/package/Catalog.hxx

@@ -134,6 +134,12 @@ public:
     SGPropertyNode* properties() const;
 
     Delegate::StatusCode status() const;
+    
+    /**
+     * is this Catalog usable? This may be false if the catalog is currently
+     * failing a version check or cannot be updated
+     */
+    bool isEnabled() const;
 
     typedef boost::function<void(Catalog*)> Callback;
 
@@ -149,7 +155,8 @@ private:
 
     class Downloader;
     friend class Downloader;
-
+    friend class Root;
+    
     void parseProps(const SGPropertyNode* aProps);
 
     void refreshComplete(Delegate::StatusCode aReason);
@@ -157,10 +164,23 @@ private:
     void parseTimestamp();
     void writeTimestamp();
 
+    /**
+     * @brief wipe the catalog directory from the disk
+     */
+    bool removeDirectory();
+    
+    /**
+     * @brief Helper to ensure all packages are at least somewhat valid, in terms
+     * of an ID, name and directory.
+     */
+    bool validatePackages() const;
+    
     std::string getLocalisedString(const SGPropertyNode* aRoot, const char* aName) const;
 
     void changeStatus(Delegate::StatusCode newStatus);
 
+    void processAlternate(SGPropertyNode_ptr alt);
+    
     Root* m_root;
     SGPropertyNode_ptr m_props;
     SGPath m_installRoot;

simgear/package/CatalogTest.cxx

@@ -61,6 +61,7 @@ std::string readFileIntoString(const SGPath& path)
 
 SGPath global_serverFilesRoot;
 unsigned int global_catalogVersion = 0;
+bool global_failRequests = false;
 
 class TestPackageChannel : public TestServerChannel
 {
@@ -71,6 +72,10 @@ public:
         state = STATE_IDLE;
         SGPath localPath(global_serverFilesRoot);
 
+        if (global_failRequests) {
+            closeWhenDone();
+            return;
+        }
 
         if (path == "/catalogTest1/catalog.xml") {
             if (global_catalogVersion > 0) {
@@ -79,6 +84,19 @@ public:
                 path = ss.str();
             }
         }
+        
+        if (path == "/catalogTestInvalid/catalog.xml") {
+            if (global_catalogVersion > 0) {
+                std::stringstream ss;
+                ss << "/catalogTestInvalid/catalog-v" << global_catalogVersion << ".xml";
+                path = ss.str();
+            }
+        }
+        
+        // return zip data for this computed URL
+        if (path.find("/catalogTest1/movies") == 0) {
+            path = "/catalogTest1/movies-data.zip";
+        }
 
         localPath.append(path);
 
@@ -119,6 +137,12 @@ void waitForUpdateComplete(HTTP::Client* cl, pkg::Root* root)
     std::cerr << "timed out" << std::endl;
 }
 
+template<class T>
+bool vectorContains(const std::vector<T>& vec, const T value)
+{
+    return std::find(vec.begin(), vec.end(), value) != vec.end();
+}
+
 int parseTest()
 {
     SGPath rootPath = simgear::Dir::current().path();
@@ -133,7 +157,7 @@ int parseTest()
     SG_CHECK_EQUAL(cat->description(), "First test catalog");
 
 // check the packages too
-    SG_CHECK_EQUAL(cat->packages().size(), 4);
+    SG_CHECK_EQUAL(cat->packages().size(), 5);
 
     pkg::PackageRef p1 = cat->packages().front();
     SG_CHECK_EQUAL(p1->catalog(), cat.ptr());
@@ -307,7 +331,7 @@ void testAddCatalog(HTTP::Client* cl)
     p.append("org.flightgear.test.catalog1");
     p.append("catalog.xml");
     SG_VERIFY(p.exists());
-    SG_CHECK_EQUAL(root->allPackages().size(), 4);
+    SG_CHECK_EQUAL(root->allPackages().size(), 5);
     SG_CHECK_EQUAL(root->catalogs().size(), 1);
 
     pkg::PackageRef p1 = root->getPackageById("alpha");
@@ -540,6 +564,498 @@ void testInstallTarPackage(HTTP::Client* cl)
     SG_VERIFY(p.exists());
 }
 
+void testInstallArchiveType(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("pkg_install_archive_type");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    // specify a test dir
+    root->setHTTPClient(cl);
+    
+    pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+    waitForUpdateComplete(cl, root);
+    
+    pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.movies");
+    SG_CHECK_EQUAL(p1->id(), "movies");
+    pkg::InstallRef ins = p1->install();
+    
+    SG_VERIFY(ins->isQueued());
+    
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(p1->isInstalled());
+    SG_VERIFY(p1->existingInstall() == ins);
+    
+    // verify on disk state
+    SGPath p(rootPath);
+    p.append("org.flightgear.test.catalog1");
+    p.append("Aircraft");
+    p.append("movies");
+    
+    SG_CHECK_EQUAL(p, ins->path());
+    
+    p.append("movie-list.json");
+    SG_VERIFY(p.exists());
+}
+
+void testDisableDueToVersion(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_disable_at_version");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        root->setHTTPClient(cl);
+        
+        pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(c->isEnabled());
+
+        // install a package
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        pkg::InstallRef ins = p1->install();
+        SG_VERIFY(ins->isQueued());
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(p1->isInstalled());
+    }
+    
+    // bump version and refresh
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "9.1.2"));
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_CHECK_EQUAL(root->allCatalogs().size(), 1);
+        SG_VERIFY(!cat->isEnabled());
+        SG_CHECK_EQUAL(root->catalogs().size(), 0);
+
+        root->setHTTPClient(cl);
+        root->refresh();
+        waitForUpdateComplete(cl, root);
+        SG_CHECK_EQUAL(root->allCatalogs().size(), 1);
+
+        
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VERSION);
+        SG_VERIFY(!cat->isEnabled());
+        SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1");
+        
+        auto enabledCats = root->catalogs();
+        auto it = std::find(enabledCats.begin(), enabledCats.end(), cat);
+        SG_VERIFY(it == enabledCats.end());
+        SG_CHECK_EQUAL(enabledCats.size(), 0);
+        
+        auto allCats = root->allCatalogs();
+        auto j = std::find(allCats.begin(), allCats.end(), cat);
+        SG_VERIFY(j != allCats.end());
+
+        SG_CHECK_EQUAL(allCats.size(), 1);
+
+        // ensure existing package is still installed but not directly list
+        
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_VERIFY(p1 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+
+        auto packs = root->allPackages();
+        auto k = std::find(packs.begin(), packs.end(), p1);
+        SG_VERIFY(k == packs.end());
+    }
+}
+
+void testVersionMigrate(HTTP::Client* cl)
+{
+    global_catalogVersion = 2; // version which has migration info
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_migrate_version");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        root->setHTTPClient(cl);
+        
+        pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(c->isEnabled());
+        
+        // install a package
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        pkg::InstallRef ins = p1->install();
+        SG_VERIFY(ins->isQueued());
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(p1->isInstalled());
+    }
+    
+    // bump version and refresh
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "10.1.2"));
+        root->setHTTPClient(cl);
+        
+        // this should cause auto-migration
+        root->refresh(true);
+        waitForUpdateComplete(cl, root);
+        
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_VERIFY(cat->isEnabled());
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::STATUS_REFRESHED);
+        SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1");
+        SG_CHECK_EQUAL(cat->url(), "http://localhost:2000/catalogTest1/catalog-v10.xml");
+
+        auto enabledCats = root->catalogs();
+        auto it = std::find(enabledCats.begin(), enabledCats.end(), cat);
+        SG_VERIFY(it != enabledCats.end());
+        
+        // ensure existing package is still installed
+        
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_VERIFY(p1 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        
+        auto packs = root->allPackages();
+        auto k = std::find(packs.begin(), packs.end(), p1);
+        SG_VERIFY(k != packs.end());
+    }
+}
+
+void testVersionMigrateToId(HTTP::Client* cl)
+{
+    global_catalogVersion = 2; // version which has migration info
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_migrate_version_id");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        root->setHTTPClient(cl);
+        
+        pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(c->isEnabled());
+        
+        // install a package
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        pkg::InstallRef ins = p1->install();
+        SG_VERIFY(ins->isQueued());
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(p1->isInstalled());
+    }
+    
+    // change version to an alternate one
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "7.5"));
+        root->setHTTPClient(cl);
+        
+        // this should cause the alternate package to be loaded
+        root->refresh(true);
+        waitForUpdateComplete(cl, root);
+        
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_VERIFY(!cat->isEnabled());
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VERSION);
+        SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1");
+        SG_CHECK_EQUAL(cat->url(), "http://localhost:2000/catalogTest1/catalog.xml");
+        
+        auto enabledCats = root->catalogs();
+        auto it = std::find(enabledCats.begin(), enabledCats.end(), cat);
+        SG_VERIFY(it == enabledCats.end());
+                
+        // ensure existing package is still installed
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_VERIFY(p1 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        
+        // but not listed
+        auto packs = root->allPackages();
+        auto k = std::find(packs.begin(), packs.end(), p1);
+        SG_VERIFY(k == packs.end());
+        
+        // check the new catalog
+        auto altCat = root->getCatalogById("org.flightgear.test.catalog-alt");
+        SG_VERIFY(altCat->isEnabled());
+        SG_CHECK_EQUAL(altCat->status(), pkg::Delegate::STATUS_REFRESHED);
+        SG_CHECK_EQUAL(altCat->id(), "org.flightgear.test.catalog-alt");
+        SG_CHECK_EQUAL(altCat->url(), "http://localhost:2000/catalogTest1/catalog-alt.xml");
+        
+        it = std::find(enabledCats.begin(), enabledCats.end(), altCat);
+        SG_VERIFY(it != enabledCats.end());
+        
+        // install a parallel package from the new catalog
+        pkg::PackageRef p2 = root->getPackageById("org.flightgear.test.catalog-alt.b737-NG");
+        SG_CHECK_EQUAL(p2->id(), "b737-NG");
+        pkg::InstallRef ins = p2->install();
+        SG_VERIFY(ins->isQueued());
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(p2->isInstalled());
+        
+        // do a non-scoped lookup, we should get the new one
+        pkg::PackageRef p3 = root->getPackageById("b737-NG");
+        SG_CHECK_EQUAL(p2, p3);
+    }
+    
+    // test that re-init-ing doesn't mirgate again
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "7.5"));
+        root->setHTTPClient(cl);
+        
+        root->refresh(true);
+        waitForUpdateComplete(cl, root);
+        
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_VERIFY(!cat->isEnabled());
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VERSION);
+        
+        auto altCat = root->getCatalogById("org.flightgear.test.catalog-alt");
+        SG_VERIFY(altCat->isEnabled());
+        
+        auto packs = root->allPackages();
+        SG_CHECK_EQUAL(packs.size(), 4);
+    }
+    
+    // and now switch back to the older version
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.0"));
+        root->setHTTPClient(cl);
+        root->refresh(true);
+        waitForUpdateComplete(cl, root);
+        
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_VERIFY(cat->isEnabled());
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::STATUS_REFRESHED);
+        
+        auto altCat = root->getCatalogById("org.flightgear.test.catalog-alt");
+        SG_VERIFY(!altCat->isEnabled());
+
+        // verify the original aircraft is still installed and available
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_VERIFY(p1 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        SG_VERIFY(p1->isInstalled());
+        
+        // verify the alt package is still installed,
+        pkg::PackageRef p2 = root->getPackageById("org.flightgear.test.catalog-alt.b737-NG");
+        SG_VERIFY(p2 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p2->id(), "b737-NG");
+        SG_VERIFY(p2->isInstalled());
+    }
+}
+
+void testOfflineMode(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_offline_mode");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        root->setHTTPClient(cl);
+        
+        pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(c->isEnabled());
+        
+        // install a package
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        pkg::InstallRef ins = p1->install();
+        SG_VERIFY(ins->isQueued());
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(p1->isInstalled());
+    }
+    
+    global_failRequests = true;
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        SG_CHECK_EQUAL(root->catalogs().size(), 1);
+
+        root->setHTTPClient(cl);
+        root->refresh(true);
+        waitForUpdateComplete(cl, root);
+        SG_CHECK_EQUAL(root->catalogs().size(), 1);
+
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_VERIFY(cat->isEnabled());
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_DOWNLOAD);
+        SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1");
+        
+        auto enabledCats = root->catalogs();
+        auto it = std::find(enabledCats.begin(), enabledCats.end(), cat);
+        SG_VERIFY(it != enabledCats.end());
+        
+        // ensure existing package is still installed
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_VERIFY(p1 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        
+        auto packs = root->allPackages();
+        auto k = std::find(packs.begin(), packs.end(), p1);
+        SG_VERIFY(k != packs.end());
+    }
+    
+    global_failRequests = false;
+}
+
+int parseInvalidTest()
+{
+    SGPath rootPath = simgear::Dir::current().path();
+    rootPath.append("testRoot");
+    pkg::Root* root = new pkg::Root(rootPath, "8.1.12");
+    pkg::CatalogRef cat = pkg::Catalog::createFromPath(root, SGPath(SRC_DIR "/catalogTestInvalid"));
+    SG_VERIFY(cat.valid());
+
+    SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VALIDATION);
+ 
+    return 0;
+}
+
+void removeInvalidCatalog(HTTP::Client* cl)
+{
+    global_catalogVersion = 0; // fetch the good version
+    
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_remove_invalid");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    root->setHTTPClient(cl);
+    
+    // another catalog so the dicts are non-empty
+    pkg::CatalogRef anotherCat = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+
+    pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTestInvalid/catalog.xml");
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(!c->isEnabled());
+    SG_VERIFY(c->status() == pkg::Delegate::FAIL_VALIDATION);
+    SG_VERIFY(!vectorContains(root->catalogs(), c));
+    SG_VERIFY(vectorContains(root->allCatalogs(), c));
+ 
+    // now remove it
+    root->removeCatalog(c);
+    SG_VERIFY(!vectorContains(root->catalogs(), c));
+    SG_VERIFY(!vectorContains(root->allCatalogs(), c));
+    c.clear(); // drop the catalog
+
+    // re-add it again, and remove it again
+    {
+        pkg::CatalogRef c2 = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTestInvalid/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(!c2->isEnabled());
+        SG_VERIFY(c2->status() == pkg::Delegate::FAIL_VALIDATION);
+        SG_VERIFY(!vectorContains(root->catalogs(), c2));
+        SG_VERIFY(vectorContains(root->allCatalogs(), c2));
+        
+        // now remove it
+        root->removeCatalog(c2);
+        SG_VERIFY(!vectorContains(root->catalogs(), c2));
+        SG_VERIFY(!vectorContains(root->allCatalogs(), c2));
+    }
+    
+    // only the other catalog (testCatalog should be left)
+    SG_VERIFY(root->allCatalogs().size() == 1);
+    SG_VERIFY(root->catalogs().size() == 1);
+    
+    SG_LOG(SG_GENERAL, SG_INFO, "Remove invalid catalog test passeed");
+}
+
+void updateInvalidToValid(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_update_invalid_to_valid");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+// first, sync the invalid version
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    root->setHTTPClient(cl);
+    
+    pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTestInvalid/catalog.xml");
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(!c->isEnabled());
+    
+    SG_VERIFY(c->status() == pkg::Delegate::FAIL_VALIDATION);
+    SG_VERIFY(!vectorContains(root->catalogs(), c));
+    SG_VERIFY(vectorContains(root->allCatalogs(), c));
+
+// now refrsh the good one
+    global_catalogVersion = 2;
+    c->refresh();
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(c->isEnabled());
+    SG_VERIFY(c->status() == pkg::Delegate::STATUS_REFRESHED);
+    SG_VERIFY(vectorContains(root->catalogs(), c));
+
+}
+
+void updateValidToInvalid(HTTP::Client* cl)
+{
+    global_catalogVersion = 2; // fetch the good version
+    
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_update_valid_to_invalid");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    // first, sync the invalid version
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    root->setHTTPClient(cl);
+    
+    pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTestInvalid/catalog.xml");
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(c->isEnabled());
+    SG_VERIFY(c->status() == pkg::Delegate::STATUS_REFRESHED);
+    SG_VERIFY(vectorContains(root->catalogs(), c));
+    SG_VERIFY(vectorContains(root->allCatalogs(), c));
+    
+    // now refrsh the bad one
+    global_catalogVersion = 3;
+    c->refresh();
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(!c->isEnabled());
+    SG_VERIFY(c->status() == pkg::Delegate::FAIL_VALIDATION);
+    SG_VERIFY(!vectorContains(root->catalogs(), c));
+}
+
+void updateInvalidToInvalid(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_update_invalid_to_inalid");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    // first, sync the invalid version
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    root->setHTTPClient(cl);
+    
+    pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTestInvalid/catalog.xml");
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(!c->isEnabled());
+    
+    SG_VERIFY(c->status() == pkg::Delegate::FAIL_VALIDATION);
+    SG_VERIFY(!vectorContains(root->catalogs(), c));
+    SG_VERIFY(vectorContains(root->allCatalogs(), c));
+    
+    // now refresh to a different, but still bad one
+    global_catalogVersion = 3;
+    c->refresh();
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(!c->isEnabled());
+    SG_VERIFY(c->status() == pkg::Delegate::FAIL_VALIDATION);
+    SG_VERIFY(!vectorContains(root->catalogs(), c));
+    
+}
 
 int main(int argc, char* argv[])
 {
@@ -554,6 +1070,8 @@ int main(int argc, char* argv[])
 
     parseTest();
 
+    parseInvalidTest();
+    
     testInstallPackage(&cl);
 
     testUninstall(&cl);
@@ -563,7 +1081,23 @@ int main(int argc, char* argv[])
     testRefreshCatalog(&cl);
 
     testInstallTarPackage(&cl);
-
-    std::cout << "Successfully passed all tests!" << std::endl;
+    
+    testInstallArchiveType(&cl);
+    
+    testDisableDueToVersion(&cl);
+    
+    testOfflineMode(&cl);
+    
+    testVersionMigrate(&cl);
+    
+    updateInvalidToValid(&cl);
+    updateValidToInvalid(&cl);
+    updateInvalidToInvalid(&cl);
+    
+    removeInvalidCatalog(&cl);
+    
+    testVersionMigrateToId(&cl);
+    
+    SG_LOG(SG_GENERAL, SG_INFO, "Successfully passed all tests!");
     return EXIT_SUCCESS;
 }

simgear/package/Delegate.hxx

@@ -54,6 +54,7 @@ public:
         FAIL_VERSION,       ///< version check mismatch
         FAIL_NOT_FOUND,     ///< package URL returned a 404
         FAIL_HTTP_FORBIDDEN, ///< URL returned a 403. Marked specially to catch rate-limiting
+        FAIL_VALIDATION,    ///< catalog or package failed to validate
         STATUS_REFRESHED,
         USER_CANCELLED
     } StatusCode;

simgear/package/Install.cxx

@@ -36,10 +36,6 @@
 #include <simgear/misc/strutils.hxx>
 #include <simgear/io/iostreams/sgstream.hxx>
 
-extern "C" {
-    void fill_memory_filefunc (zlib_filefunc_def*);
-}
-
 namespace simgear {
 
 namespace pkg {
@@ -49,14 +45,17 @@ class Install::PackageArchiveDownloader : public HTTP::Request
 public:
     PackageArchiveDownloader(InstallRef aOwner) :
         HTTP::Request("" /* dummy URL */),
-        m_owner(aOwner),
-        m_downloaded(0)
+        m_owner(aOwner)
     {
         m_urls = m_owner->package()->downloadUrls();
         if (m_urls.empty()) {
             throw sg_exception("no package download URLs");
         }
 
+       // if (m_owner->package()->properties()->hasChild("archive-type")) {
+       //     setArchiveTypeFromExtension(m_owner->package()->properties()->getStringValue("archive-type"));
+        //}
+        
         // TODO randomise order of m_urls
 
         m_extractPath = aOwner->path().dir();
@@ -106,17 +105,18 @@ protected:
         Dir d(m_extractPath);
         d.create(0755);
 
+		m_extractor.reset(new ArchiveExtractor(m_extractPath));
         memset(&m_md5, 0, sizeof(SG_MD5_CTX));
         SG_MD5Init(&m_md5);
     }
 
     virtual void gotBodyData(const char* s, int n)
     {
-        m_buffer += std::string(s, n);
-        SG_MD5Update(&m_md5, (unsigned char*) s, n);
-
-        m_downloaded = m_buffer.size();
-        m_owner->installProgress(m_buffer.size(), responseLength());
+		const uint8_t* ubytes = (uint8_t*) s;
+        SG_MD5Update(&m_md5, ubytes, n);
+        m_downloaded += n;
+        m_owner->installProgress(m_downloaded, responseLength());
+		m_extractor->extractBytes(ubytes, n);
     }
 
     virtual void onDone()
@@ -147,7 +147,8 @@ protected:
             return;
         }
 
-        if (!extract()) {
+		m_extractor->flush();
+        if (m_extractor->hasError() || !m_extractor->isAtEndOfArchive()) {
             SG_LOG(SG_GENERAL, SG_WARN, "archive extraction failed");
             doFailure(Delegate::FAIL_EXTRACT);
             return;
@@ -165,7 +166,12 @@ protected:
 
         // build a path like /path/to/packages/org.some.catalog/Aircraft/extract_xxxx/MyAircraftDir
         SGPath extractedPath = m_extractPath;
-        extractedPath.append(m_owner->package()->dirName());
+        if (m_owner->package()->properties()->hasChild("archive-path")) {
+            extractedPath.append(m_owner->package()->properties()->getStringValue("archive-path"));
+        } else {
+            extractedPath.append(m_owner->package()->dirName());
+        }
+
 
         // rename it to path/to/packages/org.some.catalog/Aircraft/MyAircraftDir
         bool ok = extractedPath.rename(m_owner->path());
@@ -175,6 +181,8 @@ protected:
         }
 
         // extract_xxxx directory is now empty, so remove it
+        // (note it might not be empty if the archive contained some other
+        // files, but we delete those in such a case
         if (m_extractPath.exists()) {
             simgear::Dir(m_extractPath).remove();
         }
@@ -196,128 +204,6 @@ protected:
     }
 
 private:
-
-    void extractCurrentFile(unzFile zip, char* buffer, size_t bufferSize)
-    {
-        unz_file_info fileInfo;
-        unzGetCurrentFileInfo(zip, &fileInfo,
-            buffer, bufferSize,
-            NULL, 0,  /* extra field */
-            NULL, 0 /* comment field */);
-
-        std::string name(buffer);
-    // no absolute paths, no 'up' traversals
-    // we could also look for suspicious file extensions here (forbid .dll, .exe, .so)
-        if ((name[0] == '/') || (name.find("../") != std::string::npos) || (name.find("..\\") != std::string::npos)) {
-            throw sg_format_exception("Bad zip path", name);
-        }
-
-        if (fileInfo.uncompressed_size == 0) {
-            // assume it's a directory for now
-            // since we create parent directories when extracting
-            // a path, we're done here
-            return;
-        }
-
-        int result = unzOpenCurrentFile(zip);
-        if (result != UNZ_OK) {
-            throw sg_io_exception("opening current zip file failed", sg_location(name));
-        }
-
-        sg_ofstream outFile;
-        bool eof = false;
-        SGPath path(m_extractPath);
-        path.append(name);
-
-    // create enclosing directory heirarchy as required
-        Dir parentDir(path.dir());
-        if (!parentDir.exists()) {
-            bool ok = parentDir.create(0755);
-            if (!ok) {
-                throw sg_io_exception("failed to create directory heirarchy for extraction", path);
-            }
-        }
-
-        outFile.open(path, std::ios::binary | std::ios::trunc | std::ios::out);
-        if (outFile.fail()) {
-            throw sg_io_exception("failed to open output file for writing", path);
-        }
-
-        while (!eof) {
-            int bytes = unzReadCurrentFile(zip, buffer, bufferSize);
-            if (bytes < 0) {
-                throw sg_io_exception("unzip failure reading curent archive", sg_location(name));
-            } else if (bytes == 0) {
-                eof = true;
-            } else {
-                outFile.write(buffer, bytes);
-            }
-        }
-
-        outFile.close();
-        unzCloseCurrentFile(zip);
-    }
-
-    bool extract()
-    {
-        const std::string u(url());
-        const size_t ul(u.length());
-        if (u.rfind(".zip") == (ul - 4)) {
-            return extractUnzip();
-        }
-
-        if (u.rfind(".tar.gz") == (ul - 7)) {
-            return extractTar();
-        }
-
-        SG_LOG(SG_IO, SG_WARN, "unsupported archive format:" << u);
-        return false;
-    }
-
-    bool extractUnzip()
-    {
-        bool result = true;
-        zlib_filefunc_def memoryAccessFuncs;
-        fill_memory_filefunc(&memoryAccessFuncs);
-
-        char bufferName[128];
-        snprintf(bufferName, 128, "%p+%lx", m_buffer.data(), m_buffer.size());
-        unzFile zip = unzOpen2(bufferName, &memoryAccessFuncs);
-
-        const size_t BUFFER_SIZE = 32 * 1024;
-        void* buf = malloc(BUFFER_SIZE);
-
-        try {
-            int result = unzGoToFirstFile(zip);
-            if (result != UNZ_OK) {
-                throw sg_exception("failed to go to first file in archive");
-            }
-
-            while (true) {
-                extractCurrentFile(zip, (char*) buf, BUFFER_SIZE);
-                result = unzGoToNextFile(zip);
-                if (result == UNZ_END_OF_LIST_OF_FILE) {
-                    break;
-                } else if (result != UNZ_OK) {
-                    throw sg_io_exception("failed to go to next file in the archive");
-                }
-            }
-        } catch (sg_exception& ) {
-            result = false;
-        }
-
-        free(buf);
-        unzClose(zip);
-        return result;
-    }
-
-    bool extractTar()
-    {
-        TarExtractor tx(m_extractPath);
-        tx.extractBytes(m_buffer.data(), m_buffer.size());
-        return !tx.hasError() && tx.isAtEndOfArchive();
-    }
-
     void doFailure(Delegate::StatusCode aReason)
     {
         Dir dir(m_extractPath);
@@ -330,12 +216,13 @@ private:
         m_owner->installResult(aReason);
     }
 
+    
     InstallRef m_owner;
     string_list m_urls;
     SG_MD5_CTX m_md5;
-    std::string m_buffer;
     SGPath m_extractPath;
-    size_t m_downloaded;
+    size_t m_downloaded = 0;
+	std::unique_ptr<ArchiveExtractor> m_extractor;
 };
 
 ////////////////////////////////////////////////////////////////////

simgear/package/Package.cxx

@@ -497,6 +497,23 @@ Package::PreviewVec Package::previewsFromProps(const SGPropertyNode_ptr& ptr) co
     return result;
 }
 
+bool Package::validate() const
+{
+    if (m_id.empty())
+        return false;
+    
+    std::string nm(m_props->getStringValue("name"));
+    if (nm.empty())
+        return false;
+    
+    std::string dir(m_props->getStringValue("dir"));
+    if (dir.empty())
+        return false;
+    
+    return true;
+}
+
+    
 } // of namespace pkg
 
 } // of namespace simgear

simgear/package/Package.hxx

@@ -99,10 +99,8 @@ public:
     /**
      * human-readable name - note this is probably not localised,
      * although this is not ruled out for the future.
-     *
-     * Deprecated - please use nameForVariant
      */
-    SG_DEPRECATED(std::string name() const);
+    std::string name() const;
 
     /**
      * Human readable name of a variant
@@ -112,12 +110,9 @@ public:
     std::string nameForVariant(const unsigned int vIndex) const;
 
     /**
-     * syntactic sugar to get the localised description
-     *
-     * Deprecated - please use getLocalisedProp to get the variant-specific
-     * description.
+     * syntactic sugar to get the localised description of the main aircraft
      */
-    SG_DEPRECATED(std::string description() const);
+    std::string description() const;
 
     /**
      * access the raw property data in the package
@@ -225,6 +220,11 @@ private:
 
     void updateFromProps(const SGPropertyNode* aProps);
 
+    /**
+     * @brief check the Package passes some basic consistence checks
+     */
+    bool validate() const;
+    
     std::string getLocalisedString(const SGPropertyNode* aRoot, const char* aName) const;
 
     PreviewVec previewsFromProps(const SGPropertyNode_ptr& ptr) const;

simgear/package/Root.cxx

@@ -408,15 +408,13 @@ Root::Root(const SGPath& aPath, const std::string& aVersion) :
     }
     
     for (SGPath c : dir.children(Dir::TYPE_DIR | Dir::NO_DOT_OR_DOTDOT)) {
-        CatalogRef cat = Catalog::createFromPath(this, c);
-        if (cat) {
-            if (cat->status() == Delegate::STATUS_SUCCESS) {
-                d->catalogs[cat->id()] = cat;
-            } else {
-                // catalog has problems, such as needing an update
-                // keep it out of the main collection for now
-                d->disabledCatalogs.push_back(cat);
-            }
+        // note this will set the catalog status, which will insert into
+        // disabled catalogs automatically if necesary
+        auto cat = Catalog::createFromPath(this, c);
+        if (cat && cat->isEnabled()) {
+            d->catalogs.insert({cat->id(), cat});
+        } else if (cat) {
+            SG_LOG(SG_GENERAL, SG_DEBUG, "Package-Root init: catalog is disabled: " << cat->id());
         }
     } // of child directories iteration
 }
@@ -438,14 +436,31 @@ std::string Root::applicationVersion() const
 
 CatalogRef Root::getCatalogById(const std::string& aId) const
 {
-    CatalogDict::const_iterator it = d->catalogs.find(aId);
+    auto it = d->catalogs.find(aId);
     if (it == d->catalogs.end()) {
-        return NULL;
+        // check disabled catalog list
+        auto j = std::find_if(d->disabledCatalogs.begin(), d->disabledCatalogs.end(),
+                              [aId](const CatalogRef& cat) { return cat->id() == aId; });
+        if (j != d->disabledCatalogs.end()) {
+            return *j;
+        }
+        return nullptr;
     }
 
     return it->second;
 }
 
+CatalogRef Root::getCatalogByUrl(const std::string& aUrl) const
+{
+    auto it = std::find_if(d->catalogs.begin(), d->catalogs.end(),
+                           [aUrl](const CatalogDict::value_type& v)
+                           { return (v.second->url() == aUrl); });
+    if (it == d->catalogs.end())
+        return {};
+    
+    return it->second;
+}
+    
 PackageRef Root::getPackageById(const std::string& aName) const
 {
     size_t lastDot = aName.rfind('.');
@@ -484,6 +499,13 @@ CatalogList Root::catalogs() const
 
     return r;
 }
+    
+CatalogList Root::allCatalogs() const
+{
+    CatalogList r = catalogs();
+    r.insert(r.end(), d->disabledCatalogs.begin(), d->disabledCatalogs.end());
+    return r;
+}
 
 PackageList
 Root::allPackages() const
@@ -544,12 +566,9 @@ void Root::refresh(bool aForce)
 
     toRefresh.insert(toRefresh.end(), d->disabledCatalogs.begin(),
                      d->disabledCatalogs.end());
-
-
-    CatalogList::iterator j = toRefresh.begin();
-    for (; j != toRefresh.end(); ++j) {
-        (*j)->refresh();
-        didStartAny =  true;
+    for (auto cat : toRefresh) {
+        cat->refresh();
+        didStartAny = true;
     }
 
     if (!didStartAny) {
@@ -586,6 +605,12 @@ void Root::scheduleToUpdate(InstallRef aInstall)
     if (!aInstall) {
         throw sg_exception("missing argument to scheduleToUpdate");
     }
+    
+    auto it = std::find(d->updateDeque.begin(), d->updateDeque.end(), aInstall);
+    if (it != d->updateDeque.end()) {
+        // already scheduled to update
+        return;
+    }
 
     PackageList deps = aInstall->package()->dependencies();
     for (Package* dep : deps) {
@@ -663,7 +688,7 @@ void Root::cancelDownload(InstallRef aInstall)
 
 void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason)
 {
-    CatalogDict::iterator catIt = d->catalogs.find(aCat->id());
+    auto catIt = d->catalogs.find(aCat->id());
     d->fireRefreshStatus(aCat, aReason);
 
     if (aReason == Delegate::STATUS_IN_PROGRESS) {
@@ -677,23 +702,20 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason)
         d->catalogs.insert(catIt, CatalogDict::value_type(aCat->id(), aCat));
 
         // catalog might have been previously disabled, let's remove in that case
-        CatalogList::iterator j = std::find(d->disabledCatalogs.begin(),
-                                            d->disabledCatalogs.end(),
-                                            aCat);
+        auto j = std::find(d->disabledCatalogs.begin(),
+                           d->disabledCatalogs.end(),
+                           aCat);
         if (j != d->disabledCatalogs.end()) {
             SG_LOG(SG_GENERAL, SG_INFO, "re-enabling disabled catalog:" << aCat->id());
             d->disabledCatalogs.erase(j);
         }
     }
 
-    if ((aReason != Delegate::STATUS_REFRESHED) &&
-        (aReason != Delegate::STATUS_IN_PROGRESS) &&
-        (aReason != Delegate::STATUS_SUCCESS))
-    {
+    if (!aCat->isEnabled()) {
         // catalog has errors, disable it
-        CatalogList::iterator j = std::find(d->disabledCatalogs.begin(),
-                                            d->disabledCatalogs.end(),
-                                            aCat);
+        auto j = std::find(d->disabledCatalogs.begin(),
+                           d->disabledCatalogs.end(),
+                           aCat);
         if (j == d->disabledCatalogs.end()) {
             SG_LOG(SG_GENERAL, SG_INFO, "disabling catalog:" << aCat->id());
             d->disabledCatalogs.push_back(aCat);
@@ -703,13 +725,39 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason)
         if (catIt != d->catalogs.end()) {
             d->catalogs.erase(catIt);
         }
-    } // of catalog has errors case
+    } // of catalog is disabled
 
     if (d->refreshing.empty()) {
         d->fireRefreshStatus(CatalogRef(), Delegate::STATUS_REFRESHED);
         d->firePackagesChanged();
     }
 }
+    
+bool Root::removeCatalog(CatalogRef cat)
+{
+    if (!cat)
+        return false;
+    
+    // normal remove path
+    if (!cat->id().empty()) {
+        return removeCatalogById(cat->id());
+    }
+    
+    if (!cat->removeDirectory()) {
+        SG_LOG(SG_GENERAL, SG_WARN, "removeCatalog: failed to remove directory " << cat->installRoot());
+    }
+    auto it = std::find(d->disabledCatalogs.begin(),
+                       d->disabledCatalogs.end(),
+                       cat);
+    if (it != d->disabledCatalogs.end()) {
+        d->disabledCatalogs.erase(it);
+    }
+    
+    // notify that a catalog is being removed
+    d->firePackagesChanged();
+    
+    return true;
+}
 
 bool Root::removeCatalogById(const std::string& aId)
 {
@@ -718,13 +766,8 @@ bool Root::removeCatalogById(const std::string& aId)
     CatalogDict::iterator catIt = d->catalogs.find(aId);
     if (catIt == d->catalogs.end()) {
         // check the disabled list
-        CatalogList::iterator j = d->disabledCatalogs.begin();
-        for (; j != d->disabledCatalogs.end(); ++j) {
-            if ((*j)->id() == aId) {
-                break;
-            }
-        }
-
+        auto j = std::find_if(d->disabledCatalogs.begin(), d->disabledCatalogs.end(),
+                              [aId](const CatalogRef& cat) { return cat->id() == aId; });
         if (j == d->disabledCatalogs.end()) {
             SG_LOG(SG_GENERAL, SG_WARN, "removeCatalogById: no catalog with id:" << aId);
             return false;
@@ -738,10 +781,10 @@ bool Root::removeCatalogById(const std::string& aId)
         d->catalogs.erase(catIt);
     }
 
-    bool ok = cat->uninstall();
+    bool ok = cat->removeDirectory();
     if (!ok) {
         SG_LOG(SG_GENERAL, SG_WARN, "removeCatalogById: catalog :" << aId
-            << "failed to uninstall");
+            << "failed to remove directory");
     }
 
     // notify that a catalog is being removed
@@ -752,6 +795,11 @@ bool Root::removeCatalogById(const std::string& aId)
 
 void Root::requestThumbnailData(const std::string& aUrl)
 {
+    if (aUrl.empty()) {
+        SG_LOG(SG_GENERAL, SG_DEV_WARN, "requestThumbnailData: empty URL requested");
+        return;
+    }
+
     auto it = d->thumbnailCache.find(aUrl);
     if (it == d->thumbnailCache.end()) {
         bool cachedOnDisk = d->checkPersistentCache(aUrl);

simgear/package/Root.hxx

@@ -69,7 +69,13 @@ public:
     std::string getLocale() const;
     
     CatalogList catalogs() const;
-        
+    
+    /**
+     * retrive all catalogs, including currently disabled ones
+     */
+    CatalogList allCatalogs() const;
+    
+    
     void setMaxAgeSeconds(unsigned int seconds);
     unsigned int maxAgeSeconds() const;
     
@@ -126,6 +132,8 @@ public:
     
     CatalogRef getCatalogById(const std::string& aId) const;
     
+    CatalogRef getCatalogByUrl(const std::string& aUrl) const;
+    
     void scheduleToUpdate(InstallRef aInstall);
     
     /**
@@ -134,6 +142,13 @@ public:
      */
     bool removeCatalogById(const std::string& aId);
     
+    /**
+     * remove a catalog by reference (used when abandoning installs, since
+     * there may not be a valid catalog Id)
+     */
+    bool removeCatalog(CatalogRef cat);
+
+    
     /**
      * request thumbnail data from the cache / network
      */

simgear/package/catalogTest1/catalog-alt.xml

@@ -0,0 +1,141 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog-alt</id>
+    <description>Alternate test catalog</description>
+    <url>http://localhost:2000/catalogTest1/catalog-alt.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>7.*</version>
+
+
+    <package>
+        <id>alpha</id>
+        <name>Alpha package</name>
+        <revision type="int">9</revision>
+        <file-size-bytes type="int">593</file-size-bytes>
+
+        <md5>a469c4b837f0521db48616cfe65ac1ea</md5>
+        <url>http://localhost:2000/catalogTest1/alpha.zip</url>
+
+        <dir>alpha</dir>
+    </package>
+
+    <package>
+        <id>c172p</id>
+        <name>Cessna 172-P</name>
+        <dir>c172p</dir>
+        <description>A plane made by Cessna</description>
+        <revision type="int">42</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+
+        <tag>cessna</tag>
+        <tag>ga</tag>
+        <tag>piston</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">3</FDM>
+          <systems type="int">4</systems>
+          <model type="int">5</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+
+        <preview>
+          <type>exterior</type>
+          <path>thumb-exterior.png</path>
+          <url>http://foo.bar.com/thumb-exterior.png</url>
+        </preview>
+
+        <preview>
+          <type>panel</type>
+          <path>thumb-panel.png</path>
+          <url>http://foo.bar.com/thumb-panel.png</url>
+        </preview>
+
+        <preview>
+          <path>thumb-something.png</path>
+          <url>http://foo.bar.com/thumb-something.png</url>
+        </preview>
+
+        <variant>
+            <id>c172p-2d-panel</id>
+            <name>C172 with 2d panel only</name>
+        </variant>
+
+        <variant>
+            <id>c172p-floats</id>
+            <name>C172 with floats</name>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-floats.png</path>
+              <url>http://foo.bar.com/thumb-exterior-floats.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172p-skis</id>
+            <name>C172 with skis</name>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-skis.png</path>
+              <url>http://foo.bar.com/thumb-exterior-skis.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <md5>ec0e2ffdf98d6a5c05c77445e5447ff5</md5>
+        <url>http://localhost:2000/catalogTest1/c172p.zip</url>
+
+    </package>
+
+    <package>
+        <id>b737-NG</id>
+        <name>Boeing 737 NG</name>
+        <dir>b737NG</dir>
+        <description>A popular twin-engined narrow body jet</description>
+        <revision type="int">112</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+
+        <tag>boeing</tag>
+        <tag>jet</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">5</FDM>
+          <systems type="int">5</systems>
+          <model type="int">4</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <md5>a94ca5704f305b90767f40617d194ed6</md5>
+        <url>http://localhost:2000/catalogTest1/b737.tar.gz</url>
+
+      </package>
+
+    <package>
+        <id>dc3</id>
+        <name>DC-3</name>
+        <revision type="int">9</revision>
+        <file-size-bytes type="int">593</file-size-bytes>
+
+        <md5>a469c4b837f0521db48616cfe65ac1ea</md5>
+        <url>http://localhost:2000/catalogTest1/alpha.zip</url>
+
+        <dir>dc3</dir>
+    </package>
+</PropertyList>

simgear/package/catalogTest1/catalog-v10.xml

@@ -0,0 +1,178 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog1</id>
+    <description>First test catalog</description>
+    <url>http://localhost:2000/catalogTest1/catalog-v10.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>10.0.*</version>
+    <version>10.1.*</version>
+    <package>
+        <id>alpha</id>
+        <name>Alpha package</name>
+        <revision type="int">8</revision>
+        <file-size-bytes type="int">593</file-size-bytes>
+
+        <md5>a469c4b837f0521db48616cfe65ac1ea</md5>
+        <url>http://localhost:2000/catalogTest1/alpha.zip</url>
+
+        <dir>alpha</dir>
+
+    </package>
+
+    <package>
+        <id>c172p</id>
+        <name>Cessna 172-P</name>
+        <dir>c172p</dir>
+        <description>A plane made by Cessna on Jupiter</description>
+        <revision type="int">42</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+        <author>Standard author</author>
+
+        <tag>cessna</tag>
+        <tag>ga</tag>
+        <tag>piston</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">3</FDM>
+          <systems type="int">4</systems>
+          <model type="int">5</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <!-- local dependency -->
+        <depends>
+            <id>org.flightgear.test.catalog1.common-sounds</id>
+            <revision>10</revision>
+        </depends>
+
+        <preview>
+          <type>exterior</type>
+          <path>thumb-exterior.png</path>
+          <url>http://foo.bar.com/thumb-exterior.png</url>
+        </preview>
+
+        <preview>
+          <type>panel</type>
+          <path>thumb-panel.png</path>
+          <url>http://foo.bar.com/thumb-panel.png</url>
+        </preview>
+
+        <preview>
+          <path>thumb-something.png</path>
+          <url>http://foo.bar.com/thumb-something.png</url>
+        </preview>
+
+        <variant>
+            <id>c172p-2d-panel</id>
+            <name>C172 with 2d panel only</name>
+        </variant>
+
+        <variant>
+            <id>c172p-floats</id>
+            <name>C172 with floats</name>
+            <description>A plane with floats</description>
+            <author>Floats variant author</author>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-floats.png</path>
+              <url>http://foo.bar.com/thumb-exterior-floats.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+
+            <thumbnail>http://foo.bar.com/thumb-floats.png</thumbnail>
+            <thumbnail-path>thumb-floats.png</thumbnail-path>
+        </variant>
+
+        <variant>
+            <id>c172p-skis</id>
+            <name>C172 with skis</name>
+            <description>A plane with skis</description>
+
+            <variant-of>c172p</variant-of>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-skis.png</path>
+              <url>http://foo.bar.com/thumb-exterior-skis.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172r</id>
+            <name>C172R</name>
+            <description>Equally good version of the C172</description>
+            <variant-of>_package_</variant-of>
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172r-floats</id>
+            <name>C172R-floats</name>
+            <description>Equally good version of the C172 with floats</description>
+            <variant-of>c172r</variant-of>
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <md5>ec0e2ffdf98d6a5c05c77445e5447ff5</md5>
+        <url>http://localhost:2000/catalogTest1/c172p.zip</url>
+
+        <thumbnail>http://foo.bar.com/thumb-exterior.png</thumbnail>
+        <thumbnail-path>exterior.png</thumbnail-path>
+    </package>
+
+    <package>
+        <id>b737-NG</id>
+        <name>Boeing 737 NG</name>
+        <dir>b737NG</dir>
+        <description>A popular twin-engined narrow body jet</description>
+        <revision type="int">111</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+
+        <tag>boeing</tag>
+        <tag>jet</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">5</FDM>
+          <systems type="int">5</systems>
+          <model type="int">4</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <md5>a94ca5704f305b90767f40617d194ed6</md5>
+        <url>http://localhost:2000/catalogTest1/b737.tar.gz</url>
+      </package>
+
+    <package>
+        <id>common-sounds</id>
+        <name>Common sound files for test catalog aircraft</name>
+        <revision>10</revision>
+        <dir>sounds</dir>
+        <url>http://localhost:2000/catalogTest1/common-sounds.zip</url>
+        <file-size-bytes>360</file-size-bytes>
+        <md5>acf9eb89cf396eb42f8823d9cdf17584</md5>
+    </package>
+</PropertyList>

simgear/package/catalogTest1/catalog-v2.xml

@@ -10,6 +10,17 @@
     <version>8.0.0</version>
     <version>8.2.0</version>
 
+    <alternate-version>
+      <version>10.*.*</version>
+      <url>http://localhost:2000/catalogTest1/catalog-v10.xml</url>
+    </alternate-version>
+    
+      <alternate-version>
+        <version>7.*</version>
+        <id>org.flightgear.test.catalog-alt</id>
+        <url>http://localhost:2000/catalogTest1/catalog-alt.xml</url>
+      </alternate-version>
+
     <package>
         <id>alpha</id>
         <name>Alpha package</name>

simgear/package/catalogTest1/catalog-v9.xml

@@ -0,0 +1,178 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog1</id>
+    <description>First test catalog</description>
+    <url>http://localhost:2000/catalogTest1/catalog.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>9.1.*</version>
+
+    <package>
+        <id>alpha</id>
+        <name>Alpha package</name>
+        <revision type="int">8</revision>
+        <file-size-bytes type="int">593</file-size-bytes>
+
+        <md5>a469c4b837f0521db48616cfe65ac1ea</md5>
+        <url>http://localhost:2000/catalogTest1/alpha.zip</url>
+
+        <dir>alpha</dir>
+
+    </package>
+
+    <package>
+        <id>c172p</id>
+        <name>Cessna 172-P</name>
+        <dir>c172p</dir>
+        <description>A plane made by Cessna on Jupiter</description>
+        <revision type="int">42</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+        <author>Standard author</author>
+
+        <tag>cessna</tag>
+        <tag>ga</tag>
+        <tag>piston</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">3</FDM>
+          <systems type="int">4</systems>
+          <model type="int">5</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <!-- local dependency -->
+        <depends>
+            <id>org.flightgear.test.catalog1.common-sounds</id>
+            <revision>10</revision>
+        </depends>
+
+        <preview>
+          <type>exterior</type>
+          <path>thumb-exterior.png</path>
+          <url>http://foo.bar.com/thumb-exterior.png</url>
+        </preview>
+
+        <preview>
+          <type>panel</type>
+          <path>thumb-panel.png</path>
+          <url>http://foo.bar.com/thumb-panel.png</url>
+        </preview>
+
+        <preview>
+          <path>thumb-something.png</path>
+          <url>http://foo.bar.com/thumb-something.png</url>
+        </preview>
+
+        <variant>
+            <id>c172p-2d-panel</id>
+            <name>C172 with 2d panel only</name>
+        </variant>
+
+        <variant>
+            <id>c172p-floats</id>
+            <name>C172 with floats</name>
+            <description>A plane with floats</description>
+            <author>Floats variant author</author>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-floats.png</path>
+              <url>http://foo.bar.com/thumb-exterior-floats.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+
+            <thumbnail>http://foo.bar.com/thumb-floats.png</thumbnail>
+            <thumbnail-path>thumb-floats.png</thumbnail-path>
+        </variant>
+
+        <variant>
+            <id>c172p-skis</id>
+            <name>C172 with skis</name>
+            <description>A plane with skis</description>
+
+            <variant-of>c172p</variant-of>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-skis.png</path>
+              <url>http://foo.bar.com/thumb-exterior-skis.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172r</id>
+            <name>C172R</name>
+            <description>Equally good version of the C172</description>
+            <variant-of>_package_</variant-of>
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172r-floats</id>
+            <name>C172R-floats</name>
+            <description>Equally good version of the C172 with floats</description>
+            <variant-of>c172r</variant-of>
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <md5>ec0e2ffdf98d6a5c05c77445e5447ff5</md5>
+        <url>http://localhost:2000/catalogTest1/c172p.zip</url>
+
+        <thumbnail>http://foo.bar.com/thumb-exterior.png</thumbnail>
+        <thumbnail-path>exterior.png</thumbnail-path>
+    </package>
+
+    <package>
+        <id>b737-NG</id>
+        <name>Boeing 737 NG</name>
+        <dir>b737NG</dir>
+        <description>A popular twin-engined narrow body jet</description>
+        <revision type="int">111</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+
+        <tag>boeing</tag>
+        <tag>jet</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">5</FDM>
+          <systems type="int">5</systems>
+          <model type="int">4</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <md5>a94ca5704f305b90767f40617d194ed6</md5>
+        <url>http://localhost:2000/catalogTest1/b737.tar.gz</url>
+      </package>
+
+    <package>
+        <id>common-sounds</id>
+        <name>Common sound files for test catalog aircraft</name>
+        <revision>10</revision>
+        <dir>sounds</dir>
+        <url>http://localhost:2000/catalogTest1/common-sounds.zip</url>
+        <file-size-bytes>360</file-size-bytes>
+        <md5>acf9eb89cf396eb42f8823d9cdf17584</md5>
+    </package>    
+</PropertyList>

simgear/package/catalogTest1/catalog.xml

@@ -177,4 +177,18 @@
         <file-size-bytes>360</file-size-bytes>
         <md5>acf9eb89cf396eb42f8823d9cdf17584</md5>
     </package>
+
+
+    <package>
+        <id>movies</id>
+        <name>movies files for test catalog aircraft</name>
+        <revision>10</revision>
+        <dir>movies</dir>
+        <!-- url has no file extension, instead we set arcive-type -->
+        <url>http://localhost:2000/catalogTest1/movies?wierd=foo;bar=thing</url>
+        <archive-type>zip</archive-type>
+        <archive-path>movies_6789</archive-path>
+        <file-size-bytes>232</file-size-bytes>
+        <md5>e5f89c3f1ed1bdda16174c868f3c7b30</md5>
+    </package>
 </PropertyList>