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

Fix redefined warnings from config.h #303

Merged
merged 1 commit into from
Mar 21, 2018

Conversation

sergeyvfx
Copy link
Contributor

The issue was caused by config.h header being included from both
header files and implementation files.

Proposed solution is to have regular header guard in the generated
config.h. Benefit of this solution is that it's least intrusive.
Downside is that it only solves issue for CMake build system, and
autoconf one is not fixed since header template is automatically
generated by autoheader who does not add header guard.

@@ -1,3 +1,6 @@
#ifndef __GLOG_CONFIG_H__
#define __GLOG_CONFIG_H__
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Macro names with prefix underscores are reserved for compiler use. Just change the define into GLOG_CONFIG_H. That should do just fine.

The issue was caused by config.h header being included from both
header files and implementation files.

Proposed solution is to have regular header guard in the generated
config.h. Benefit of this solution is that it's least intrusive.
Downside is that it only solves issue for CMake build system, and
autoconf one is not fixed since header template is automatically
generated by autoheader who does not add header guard.
@sergeyvfx
Copy link
Contributor Author

The patch should now be updated with removed underscores.

@sergiud sergiud added the LGTM label Mar 20, 2018
@sergiud sergiud merged commit 11afec2 into google:master Mar 21, 2018
durswd pushed a commit to durswd/glog that referenced this pull request Sep 2, 2019
Fix redefined warnings from config.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants