From: Dan Mihai Date: Tue, 15 Nov 2016 08:23:04 +0000 (-0800) Subject: [IOT-1561] Improve vs12_snprintf X-Git-Tag: 1.2.1~104 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b2a398ea5826f2af154ef668e19197bfdc5e7be7;p=platform%2Fupstream%2Fiotivity.git [IOT-1561] Improve vs12_snprintf Improve the snprintf surrogate used to build for Windows using Visual Studio 2013 or older. Also remove the dependency on cmake for building gtest for Windows, to fix a build break for the new snprintf_test.cpp. Change-Id: I70c9d0766a9a316a19cbd283671137e61ab88dbf Signed-off-by: Dan Mihai Reviewed-on: https://gerrit.iotivity.org/gerrit/14387 Reviewed-by: Kevin Kane Tested-by: jenkins-iotivity Reviewed-by: Dave Thaler --- diff --git a/extlibs/gtest/SConscript b/extlibs/gtest/SConscript index 9e17973..d127ccf 100644 --- a/extlibs/gtest/SConscript +++ b/extlibs/gtest/SConscript @@ -97,71 +97,18 @@ elif target_os == 'msys_nt': gtest_env.Configure(gtest_dir, 'mv libgtest_main.a lib/.libs') elif target_os == 'windows': - if os.path.exists(gtest_dir): - if gtest_env.get('RELEASE'): - output_dir = os.path.join(gtest_dir, 'Release') + os.sep - else: - output_dir = os.path.join(gtest_dir, 'Debug') + os.sep - - # Three parts to the gtest config string... - # 1. "Visual Studio" toolchain name. - # 2. VS Version + Year ("14 2015", "12 2013"). - # 3. Target Architecture ("Win64", "Win32"). - vs_version_year = "" - vs_target_arch = "" - vs_num = env['MSVC_VERSION'] - if "12.0" in vs_num: - vs_version_year = "Visual Studio 12 2013" - elif "14.0" in vs_num: - vs_version_year = "Visual Studio 14 2015" - else: - print "Error: unknown Visual Studio version %s" % vs_num - - vs_arch = env['TARGET_ARCH'] - if "amd64" in vs_arch: - vs_target_arch = "Win64" - elif "x86" in vs_arch: - vs_target_arch = "" - else: - print "Error: unknown Visual Studio target arch %s" % vs_arch - - vs_target_string = vs_version_year + " " + vs_target_arch - - # Exit if we didn't get a match for one of the above. - if not vs_version_year or not vs_target_arch: - Exit(1) - - if not gtest_env.WhereIs('cmake', gtest_env.get('PATH')): - print '''*********************** Error ************************ -* * -* Please make sure that CMake is in your System PATH * -* * -* You can download CMake from: * -* https://cmake.org/download/ * -* * -****************************************************** -''' - Exit(1) - - if not os.path.exists(gtest_lib_dir): - # Create lib dir - os.mkdir(gtest_lib_dir) - os.mkdir(gtest_dotlib_dir) - - # Run configure on gtest - print 'Configuring google unit test for compilation' - gtest_env.Configure(gtest_dir, 'cmake . -G"' + vs_target_string + '" -Dgtest_force_shared_crt=ON') - - # Run make on gtest - print 'Making google unit test' - gtest_env.Configure(gtest_dir, 'msbuild gtest.vcxproj') - gtest_env.Configure(gtest_dir, 'msbuild gtest_main.vcxproj') - - print 'Moving libraries to lib folder' - gtest_env.Configure(gtest_dir, 'copy '+output_dir+'gtest.lib %s' % gtest_lib_dir) - gtest_env.Configure(gtest_dir, 'move '+output_dir+'gtest.lib %s' % gtest_dotlib_dir ) - gtest_env.Configure(gtest_dir, 'copy '+output_dir+'gtest_main.lib %s' % gtest_lib_dir) - gtest_env.Configure(gtest_dir, 'move '+output_dir+'gtest_main.lib %s' % gtest_dotlib_dir) + # Avoid building the same StaticLibrary in more than one environment, by using the + # IOTIVITY_GTEST_HAS_BEEN_BUILT environment variable + if not env.has_key('IOTIVITY_GTEST_HAS_BEEN_BUILT'): + gtest_env.Append(CPPPATH = [ gtest_dir ]) + gtest = gtest_env.StaticLibrary(target = 'gtest', source = [ '%s/src/gtest-all.cc' % gtest_dir ]) + gtest_main = gtest_env.StaticLibrary(target = 'gtest_main', source = [ '%s/src/gtest_main.cc' % gtest_dir ]) + gtest_env.InstallTarget(gtest, 'gtest') + gtest_env.InstallTarget(gtest_main, 'gtest_main') + + vars = Variables(); + vars.AddVariables(('IOTIVITY_GTEST_HAS_BEEN_BUILT', '', '1')) + vars.Update(env) # Export flags once for all if target_os in targets_need_gtest: @@ -177,5 +124,7 @@ if target_os in targets_need_gtest: gtest_env.AppendUnique(CXXFLAGS = ['-pthread']) gtest_env.PrependUnique(LIBS = ['pthread']) gtest_env.PrependUnique(LIBS = ['gtest', 'gtest_main']) + if target_os in ['windows']: + gtest_env.AppendUnique(LINKFLAGS = ['/subsystem:CONSOLE']) Return('gtest_env') diff --git a/resource/c_common/windows/src/snprintf.c b/resource/c_common/windows/src/snprintf.c index c9c7de5..1a65930 100644 --- a/resource/c_common/windows/src/snprintf.c +++ b/resource/c_common/windows/src/snprintf.c @@ -23,19 +23,39 @@ int vs12_snprintf(char *buffer, size_t count, const char *format, ...) { - va_list args; + // These three values are invalid when calling _vsnprintf + if ((buffer == NULL) || (count == 0) || (format == NULL)) + { + return -1; + } + // _vsnprintf behaves similarly to snprintf, but: + // + // - It doesn't always add a zero terminator to the output buffer + // - When the output buffer is too small, it returns -1 instead of the + // required buffer size + va_list args; va_start(args, format); - int length = _vsnprintf(buffer, count, format, args); - va_end(args); - if (length > count) + if ((length == -1) || (length == count)) { + // Add the missing zero character terminator. buffer[count - 1] = '\0'; + + // When length == -1, it would be good to compute the required output + // buffer size, and return that computed value instead of -1. That would + // bring the behavior of vs12_snprintf closer to the C99 version of + // snprintf. However, IoTivity doesn't have an easy way to compute the + // required output buffer size. Also, there are no known callers of + // snprintf in IoTivity that are relying on a compliant return value, + // when that compliant return value would be larger than the 'count' + // input parameter. + // + // @todo Compute and return the output buffer size, instead of -1, + // when the output buffer was too small. } return length; } - diff --git a/resource/c_common/windows/test/SConscript b/resource/c_common/windows/test/SConscript new file mode 100644 index 0000000..fbffdfb --- /dev/null +++ b/resource/c_common/windows/test/SConscript @@ -0,0 +1,53 @@ +#****************************************************************** +# +# Copyright 2016 Microsoft +# +#-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +#-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= + +import os.path + +###################################################################### +# Build the gtest framework +###################################################################### +gtest_env = SConscript('#extlibs/gtest/SConscript') + +c_common_windows_test_env = gtest_env.Clone() + +###################################################################### +# Build flags +###################################################################### +c_common_windows_test_env.PrependUnique(CPPPATH = ['../include']) + +c_common_windows_test_env.AppendUnique(LIBPATH = [os.path.join(c_common_windows_test_env.get('BUILD_DIR'), 'resource', 'c_common')]) +c_common_windows_test_env.PrependUnique(LIBS = ['win_helper']) + +if c_common_windows_test_env.get('LOGGING'): + c_common_windows_test_env.AppendUnique(CPPDEFINES = ['TB_LOG']) + +###################################################################### +# Source files and Targets +###################################################################### +c_common_windows_tests = c_common_windows_test_env.Program('c_common_windows_tests', ['snprintf_test.cpp']) + +Alias("test", [c_common_windows_tests]) + +c_common_windows_test_env.AppendTarget('test') + +from tools.scons.RunTest import * +run_test(c_common_windows_test_env, + 'resource_c_common_windows_test.memcheck', + 'resource/c_common/windows/test/c_common_windows_tests') diff --git a/resource/c_common/windows/test/snprintf_test.cpp b/resource/c_common/windows/test/snprintf_test.cpp new file mode 100644 index 0000000..cce6276 --- /dev/null +++ b/resource/c_common/windows/test/snprintf_test.cpp @@ -0,0 +1,122 @@ +/* ***************************************************************** + * + * Copyright 2016 Microsoft + * + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * *****************************************************************/ + +#include +#include +#include "vs12_snprintf.h" + +void OutputBufferLargerThanNeeded(size_t outputBufferSize, size_t inputBufferSize, int callerLineNumber) +{ + ASSERT_GT(outputBufferSize, inputBufferSize); + ASSERT_GT(inputBufferSize, 1); + + char *inputBuffer = new char[inputBufferSize]; + memset(inputBuffer, 'i', inputBufferSize - 1); + inputBuffer[inputBufferSize - 1] = '\0'; + + char *outputBuffer = new char[outputBufferSize]; + memset(outputBuffer, 'o', outputBufferSize); + + EXPECT_EQ((inputBufferSize - 1), + vs12_snprintf(outputBuffer, outputBufferSize, "%s", inputBuffer)) + << "caller line number: " << callerLineNumber; + EXPECT_STREQ(inputBuffer, outputBuffer) << "caller line number: " << callerLineNumber; + + delete[] inputBuffer; + delete[] outputBuffer; +} + +void OutputBufferSmallerThanNeeded(size_t outputBufferSize, size_t inputBufferSize, int callerLineNumber) +{ + ASSERT_GT(inputBufferSize, outputBufferSize); + ASSERT_GT(outputBufferSize, 1); + + char *inputBuffer = new char[inputBufferSize]; + memset(inputBuffer, 'i', inputBufferSize - 1); + inputBuffer[inputBufferSize - 1] = '\0'; + + char *outputBuffer = new char[outputBufferSize]; + memset(outputBuffer, 'o', outputBufferSize); + + if (outputBufferSize == (inputBufferSize - 1)) + { + // When the output buffer size is just off by 1, the return value of + // vs12_snprintf is C99-compliant. + EXPECT_EQ((inputBufferSize - 1), + vs12_snprintf(outputBuffer, outputBufferSize, "%s", inputBuffer)) + << "caller line number: " << callerLineNumber; + } + else + { + // vs12_snprintf returns -1 in this case (a non C99-compliant value). + EXPECT_EQ(-1, + vs12_snprintf(outputBuffer, outputBufferSize, "%s", inputBuffer)) + << "caller line number: " << callerLineNumber; + } + + // snprintf output should be zero-terminated, even when the output buffer was too small. + EXPECT_EQ('\0', outputBuffer[outputBufferSize - 1]) << "caller line number: " << callerLineNumber; + + // Truncate the input buffer, then compare it with the output buffer. + inputBuffer[outputBufferSize - 1] = '\0'; + EXPECT_STREQ(inputBuffer, outputBuffer) << "caller line number: " << callerLineNumber; + + delete[] inputBuffer; + delete[] outputBuffer; +} + +TEST(vs12_snprintfTest, OutputBufferLargerThanNeeded1) +{ + OutputBufferLargerThanNeeded(10, 10 - 1, __LINE__); +} + +TEST(vs12_snprintfTest, OutputBufferLargerThanNeeded2) +{ + OutputBufferLargerThanNeeded(123, 123 - 2, __LINE__); +} + +TEST(vs12_snprintfTest, OutputBufferAsLargeAsNeeded) +{ + char outputBuffer[54]; + char inputBuffer[sizeof(outputBuffer)]; + + memset(inputBuffer, 'i', sizeof(inputBuffer) - 1); + inputBuffer[sizeof(inputBuffer) - 1] = '\0'; + + memset(outputBuffer, 'o', sizeof(outputBuffer)); + + EXPECT_EQ((sizeof(inputBuffer) - 1), + vs12_snprintf(outputBuffer, sizeof(outputBuffer), "%s", inputBuffer)); + EXPECT_STREQ(inputBuffer, outputBuffer); +} + +TEST(vs12_snprintfTest, OutputBufferSmallerThanNeeded1) +{ + OutputBufferSmallerThanNeeded(100, 100 + 1, __LINE__); +} + +TEST(vs12_snprintfTest, OutputBufferSmallerThanNeeded2) +{ + OutputBufferSmallerThanNeeded(123, 123 + 2, __LINE__); +} + +TEST(vs12_snprintfTest, OutputBufferSmallerThanNeeded3) +{ + OutputBufferSmallerThanNeeded(432, 2 * 432, __LINE__); +} diff --git a/resource/unit_tests.scons b/resource/unit_tests.scons index 40b1e79..fd18284 100644 --- a/resource/unit_tests.scons +++ b/resource/unit_tests.scons @@ -66,6 +66,9 @@ elif target_os == 'windows' and env.get('TEST') == '1': # get it and install it SConscript('#extlibs/gtest/SConscript') + # Build Common unit tests + SConscript('c_common/windows/test/SConscript') + # Build C stack's unit tests. SConscript('csdk/stack/test/SConscript') SConscript('csdk/connectivity/test/SConscript')