Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kdbconfig.h macro conflicts #2759

Open
kodebach opened this issue Jun 7, 2019 · 6 comments

Comments

2 participants
@kodebach
Copy link
Contributor

commented Jun 7, 2019

The DEBUG macro (used to indicate debug mode) should not be defined in kdbconfig.h. Instead it should injected by the compiler via CMake.

This is because kdbconfig.h is a public header that will be included in applications using Elektra. However, in such a case the DEBUG macro might conflict without an application's DEBUG macro (see for example LCDproc).

This also means DEBUG should not be used in any other headers that are installed via CMake. Currently this is already the case AFAIK.

@kodebach kodebach changed the title DEBUG macro conflicts kdbconfig.h macro conflicts Jun 7, 2019

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

The same is actually true for all some of the HAVE_* macros as well as the VERBOSE macro.

The SIZEOF_* macros have the same problem, but they are needed in kdbtypes.h, so we probably have to rename or modify kdbtypes.h.

EDIT: most of the HAVE_* macros are fine because they are only defined inside #ifndef.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

Thank you for reporting the problem!

kdbconfig.h was always intended to be a private internal header file, it should not be included in user code.

Why do you include it there?

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

I am not including it directly, but it is included by some of our public headers. Including kdbtypes.h.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

I see. kdbtypes.h was initially also not meant for public use. But as we have it now in the API it obviously needs to be exposed. I think the best way forward here is to split kdbconfig.h to:

  • an internal kdbconfig.h with everything that we query via cmake
  • a public kdbconfig.h (with different name) that has the former SIZEOF_ macros (and maybe some other things externally needed) but always prefixes all macros with ELEKTRA_

@markus2330 markus2330 added this to the 0.9.0 milestone Jun 9, 2019

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

I think we should also move all the internal (i.e. not installed) headers in src/include into src/include/internal so it is more obvious which headers are installed and which aren't without looking at the CMakeLists file. Then this problem shouldn't arise in future, or at least we would notice it beforehand.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

I totally agree!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.