From cf74fc24d4e2b2910921323ea1862bc234919585 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 2 Sep 2015 16:24:52 -0400 Subject: cmTarget: Fix buildsystem property empty value set and append operations Refactoring in commit 1f54bc1c (cmTarget: Split storage of include directories from genexes, 2015-08-04), commit 772ecef4 (cmTarget: Split storage of compile options from genexes, 2015-08-04), commit 44e071ae (cmTarget: Split storage of compile features from genexes, 2015-08-04), and commit 197f4de1 (cmTarget: Split storage of compile definitions from genexes, 2015-08-04) failed to account for value==NULL in SetProperty and AppendProperty methods. --- Source/cmTarget.cxx | 72 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 590654d..396715d 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1601,33 +1601,45 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) { this->Internal->IncludeDirectoriesEntries.clear(); this->Internal->IncludeDirectoriesBacktraces.clear(); - this->Internal->IncludeDirectoriesEntries.push_back(value); - cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); - this->Internal->IncludeDirectoriesBacktraces.push_back(lfbt); + if (value) + { + this->Internal->IncludeDirectoriesEntries.push_back(value); + cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); + this->Internal->IncludeDirectoriesBacktraces.push_back(lfbt); + } } else if(prop == "COMPILE_OPTIONS") { this->Internal->CompileOptionsEntries.clear(); this->Internal->CompileOptionsBacktraces.clear(); - this->Internal->CompileOptionsEntries.push_back(value); - cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); - this->Internal->CompileOptionsBacktraces.push_back(lfbt); + if (value) + { + this->Internal->CompileOptionsEntries.push_back(value); + cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); + this->Internal->CompileOptionsBacktraces.push_back(lfbt); + } } else if(prop == "COMPILE_FEATURES") { this->Internal->CompileFeaturesEntries.clear(); this->Internal->CompileFeaturesBacktraces.clear(); - this->Internal->CompileFeaturesEntries.push_back(value); - cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); - this->Internal->CompileFeaturesBacktraces.push_back(lfbt); + if (value) + { + this->Internal->CompileFeaturesEntries.push_back(value); + cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); + this->Internal->CompileFeaturesBacktraces.push_back(lfbt); + } } else if(prop == "COMPILE_DEFINITIONS") { this->Internal->CompileDefinitionsEntries.clear(); this->Internal->CompileDefinitionsBacktraces.clear(); - this->Internal->CompileDefinitionsEntries.push_back(value); - cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); - this->Internal->CompileDefinitionsBacktraces.push_back(lfbt); + if (value) + { + this->Internal->CompileDefinitionsEntries.push_back(value); + cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); + this->Internal->CompileDefinitionsBacktraces.push_back(lfbt); + } } else if(prop == "EXPORT_NAME" && this->IsImported()) { @@ -1693,27 +1705,39 @@ void cmTarget::AppendProperty(const std::string& prop, const char* value, } else if(prop == "INCLUDE_DIRECTORIES") { - this->Internal->IncludeDirectoriesEntries.push_back(value); - cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); - this->Internal->IncludeDirectoriesBacktraces.push_back(lfbt); + if (value) + { + this->Internal->IncludeDirectoriesEntries.push_back(value); + cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); + this->Internal->IncludeDirectoriesBacktraces.push_back(lfbt); + } } else if(prop == "COMPILE_OPTIONS") { - this->Internal->CompileOptionsEntries.push_back(value); - cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); - this->Internal->CompileOptionsBacktraces.push_back(lfbt); + if (value) + { + this->Internal->CompileOptionsEntries.push_back(value); + cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); + this->Internal->CompileOptionsBacktraces.push_back(lfbt); + } } else if(prop == "COMPILE_FEATURES") { - this->Internal->CompileFeaturesEntries.push_back(value); - cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); - this->Internal->CompileFeaturesBacktraces.push_back(lfbt); + if (value) + { + this->Internal->CompileFeaturesEntries.push_back(value); + cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); + this->Internal->CompileFeaturesBacktraces.push_back(lfbt); + } } else if(prop == "COMPILE_DEFINITIONS") { - this->Internal->CompileDefinitionsEntries.push_back(value); - cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); - this->Internal->CompileDefinitionsBacktraces.push_back(lfbt); + if (value) + { + this->Internal->CompileDefinitionsEntries.push_back(value); + cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); + this->Internal->CompileDefinitionsBacktraces.push_back(lfbt); + } } else if(prop == "EXPORT_NAME" && this->IsImported()) { -- cgit v0.12 From 407ff47eca85c6d73c9cefee0d1b0afef41e0b19 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 3 Sep 2015 06:37:26 -0400 Subject: cmTarget: Fix memory leak when SOURCES property is cleared --- Source/cmTarget.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 396715d..3425f34 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1671,6 +1671,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) this->Internal->SourceFilesMap.clear(); cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); cmGeneratorExpression ge(lfbt); + cmDeleteAll(this->Internal->SourceEntries); this->Internal->SourceEntries.clear(); cmsys::auto_ptr cge = ge.Parse(value); this->Internal->SourceEntries.push_back( -- cgit v0.12 From b9856862fe46b04b25d99c3488899d6b12bd6e1a Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 2 Sep 2015 15:57:41 -0400 Subject: Tests: Cover set_property for buildsystem target properties Extend the RunCMake.set_property test with cases covering buildsystem target properties: * COMPILE_DEFINITIONS * COMPILE_FEATURES * COMPILE_OPTIONS * INCLUDE_DIRECTORIES * LINK_LIBRARIES * SOURCES Also test a non-buildsystem property to document the current difference in behavior. Refactor the existing LINK_LIBRARIES case to the same more-extensive test as the rest. Use the output generated by CMake 3.3 as the expected output for each test case. --- Tests/RunCMake/set_property/COMPILE_DEFINITIONS-stdout.txt | 1 + Tests/RunCMake/set_property/COMPILE_DEFINITIONS.cmake | 2 ++ Tests/RunCMake/set_property/COMPILE_FEATURES-stdout.txt | 1 + Tests/RunCMake/set_property/COMPILE_FEATURES.cmake | 2 ++ Tests/RunCMake/set_property/COMPILE_OPTIONS-stdout.txt | 1 + Tests/RunCMake/set_property/COMPILE_OPTIONS.cmake | 2 ++ Tests/RunCMake/set_property/Common.cmake | 14 ++++++++++++++ Tests/RunCMake/set_property/INCLUDE_DIRECTORIES-stdout.txt | 1 + Tests/RunCMake/set_property/INCLUDE_DIRECTORIES.cmake | 2 ++ Tests/RunCMake/set_property/LINK_LIBRARIES-stdout.txt | 1 + Tests/RunCMake/set_property/LINK_LIBRARIES.cmake | 9 ++------- Tests/RunCMake/set_property/RunCMakeTest.cmake | 6 ++++++ Tests/RunCMake/set_property/SOURCES-stdout.txt | 1 + Tests/RunCMake/set_property/SOURCES.cmake | 2 ++ Tests/RunCMake/set_property/USER_PROP-stdout.txt | 1 + Tests/RunCMake/set_property/USER_PROP.cmake | 2 ++ 16 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 Tests/RunCMake/set_property/COMPILE_DEFINITIONS-stdout.txt create mode 100644 Tests/RunCMake/set_property/COMPILE_DEFINITIONS.cmake create mode 100644 Tests/RunCMake/set_property/COMPILE_FEATURES-stdout.txt create mode 100644 Tests/RunCMake/set_property/COMPILE_FEATURES.cmake create mode 100644 Tests/RunCMake/set_property/COMPILE_OPTIONS-stdout.txt create mode 100644 Tests/RunCMake/set_property/COMPILE_OPTIONS.cmake create mode 100644 Tests/RunCMake/set_property/Common.cmake create mode 100644 Tests/RunCMake/set_property/INCLUDE_DIRECTORIES-stdout.txt create mode 100644 Tests/RunCMake/set_property/INCLUDE_DIRECTORIES.cmake create mode 100644 Tests/RunCMake/set_property/LINK_LIBRARIES-stdout.txt create mode 100644 Tests/RunCMake/set_property/SOURCES-stdout.txt create mode 100644 Tests/RunCMake/set_property/SOURCES.cmake create mode 100644 Tests/RunCMake/set_property/USER_PROP-stdout.txt create mode 100644 Tests/RunCMake/set_property/USER_PROP.cmake diff --git a/Tests/RunCMake/set_property/COMPILE_DEFINITIONS-stdout.txt b/Tests/RunCMake/set_property/COMPILE_DEFINITIONS-stdout.txt new file mode 100644 index 0000000..b85f41d --- /dev/null +++ b/Tests/RunCMake/set_property/COMPILE_DEFINITIONS-stdout.txt @@ -0,0 +1 @@ +-- Target COMPILE_DEFINITIONS is 'a;;b;c;;d;;e' diff --git a/Tests/RunCMake/set_property/COMPILE_DEFINITIONS.cmake b/Tests/RunCMake/set_property/COMPILE_DEFINITIONS.cmake new file mode 100644 index 0000000..ec07ce9 --- /dev/null +++ b/Tests/RunCMake/set_property/COMPILE_DEFINITIONS.cmake @@ -0,0 +1,2 @@ +include(Common.cmake) +test_target_property(COMPILE_DEFINITIONS) diff --git a/Tests/RunCMake/set_property/COMPILE_FEATURES-stdout.txt b/Tests/RunCMake/set_property/COMPILE_FEATURES-stdout.txt new file mode 100644 index 0000000..81ef170 --- /dev/null +++ b/Tests/RunCMake/set_property/COMPILE_FEATURES-stdout.txt @@ -0,0 +1 @@ +-- Target COMPILE_FEATURES is 'a;;b;c;;d;;e' diff --git a/Tests/RunCMake/set_property/COMPILE_FEATURES.cmake b/Tests/RunCMake/set_property/COMPILE_FEATURES.cmake new file mode 100644 index 0000000..1ab52ef --- /dev/null +++ b/Tests/RunCMake/set_property/COMPILE_FEATURES.cmake @@ -0,0 +1,2 @@ +include(Common.cmake) +test_target_property(COMPILE_FEATURES) diff --git a/Tests/RunCMake/set_property/COMPILE_OPTIONS-stdout.txt b/Tests/RunCMake/set_property/COMPILE_OPTIONS-stdout.txt new file mode 100644 index 0000000..f18451a --- /dev/null +++ b/Tests/RunCMake/set_property/COMPILE_OPTIONS-stdout.txt @@ -0,0 +1 @@ +-- Target COMPILE_OPTIONS is 'a;;b;c;;d;;e' diff --git a/Tests/RunCMake/set_property/COMPILE_OPTIONS.cmake b/Tests/RunCMake/set_property/COMPILE_OPTIONS.cmake new file mode 100644 index 0000000..da20ec8 --- /dev/null +++ b/Tests/RunCMake/set_property/COMPILE_OPTIONS.cmake @@ -0,0 +1,2 @@ +include(Common.cmake) +test_target_property(COMPILE_OPTIONS) diff --git a/Tests/RunCMake/set_property/Common.cmake b/Tests/RunCMake/set_property/Common.cmake new file mode 100644 index 0000000..b359487 --- /dev/null +++ b/Tests/RunCMake/set_property/Common.cmake @@ -0,0 +1,14 @@ +macro(test_target_property PROP) + add_custom_target(CustomTarget) + set_property(TARGET CustomTarget PROPERTY ${PROP} x) + set_property(TARGET CustomTarget PROPERTY ${PROP}) + set_property(TARGET CustomTarget APPEND PROPERTY ${PROP}) + set_property(TARGET CustomTarget PROPERTY ${PROP} a) + set_property(TARGET CustomTarget APPEND PROPERTY ${PROP} "") + set_property(TARGET CustomTarget APPEND PROPERTY ${PROP} b c) + set_property(TARGET CustomTarget APPEND PROPERTY ${PROP}) + set_property(TARGET CustomTarget APPEND PROPERTY ${PROP} "d;;e") + get_property(val TARGET CustomTarget PROPERTY ${PROP}) + message(STATUS "Target ${PROP} is '${val}'") + set_property(TARGET CustomTarget PROPERTY ${PROP}) +endmacro() diff --git a/Tests/RunCMake/set_property/INCLUDE_DIRECTORIES-stdout.txt b/Tests/RunCMake/set_property/INCLUDE_DIRECTORIES-stdout.txt new file mode 100644 index 0000000..f9970ce --- /dev/null +++ b/Tests/RunCMake/set_property/INCLUDE_DIRECTORIES-stdout.txt @@ -0,0 +1 @@ +-- Target INCLUDE_DIRECTORIES is 'a;;b;c;;d;;e' diff --git a/Tests/RunCMake/set_property/INCLUDE_DIRECTORIES.cmake b/Tests/RunCMake/set_property/INCLUDE_DIRECTORIES.cmake new file mode 100644 index 0000000..8f44aee --- /dev/null +++ b/Tests/RunCMake/set_property/INCLUDE_DIRECTORIES.cmake @@ -0,0 +1,2 @@ +include(Common.cmake) +test_target_property(INCLUDE_DIRECTORIES) diff --git a/Tests/RunCMake/set_property/LINK_LIBRARIES-stdout.txt b/Tests/RunCMake/set_property/LINK_LIBRARIES-stdout.txt new file mode 100644 index 0000000..1f7663b --- /dev/null +++ b/Tests/RunCMake/set_property/LINK_LIBRARIES-stdout.txt @@ -0,0 +1 @@ +-- Target LINK_LIBRARIES is 'a;;b;c;;d;;e' diff --git a/Tests/RunCMake/set_property/LINK_LIBRARIES.cmake b/Tests/RunCMake/set_property/LINK_LIBRARIES.cmake index 994e874..5155f59 100644 --- a/Tests/RunCMake/set_property/LINK_LIBRARIES.cmake +++ b/Tests/RunCMake/set_property/LINK_LIBRARIES.cmake @@ -1,7 +1,2 @@ -add_custom_target(CustomTarget) -set_property(TARGET CustomTarget PROPERTY LINK_LIBRARIES) -set_property(TARGET CustomTarget APPEND PROPERTY LINK_LIBRARIES) -get_property(val TARGET CustomTarget PROPERTY LINK_LIBRARIES) -if (NOT "${val}" STREQUAL "") - message(FATAL_ERROR "LINK_LIBRARIES value is '${val}' but should be ''") -endif() +include(Common.cmake) +test_target_property(LINK_LIBRARIES) diff --git a/Tests/RunCMake/set_property/RunCMakeTest.cmake b/Tests/RunCMake/set_property/RunCMakeTest.cmake index 54e63f7..37c7124 100644 --- a/Tests/RunCMake/set_property/RunCMakeTest.cmake +++ b/Tests/RunCMake/set_property/RunCMakeTest.cmake @@ -1,3 +1,9 @@ include(RunCMake) +run_cmake(COMPILE_DEFINITIONS) +run_cmake(COMPILE_FEATURES) +run_cmake(COMPILE_OPTIONS) +run_cmake(INCLUDE_DIRECTORIES) run_cmake(LINK_LIBRARIES) +run_cmake(SOURCES) +run_cmake(USER_PROP) diff --git a/Tests/RunCMake/set_property/SOURCES-stdout.txt b/Tests/RunCMake/set_property/SOURCES-stdout.txt new file mode 100644 index 0000000..921d5b1 --- /dev/null +++ b/Tests/RunCMake/set_property/SOURCES-stdout.txt @@ -0,0 +1 @@ +-- Target SOURCES is 'a;b;c;d;e' diff --git a/Tests/RunCMake/set_property/SOURCES.cmake b/Tests/RunCMake/set_property/SOURCES.cmake new file mode 100644 index 0000000..820641e --- /dev/null +++ b/Tests/RunCMake/set_property/SOURCES.cmake @@ -0,0 +1,2 @@ +include(Common.cmake) +test_target_property(SOURCES) diff --git a/Tests/RunCMake/set_property/USER_PROP-stdout.txt b/Tests/RunCMake/set_property/USER_PROP-stdout.txt new file mode 100644 index 0000000..eaf6e37 --- /dev/null +++ b/Tests/RunCMake/set_property/USER_PROP-stdout.txt @@ -0,0 +1 @@ +-- Target USER_PROP is 'a;b;c;d;;e' diff --git a/Tests/RunCMake/set_property/USER_PROP.cmake b/Tests/RunCMake/set_property/USER_PROP.cmake new file mode 100644 index 0000000..e1f88e1 --- /dev/null +++ b/Tests/RunCMake/set_property/USER_PROP.cmake @@ -0,0 +1,2 @@ +include(Common.cmake) +test_target_property(USER_PROP) -- cgit v0.12