szaszm commented on a change in pull request #1211:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1211#discussion_r749244836
##########
File path: cmake/Extensions.cmake
##########
@@ -22,10 +22,31 @@ define_property(GLOBAL PROPERTY EXTENSION-OPTIONS
set_property(GLOBAL PROPERTY EXTENSION-OPTIONS "")
+set(extension-build-info-file
"${CMAKE_CURRENT_BINARY_DIR}/ExtensionBuildInfo.cpp")
+file(GENERATE OUTPUT ${extension-build-info-file}
+ CONTENT "\
+ #include \"utils/Export.h\"\n\
+ #ifdef BUILD_ID_VARIABLE_NAME\n\
+ EXTENSIONAPI extern const char* const BUILD_ID_VARIABLE_NAME =
\"__EXTENSION_BUILD_IDENTIFIER_BEGIN__${BUILD_IDENTIFIER}__EXTENSION_BUILD_IDENTIFIER_END__\";\n\
Review comment:
Normally `extern` is used do declare a global variable in a header that
is then defined in some implementation file without violation ODR. Otherwise a
global variable declaration would also be a definition. I don't understand why
having an initializer here is even legal c++, since it should go to the
separate definition, not the declaration.
According to [this](https://stackoverflow.com/a/57900212) stackoverflow
answer, having `extern` here is the same as not having it, because the
initializer makes a definition and all globals have external linkage by default.
What do you think about having a global function instead that returns a
`const char*` to the magic string? I think that shouldn't be stripped from the
binary even with LTO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]