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

[Marek Brudka]

User  Frank Warmerdam wrote:

>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.
>>    
>>
>
>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. 
>  
>
I agree, this is less convenient that using GDAL_DRIVER_PATH path. But 
it is simplier
and more explicit design. Simplicity of the interface is the one of the 
most desired feature for
any library. This is why OGC standards are so cool :-)

In general, one after installing gdal-devel libraries should not set 
anything in environment, because
(Morphy Law) for sure he or she will not do 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().
>  
>
Sure. But this means, that almost any application should call 
CPLSetConfigOption() before
registration of GDAL drivers. This way one achieves in two steps, namely 
CPLSetConfigOption
and GDALRegisterAll the state which can be achieved in one step, namely 
GDALRegisterAll( path ).
This is complex, because assumes the developer knows the interiors of GDAL.

Please notice, that the only responsibility of GDALRegisterAll(  ) 
function is the registration of all
drivers and plugins. But the signature of this function does not tell 
anything about the way it is done, but
makes some implicit assumptions, where plugins should be looked for. 
Then neither the developer is not told
by the interface what  should be provided, nor the compiler can guard 
proper usage of this function.

>>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.  
>  
>
We use several open source libraries under windows/linux/neutrino. We 
decided to follow standard unix tree
on windows to be consistent among OS. We have c:\usr\local\lib for 
holding import libraries and c:\usr\local\bin
for windows shared libraries, because under windows one should follow M$ 
conventions and to avoid setting
lib on %PATH%. In c:\usr\local\lib we had a number of gdal import 
libraries built for various variants (debug/release
OCI/noOCI etc.)  You can imagine the result.

I know, it is enough to set GDAL_DRIVER_PATH, but this is another 
variable a developer has to define to create
the developement framework. Multiply, please the number of developers by 
the number of each OS library settings.
Assume that each case has to be explained separatedly for every library 
update by a manager or software architect,
because in production environment people avoid reading docs/notes. This 
is why I think that configuration variables
should be avoided or moved on C++ interface level. This enforces each 
developer to enhance his knowledge..

If you decide to load plugins from a default path, when GDAL_DRIVER_PATH 
is undefined
I propose to not search in /usr/local/lib, but /usr/local/plugins/gdal/, 
/usr/local/lib/plugins/gdal or something similiar.
Please notice, that plugins aware applications usually has their own 
specific search path eg. mozilla, opera, qt .., and
do not even try to look into system wide directories.

>>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 ACE there is a convention for library names. Debug library under 
windows is called <name>d.lib, <name>d.dll,
while for unix lib<name>d.a and lib<name>d.so. Release versions does not 
have 'd'. When compiling any project a
macro is defined (or DEBUG NDEBUG standard macros are used) to tell if 
this is Debug or Release version. That's
enough to discriminate between plugins. I agree, that this solution is 
not the ideal one, but manages almost all
practical situations.

The solution you proposed is more elaborated, and I think it is better 
than the above, because really attacks
the problem.

>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. 
>  
>
I suppose that compatibility level can be controlled when loading 
plugins eg.
we can assume that version string contains detailed information on GDAL 
major and minor
versions as well as build variant. W plugin can be considered to be 
compatible when major
version and build variant agree.

>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. 
>  
>
Cool!

>>I think that plugin architecture of GDAL requires wide discussion in the
>>context of GDAL build system.
>>    
>>
Lot of work, but I can help on both concept and implementation level.

>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.   
>  
>
I'm sorry once again.

Regards
Marek