lurcher / unixODBC

The unixODBC Project goals are to develop and promote unixODBC to be the definitive standard for ODBC on non MS Windows platforms.
GNU Lesser General Public License v2.1
100 stars 52 forks source link

RFC: Splitting unixodbc_conf.h into a public and private header #99

Closed hughmcmaster closed 2 years ago

hughmcmaster commented 2 years ago

unixodbc_conf.h is an arch-specific header that contains #defines indicating how unixODBC was built.

In general, programs should check for and #define SIZEOF_LONG_INT and HAVE_LONG_LONG at compile-time. However, some programs do not #define SIZEOF_LONG_INT. In this case, unixodbc_conf.h #defines many macros, including sizeof(long), and indicates the existence of 'long long' and various system headers.

Most of the macros defined (or not, depending on the system) in unixodbc_conf.h have no relevance to downstream users. In fact, the only relevant macros appear to be the following because they're used in sqltypes.h and odbcinstext.h:

#define HAVE_LONG_LONG 1
#define HAVE_PWD_H 1
#define HAVE_SYS_TYPES_H 1
#define HAVE_UNISTD_H 1
#define SIZEOF_LONG_INT [4 or 8, depending on the arch]

There are also a few extra defines, only used in uodbc_extra.h:

HAVE_SNPRINTF
HAVE_STDARG_H
HAVE_STRCASECMP
HAVE_STRNCASECMP
HAVE_VARARGS_H
HAVE_VSNPRINTF

I'd like to suggest splitting unixodbc_conf.h into a public (unixodbc.h or similar) and private header. The former would contain the first group of #defines above. All other definitions - generally providing information about the build system - would remain in unixodbc_conf.h, which would be a private header, if needed at all.

This would create a clean separation and avoid/clear up confusion about what downstream programs need to #define to use unixODBC correctly.

This idea has evolved from the discussion in #55.

lurcher commented 2 years ago

On 21/02/2022 11:22, Hugh McMaster wrote:

unixodbc_conf.h is an arch-specific header that contains #defines indicating how unixODBC was built.

In general, programs should check for and #define SIZEOF_LONG_INT and HAVE_LONG_LONG at compile-time. However, some programs do not #define SIZEOF_LONG_INT. In this case, unixodbc_conf.h #defines many macros, including sizeof(long), and indicates the existence of 'long long' and various system headers.

Most of the macros defined (or not, depending on the system) in unixodbc_conf.h have no relevance to downstream users. In fact, the only relevant macros appear to be these:

|#define HAVE_LONG_LONG 1 #define HAVE_PWD_H 1 #define HAVE_SYS_TYPES_H 1 #define HAVE_UNISTD_H 1 #define SIZEOF_LONG_INT [4 or 8, depending on the arch] |

There are also a few extra defines, used in uodbc_extra.h:

|HAVE_SNPRINTF HAVE_STDARG_H HAVE_STRCASECMP HAVE_STRNCASECMP HAVE_VARARGS_H HAVE_VSNPRINTF |

I'd like to suggest splitting unixodbc_conf.h into a public (unixodbc.h or similar) and private header. The former would contain the first group of #defines above. All other definitions - generally providing information about the build system - would remain in unixodbc_conf.h, which would be a private header, if needed at all.

This would create a clean separation and avoid/clear up confusion about what downstream programs need to #define to use unixODBC correctly.

This idea has evolved from the discussion in #55 https://github.com/lurcher/unixODBC/issues/55.

That should like a good idea. My general idea is that the header should define how a particular installation of unixODBC IS built and I tried (though may have failed) to avoid application programs from having to explicitly set any macros other than including the config file.

hughmcmaster commented 2 years ago

Having greped the headers in include, it looks as though we can just shift the macros in the first group above to the new header.

config.h retains these macros for use at compile-time, so there is no breakage.

Would you like me to prepare a patch?

lurcher commented 2 years ago

On 22/02/2022 10:46, Hugh McMaster wrote:

Having |grep|ed the headers in |include|, it looks as though we can just shift the macros in the first group above to the new header.

|config.h| retains these macros for use at compile-time, so there is no breakage.

Would you like me to prepare a patch?

Yes, config.h is the one its built against, so the only impact is going to be on third party app building and distributions. So yes, if you can do that it will be great. In general a lot of this is the ever diminishing result of the 32 bit SQLLEN on 64 bit platforms which is just about gone now from what I see.

hughmcmaster commented 2 years ago

32-bit systems also don't #define SQLBIGINT unless HAVE_LONG_LONG is defined.

lurcher commented 2 years ago

On 22/02/2022 11:15, Hugh McMaster wrote:

32-bit systems also don't #define SQLBIGINT unless HAVE_LONG_LONG is defined.

Yes, good point. There may be better ways of finding the 64bit type on those platforms, but not sure they won;t cause more problems than they solve.

hughmcmaster commented 2 years ago

Closing due to MR #101