> You could check the value of the CMAKE_MINIMUM_REQUIRED_VERSION
> variable to decide whether to make it an error.  Extend the error
> message to mention that it could be fixed by requiring a new
> enough version of CMake.  OTOH, there have been many times in the
> past where we've converted an error case to a non-error case
> without such a check.  Projects should test themselves with the
> oldest CMake they support anyway.  I have no strong feeling on
> this requirement.

I have decided against the checking for back compatibility as it would
only make writing of modules that come with CMake harder.

Attached is the final patch. I have changed the already existing tests
and added a note regarding the change in documentation. If this patch
will not make it into CMake 3.1 the note in documentation should be
changed before applying the patch.

Please review the patch and if there are no changes required add it to
the repository.

Thanks,
Domen
From f2e76f350e094beff1b2d8d86fa7298a87badb36 Mon Sep 17 00:00:00 2001
From: Domen Vrankar <domen.vran...@gmail.com>
Date: Wed, 12 Nov 2014 23:57:16 +0100
Subject: [PATCH] string SUBSTRING length exceeding end index

string SUBSTRING command now ignores length if it points
past end of string and uses end of string instead.
String SUBSTRING tests now cover more corner cases.
---
 Help/command/string.rst                 |  5 ++++-
 Source/cmStringCommand.cxx              |  6 ++----
 Tests/CMakeTests/StringTest.cmake.in    |  4 ++--
 Tests/CMakeTests/StringTestScript.cmake | 11 +++++++----
 4 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/Help/command/string.rst b/Help/command/string.rst
index 07d0ff3..eb5d803 100644
--- a/Help/command/string.rst
+++ b/Help/command/string.rst
@@ -73,8 +73,11 @@ TOUPPER/TOLOWER will convert string to upper/lower characters.
 
 LENGTH will return a given string's length.
 
-SUBSTRING will return a substring of a given string.  If length is -1
+SUBSTRING will return a substring of a given string. If length is -1
 the remainder of the string starting at begin will be returned.
+If string is shorter than length then end of string is used instead.
+NOTE: Before CMake 3.1 an error was reported if length pointed past
+the end of string.
 
 STRIP will return a substring of a given string with leading and
 trailing spaces removed.
diff --git a/Source/cmStringCommand.cxx b/Source/cmStringCommand.cxx
index 90a8f85..ce7c854 100644
--- a/Source/cmStringCommand.cxx
+++ b/Source/cmStringCommand.cxx
@@ -711,12 +711,10 @@ bool cmStringCommand::HandleSubstringCommand(std::vector<std::string> const&
     this->SetError(ostr.str());
     return false;
     }
-  int leftOverLength = intStringLength - begin;
-  if ( end < -1 || end > leftOverLength )
+  if ( end < -1 )
     {
     cmOStringStream ostr;
-    ostr << "end index: " << end << " is out of range -1 - "
-         << leftOverLength;
+    ostr << "end index: " << end << " should be -1 or greater";
     this->SetError(ostr.str());
     return false;
     }
diff --git a/Tests/CMakeTests/StringTest.cmake.in b/Tests/CMakeTests/StringTest.cmake.in
index a9fe428..92e70c3 100644
--- a/Tests/CMakeTests/StringTest.cmake.in
+++ b/Tests/CMakeTests/StringTest.cmake.in
@@ -63,7 +63,7 @@ check_cmake_test(String
 # Execute each test listed in StringTestScript.cmake:
 #
 set(scriptname "@CMAKE_CURRENT_SOURCE_DIR@/StringTestScript.cmake")
-set(number_of_tests_expected 69)
+set(number_of_tests_expected 70)
 
 include("@CMAKE_CURRENT_SOURCE_DIR@/ExecuteScriptTests.cmake")
 execute_all_script_tests(${scriptname} number_of_tests_executed)
@@ -75,6 +75,6 @@ message(STATUS "scriptname='${scriptname}'")
 message(STATUS "number_of_tests_executed='${number_of_tests_executed}'")
 message(STATUS "number_of_tests_expected='${number_of_tests_expected}'")
 
-if(number_of_tests_executed LESS number_of_tests_expected)
+if(NOT number_of_tests_executed EQUAL number_of_tests_expected)
   message(FATAL_ERROR "error: some test cases were skipped")
 endif()
diff --git a/Tests/CMakeTests/StringTestScript.cmake b/Tests/CMakeTests/StringTestScript.cmake
index a562e71..44d5653 100644
--- a/Tests/CMakeTests/StringTestScript.cmake
+++ b/Tests/CMakeTests/StringTestScript.cmake
@@ -122,14 +122,17 @@ elseif(testname STREQUAL substring_not_enough_args) # fail
 elseif(testname STREQUAL substring_begin_too_large) # fail
   string(SUBSTRING "abcdefg" 25 100 v)
 
-elseif(testname STREQUAL substring_end_too_large) # fail
+elseif(testname STREQUAL substring_end_larger_than_strlen) # pass
   string(SUBSTRING "abcdefg" 1 100 v)
 
 elseif(testname STREQUAL substring_begin_less_than_zero) # fail
-  string(SUBSTRING "abcdefg" -2 4 v)
+  string(SUBSTRING "abcdefg" -1 4 v)
 
-elseif(testname STREQUAL substring_end_less_than_begin) # fail
-  string(SUBSTRING "abcdefg" 6 3 v)
+elseif(testname STREQUAL substring_end_less_than_zero) # pass
+  string(SUBSTRING "abcdefg" 0 -1 v)
+
+elseif(testname STREQUAL substring_end_less_than_begin) # pass
+  string(SUBSTRING "abcdefg" 6 0 v)
 
 elseif(testname STREQUAL length_not_enough_args) # fail
   string(LENGTH)
-- 
2.1.0

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to