Change in VSIFilesystemHandler memory handling

[Kurt Schwehr]

Between GDAL 1.10.0 and 1.11.0, I'm seeing a change in how VSIFilesystemHandler handles memory. Specifically, this code worked in 1.10.0 and fails in 1.11.0. Honestly, I don't know how it ever worked as I see how it ever worked in that the InstallHandler method takes a pointer to a handler and the destructor loops over the list and deletes each handler.

source:trunk/gdal/port/cpl_vsil.cpp#L1070​

The code (which is just my test of a more complicated issue):

VSIVirtualHandle *DummyFilesystem::Open(const char *path, const char *access) {
  open_last_path_ = path;
  open_last_access_ = access;
  return nullptr;
}

int DummyFilesystem::Stat(const char *path, VSIStatBufL *stat, int nFlags) {
  stat_last_path_ = path;
  memset(stat, 0, sizeof(*stat));

  // Giving a size and mode is enough to fake that the file exists.
  stat->st_size = 1000;
  stat->st_mode = 33184;

  return 0;
}

// Tests adding a driver and calling open.
TEST(CplVsiFileManagerTest, FailToOpenExistingFile) {
  GDALAllRegister();

  std::unique_ptr<DummyFilesystem> dummy_handler(new DummyFilesystem);
  VSIFileManager::InstallHandler("/dummy/", dummy_handler.get());

  const string path("/dummy/foo.tif");

  GDALDataset *foo =
      reinterpret_cast<GDALDataset*>(GDALOpen(path.c_str(), GA_ReadOnly));

  ASSERT_EQ(nullptr, foo);
  ASSERT_STREQ(path.c_str(), dummy_handler->stat_last_path_.c_str());
  ASSERT_STREQ(path.c_str(), dummy_handler->open_last_path_.c_str());
  ASSERT_STREQ("rb", dummy_handler->open_last_access_.c_str());
}

So, what changed?

And... What is the correct way to allocate a driver? I presume I must do a new.

A follow up question: If I need to install the same driver for multiple paths (for historical reasons), do I need to allocate multiple instances of the VSIFilesystemHandler subclass? Is there any danger in allocating multiple instances of a vsi handler class?

[Even Rouault]

I've diffed cpl_vsil.cpp and cpl_vsi_virtual.h of 1.11 vs 1.10 and see no significant difference, so I guess it "worked" by chance. Using std::unique_ptr is certainly calling for big troubles since, as you saw, VSIFileManager::~VSIFileManager() will manually delete the file system, whereas it would have been deleted at the end of scope of the unique_ptr. So yes, just use new.

Yes I think you can allocate multiple instance of the same subclass. Why wouldn't it work ? Just avoid static member variables...