GDALRegister driver setup questions

[Kurt Schwehr]

After r32110 for ogrsf_fmrts and r32190 with the raster frmts/*, I have a couple questions. Not all drivers are setup the same way and it's not immediately obvious why:

• Some check GDALGetDriverByName(), others do not
• Some check the version, others do not. e.g. GDAL_CHECK_VERSION("OGR/DODS driver")
• Some use a generic driver and set parameters. e.g. GDALDriver *poDriver = new GDALDriver(); There is a wide range of what is set on this. e.g. how many of the pfn functions are set
• Some just set a new instance of themselves. e.g. OGRSFDriver* poDriver = new OGRXPlaneDriver
• Some have an extern "C" declaration for the register function in the cpp file, some wrap the function in it, some do not have it. Yet all OGR drivers are inside of CPL_C_START in ogrsf_frmts.h. The local externs are therefore redundant.
• Raster drivers have gdal_frmts.h, but mostly use a local definition. Why not remove all local definitions and just use that header?

Sources:

• source:trunk/gdal/ogr/ogrsf_frmts/ogrsf_frmts.h​
• source:trunk/gdal/gcore/gdal_frmts.h​

I think we should:

• Remove all local declarations from cpp and just have them in two headers
• Remove all voids from the declarations
• Remove all wrapping of the actual functions definitions in cpp files within CPL_C_START
• Sort the driver list in the headers or add a comment to explain why they are in a particular order. Are they in the register order or random order?

[Even Rouault]

• Some check GDALGetDriverByName(), others do not

Oh really? All drivers should check they aren't really registered

• Some check the version, others do not. e.g. GDAL_CHECK_VERSION("OGR/DODS driver")

Yes, GDAL_CHECK_VERSION is useful for drivers that have dependencies to external libraries and could be compiled as plugins. This avoid linking an old plugin to a newer GDAL

• Some use a generic driver and set parameters. e.g. GDALDriver *poDriver = new GDALDriver(); There is a wide range of what is set on this. e.g. how many of the pfn functions are set
• Some just set a new instance of themselves. e.g. OGRSFDriver* poDriver = new OGRXPlaneDriver

Yeah, part of the OGR drivers that predate RFC 46 haven't been converted to GDAL drivers.

• Some have an extern "C" declaration for the register function in the cpp file, some wrap the function in it, some do not have it. Yet all OGR drivers are inside of CPL_C_START in ogrsf_frmts.h. The local externs are therefore redundant.

The local externes are generally left over of the initial stage of development where you compile the driver as a plugin before integrating it in the makefiles. Could be safely removed.

• Raster drivers have gdal_frmts.h, but mostly use a local definition. Why not remove all local definitions and just use that header?

Isn't it the same question as above ?

Sources:

• source:trunk/gdal/ogr/ogrsf_frmts/ogrsf_frmts.h​
• source:trunk/gdal/gcore/gdal_frmts.h​

I think we should:

• Remove all local declarations from cpp and just have them in two headers

+1

• Remove all voids from the declarations

XXX foo(void) is the canonical way for a C compatible header, otherwise you'll get a warning

• Remove all wrapping of the actual functions definitions in cpp files within CPL_C_START

+1

• Sort the driver list in the headers or add a comment to explain why they are in a particular order. Are they in the register order or random order?

Basically yes I believe they are in the same order in the .h as in the GDALAllRegister() function. No opinion if that must be reordered or not in the .h

[Even Rouault]

This ticket has been automatically closed because Trac is no longer used for GDAL bug tracking, since the project has migrated to GitHub. If you believe this ticket is still valid, you may file it to ​https://github.com/OSGeo/gdal/issues if it is not already reported there.