[Gdal-dev] Incorrect plugin search when registering GDAL drivers

[Frank Warmerdam]

On Wed, 19 Jan 2005 17:51:42 +0100, Marek Brudka <mbrudka at aster.pl> wrote:
> Hi,
> I've reported the following error in bugzilla
> (http://208.24.120.44/show_bug.cgi?id=746).
> 
> This bug is related with void GDALDriverManager::AutoLoadDrivers()
> method. The way this method works leads to errors, which manifest under
> windows, though it is possible other systems are also affected.
> Method GDALDriverManager::AutoLoadDrivers() tries to load every file
> found in papszSearchPath starting with gdal_. For GDAL_DRIVER_PATH
> defined this is quite resonable. However when GDAL_DRIVER_PATH is
> undefined, method tries to load files from /usr/local/lib path, which is
> frequently used for storing other import libraries. Obviously, loading
> import library as shared one results in linking errors. To fix this bug,
> GDALDriverManager::AutoLoadDrivers() method should check if given file
> is  shared library before loading.
> The current plugin loading is rather badly designed. I propose to change
> the functionality of GDALDriverManager::AutoLoadDrivers() as follows:
> 1. AutoLoadDrives should be parameterized be a search path.

Marek,

I disagree.  I want stock GDAL applications to support autoloaded
drivers with no special effort on their part to track paths and so forth.
So requring this path to come from a parameter would interfere with
this. 

> 2. GDAL_DRIVER_PATH should not be defined at gdal library level, but
> rather at the user application one. Every environment variable
> introduces configuration and deployment problems, hence they should be
> avoided. Otherwise configuration of the application becomes a nightmare.

GDAL_DRIVER_PATH is a CPL Configuration item.  It can be defined
by environment variable or applications can override it with a call to 
CPLSetConfigOption(). 

> 3. RegisterAllDrivers should be parameterized by a search path, possible
> with NULL default.

As above, I don't with this standard entry point parameterized as whatever
the parameter is used for will require attention by all GDAL applications.

> 4. Every file processed by GDALDriverManager::AutoLoadDrivers() should
> be verified if it is real shared library.

Note that an attempt is only made to load files prefixed with "gdal_".
I think it would be sensible to also modify the autoloader to verify that
the extension is .so, .dylib or .dll.  

> 5. Every shared library should be check against compilation variant.
> Loading dll compiled in Debug mode in release version of the library
> often leads to critical errors. As a rule of thumb, one should never mix
> Debug and Release variants under Windows. (this is hard issue).

It is the responsibility of the developer to ensure that all components
are compiled in a compatible way.  I think it would be prudent to 
implement a mechanism for the plugin registration methods to check
that they were compiled against a compatible version of GDAL to the
one into which they are being loaded.  Unfortunately this is challenging.
I can ensure that they are compiled for the same version via the version
string (using  GDAL_RELEASE_NAME macro) and comparing to the
runtime value returned by GDALVersionInfo().  

I considered doing this; however, it would often then consider 
plugins incompatible that would have worked if they were compiled
for a slightly different but ABI compatible version.  The central issue
here is that the GDAL C++ ABI changes frequently but not with every
minor version.  It is really only the  C ABI that I try to maintain strict
backward compatibility for. 

In general it is the responsibility of plugin users to ensure they are for
a compatible version of GDAL for now.  If there is wide agreement I 
could change this to do the GDAL_RELEASE_NAME check. 

> I think that plugin architecture of GDAL requires wide discussion in the
> context of GDAL build system.

I am agreeable to this.

PS. It can be construed as rude to suggest someone elses code is
"rather poorly designed".   Consider the possibility that it was well 
designed for objectives quite different than your own.  Of course, I do 
try to  declare my own code poorly designed.   

Best regards,

-- 
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent