From: Dan Mihai Date: Tue, 15 Nov 2016 08:23:04 +0000 (-0800) Subject: [IOT-1561] Improve vs12_snprintf X-Git-Tag: 1.3.0~1052^2~27 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8125364b517a6b3f74d88fa1386bdeda9e8ab70e;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. This is a cherry-pick of commit b2a398e from the 1.2-rel branch. 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 Reviewed-on: https://gerrit.iotivity.org/gerrit/14627 --- 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 c018e28..ed516f9 100644 --- a/resource/unit_tests.scons +++ b/resource/unit_tests.scons @@ -38,11 +38,13 @@ if target_os in ['linux', 'windows']: # get it and install it SConscript('#extlibs/hippomocks.scons') - # Build Common unit tests + # Build Common unit tests SConscript('c_common/oic_string/test/SConscript') SConscript('c_common/oic_malloc/test/SConscript') SConscript('c_common/oic_time/test/SConscript') SConscript('c_common/ocrandom/test/SConscript') + if target_os == 'windows': + SConscript('c_common/windows/test/SConscript') # Build C unit tests SConscript('csdk/stack/test/SConscript')