Allow DSN template for pgeo to be set as a config option

[gaige]

Because there are times when you need to override this and would rather not do at compile time, I propose the following:

The PGEO_DRIVER_TEMPLATE configuration option provides a way to replace the existing default string with a new one at run time or from any calling system that needs to change the default driver. This provides for multiple levels of changes to this setting. In my case, I'm checking for multiple drivers and using the one that is most suitable.

-Gaige

==== /ogr/ogrsf_frmts/pgeo/ogrpgeodatasource.cpp
87,89c87,92
<         pszDSN = (char *) CPLMalloc(strlen(pszNewName)+50);
<         sprintf( pszDSN, "DRIVER=Microsoft Access Driver (*.mdb);DBQ=%s", 
<                  pszNewName );
---
> 		const char *pszDSNStringTemplate = NULL;
> 		pszDSNStringTemplate = CPLGetConfigOption( "PGEO_DRIVER_TEMPLATE", "DRIVER=Microsoft Access Driver (*.mdb);DBQ=%s");
> 		if (pszDSNStringTemplate) {
> 			pszDSN = (char *) CPLMalloc(strlen(pszNewName)+strlen(pszDSNStringTemplate)+100);
> 			sprintf( pszDSN, pszDSNStringTemplate,  pszNewName );
> 		}

[warmerdam]

I'm not sure I grasp the need for this. Are there other appropriate odbc drivers for .mdb files on windows?

Note that this change ought to be documented in the PGEO docs.

I have no objection with implementing it.

[gaige]

On windows, it works fine. On the Macintosh, we don't have built-in support for .mdb files and there is inconsistency in the naming of the drivers due to the existence of multiple drivers that perform the same function.

Originally, I had just used a private version of the file (kept in sync by my source control system) which would assign it to some other driver by default. Unfortunately, during our testing, we found that driver to be buggy and couldn't use it for accessing pgeo databases reliably, so we switched to using a different default.

Clearly, we could just pass a different DSN, but as one of the solutions, we include a copy of an alternate mdb file ODBC driver in our code and wanted to directly access it without requiring it to be installed in the system, which required the use of the direct DRIVER= keyword instead of using the DSN. But, this introduced a different problem, because the user on the Mac is allowed to put an Application anywhere they like on the disk, so the path to the executable (and all of its resources) cannot be determined prior to runtime.

Since the construction of the file path is done inside of the PGeo driver, the only options are to modify how the PGEO driver builds the path or to use a completely different file access method above GDAL for PGeo files, I wanted to do something that solved the runtime determination of the DSN.

In this case, I took a page from the runtime configuration of GDAL_DATA, which allows runtime configuration of the location of various data files, which has worked beautifully for my needs in the past.

This seemed like a good solution, with some precedent in the code elsewhere, for the need to configure at runtime.

Does that make sense? Thanks, -Gaige

[warmerdam]

Sounds good to me, go ahead.

[gaige]

r20429 /trunk/gdal/ogr/ogrsf_frmts/pgeo/ogrpgeodatasource.cpp Allow DSN template for pgeo to be set as a config option (#3407)

[Even Rouault]

r20440 /trunk/gdal/ogr/ogrsf_frmts/pgeo/ogrpgeodatasource.cpp: PGeo: add validation of PGEO_DRIVER_TEMPLATE content to avoid crashes

[gaige]

Replying to rouault:

r20440 /trunk/gdal/ogr/ogrsf_frmts/pgeo/ogrpgeodatasource.cpp: PGeo: add validation of PGEO_DRIVER_TEMPLATE content to avoid crashes

Was this prospective, or did you experience a crash?

Thanks, -Gaige

[Even Rouault]

Did you try setting PGEO_DRIVER_TEMPLATE="foo%s%s%s%s" ;-) ?

Yes, this was just a potential vulnerability (but one can argue it is not obvious for the user that the value of this option must be a valid printf formatting string..).

By the way, might be worth documenting the option in drv_pgeo.html unless you want to keep it private for you...

[gaige]

Sorry, I didn't mean to come across as defensive, I just wanted to make sure that I didn't miss something when I ran the test suite. Yes, I do think it's a good addition and yes, I will document it in the docs, since the whole purpose of putting it into trunk was to make the system more accessible to folks who don't have Windows systems.

-Gaige

[Even Rouault]

Be reassured. I did understand the intent of your question well. I might sound ironic at times. Not always obvious to tune your way of expressing things adequately when you write in a foreign language...

[gaige]

r20441 /trunk/gdal/ogr/ogrsf_frmts/pgeo/drv_pgeo.html : Add documentation for PGEO_DRIVER_TEMPLATE (#3407)