Opened 17 years ago

Closed 16 years ago

#1589 closed defect (fixed)

cpl_config.h really shouldn't be included in public headers

Reported by: Ari Jolma Owned by: warmerdam
Priority: normal Milestone:
Component: ConfigBuild Version: unspecified
Severity: minor Keywords:
Cc:

Description

cpl_config.h contains defines which are meaningful only when GDAL is built and only for GDAL. If it is copied to a public place and included in public headers, it easily generates lot of redefinition warnings and may interfere with the building of code which uses GDAL.

Change History (8)

comment:1 by warmerdam, 17 years ago

Ari,

I don't understand really. All sorts of conditionals in the public files, like gdal.h and cpl_port.h depend on cpl_config.h and so I don't see how we can not distribute it publically.

comment:2 by Ari Jolma, 17 years ago

Frank,

cpl_config.h contains mostly things that configure determines and which are meaningful for building the package (GDAL in this case). But similar things are determined for every other package with configure too, for example PACKAGE_VERSION. The PACKAGE_VERSION in cpl_config.h is for GDAL and for GDAL only and the other package that may use GDAL has its own PACKAGE_VERSION. Surely configure may determine things that need to be included in the public headers of GDAL but then I don't see any other way than having another config.h (with another name of course). The config.h that contains generic defines like PACKAGE_VERSION should be only included in the C files.

I remember discussing this a way back few years. It just came up when I realized that I need to include the config.h which configure creates for libral and which also contains for example PACKAGE_VERSION. When I include ogr_api.h I also get cpl_config.h and the conflicts with my config.h, which I include only in C sources.

comment:3 by warmerdam, 17 years ago

Status: newassigned

Ari,

I have stripped the PACKAGE stuff out of cpl_config.h.in. It does not appear to be used, and is just "autoheader" cruft. Are there any other items causing annoying conflicts?

comment:4 by Ari Jolma, 17 years ago

Frank,

In libral, no.

comment:5 by Ari Jolma, 17 years ago

STDC_HEADERS, if I use VC 7.1

comment:6 by warmerdam, 16 years ago

Ari,

I realize I'm coming back to this long after the fact. Now I'm confused. Are we talking about items in cpl_config.h.in? If so, how does this relate to building using Visual Studio? In that case cpl_config.h.vc should be used and I see it has:

#ifndef STDC_HEADERS
#  define STDC_HEADERS
#endif

perhaps that was changed after you made this comment?

comment:7 by Ari Jolma, 16 years ago

Frank,

I'm not using Visual Studio. STDC_HEADERS is defined in cpl_config.h that is currently installed into /usr/local/include and it is redefined (but to the same value) for example in config.h, which libral's configure makes for compiling libral (but which is not installed).

There are no conflicts in my case and I guess all things currently in cpl_config.h would be the same as what are in any config.h generated for the same machine. Maybe there would be problems in the (unlikely?) case when somebody wants to build something that is actually meant for some other machine than where it is compiled (I've seen that kind of possibility in some packages).

Maybe resolve as worksforme?

Ari

comment:8 by warmerdam, 16 years ago

Component: defaultConfigBuild
Resolution: fixed
Status: assignedclosed

Ari,

You mentioned VC7.1 in http://trac.osgeo.org/gdal/ticket/1589#comment:5

Generally there are a lot of definitions which will hopefully come out the same, including whether or not there are STDC_HEADERS. It is things that are going to differ per package that need to be avoided. Mixing platform headers between different platforms is certain death.

Closing as fixed. Please reopen if you notice any additional problems.

Note: See TracTickets for help on using tickets.