diff options
author | yusukes@chromium.org <yusukes@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-02 23:06:26 +0000 |
---|---|---|
committer | yusukes@chromium.org <yusukes@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-02 23:06:26 +0000 |
commit | 34d58a296e3b0d3f81f4692a48bc54ec3128c850 (patch) | |
tree | fb059cdc70f298603e3a05fc037008571be2e776 /base | |
parent | 256513322f97e767c51012f3fb69321e2169b885 (diff) | |
download | chromium_src-34d58a296e3b0d3f81f4692a48bc54ec3128c850.zip chromium_src-34d58a296e3b0d3f81f4692a48bc54ec3128c850.tar.gz chromium_src-34d58a296e3b0d3f81f4692a48bc54ec3128c850.tar.bz2 |
Stop overwriting errno in base::StringPrintf, StringAppendV, and StringToDouble.
BUG=None
TEST=new tests added
Review URL: https://chromiumcodereview.appspot.com/14749003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@198001 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base')
-rw-r--r-- | base/base.gyp | 1 | ||||
-rw-r--r-- | base/process_util_mac.mm | 20 | ||||
-rw-r--r-- | base/scoped_clear_errno.h | 34 | ||||
-rw-r--r-- | base/scoped_clear_errno_unittest.cc | 30 | ||||
-rw-r--r-- | base/stringprintf.cc | 3 | ||||
-rw-r--r-- | base/stringprintf_unittest.cc | 12 | ||||
-rw-r--r-- | base/strings/string_number_conversions.cc | 5 | ||||
-rw-r--r-- | base/strings/string_number_conversions_unittest.cc | 4 |
8 files changed, 88 insertions, 21 deletions
diff --git a/base/base.gyp b/base/base.gyp index 2b089db..44ff1d2 100644 --- a/base/base.gyp +++ b/base/base.gyp @@ -562,6 +562,7 @@ 'rand_util_unittest.cc', 'safe_numerics_unittest.cc', 'safe_numerics_unittest.nc', + 'scoped_clear_errno_unittest.cc', 'scoped_native_library_unittest.cc', 'scoped_observer.h', 'security_unittest.cc', diff --git a/base/process_util_mac.mm b/base/process_util_mac.mm index b4ccd4a..e0174b4 100644 --- a/base/process_util_mac.mm +++ b/base/process_util_mac.mm @@ -32,6 +32,7 @@ #include "base/mac/mac_util.h" #include "base/mac/scoped_mach_port.h" #include "base/posix/eintr_wrapper.h" +#include "base/scoped_clear_errno.h" #include "base/string_util.h" #include "base/sys_info.h" #include "third_party/apple_apsl/CFBase.h" @@ -546,25 +547,6 @@ malloc_error_break_t LookUpMallocErrorBreak() { return reinterpret_cast<malloc_error_break_t>(reference_addr); } -// Simple scoper that saves the current value of errno, resets it to 0, and on -// destruction puts the old value back. This is so that CrMallocErrorBreak can -// safely test errno free from the effects of other routines. -class ScopedClearErrno { - public: - ScopedClearErrno() : old_errno_(errno) { - errno = 0; - } - ~ScopedClearErrno() { - if (errno == 0) - errno = old_errno_; - } - - private: - int old_errno_; - - DISALLOW_COPY_AND_ASSIGN(ScopedClearErrno); -}; - // Combines ThreadLocalBoolean with AutoReset. It would be convenient // to compose ThreadLocalPointer<bool> with base::AutoReset<bool>, but that // would require allocating some storage for the bool. diff --git a/base/scoped_clear_errno.h b/base/scoped_clear_errno.h new file mode 100644 index 0000000..7b972fc --- /dev/null +++ b/base/scoped_clear_errno.h @@ -0,0 +1,34 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef BASE_SCOPED_CLEAR_ERRNO_H_ +#define BASE_SCOPED_CLEAR_ERRNO_H_ + +#include <errno.h> + +#include "base/basictypes.h" + +namespace base { + +// Simple scoper that saves the current value of errno, resets it to 0, and on +// destruction puts the old value back. +class ScopedClearErrno { + public: + ScopedClearErrno() : old_errno_(errno) { + errno = 0; + } + ~ScopedClearErrno() { + if (errno == 0) + errno = old_errno_; + } + + private: + const int old_errno_; + + DISALLOW_COPY_AND_ASSIGN(ScopedClearErrno); +}; + +} // namespace base + +#endif // BASE_SCOPED_CLEAR_ERRNO_H_ diff --git a/base/scoped_clear_errno_unittest.cc b/base/scoped_clear_errno_unittest.cc new file mode 100644 index 0000000..8afb33e --- /dev/null +++ b/base/scoped_clear_errno_unittest.cc @@ -0,0 +1,30 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include <errno.h> + +#include "base/scoped_clear_errno.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace base { + +TEST(ScopedClearErrno, TestNoError) { + errno = 1; + { + ScopedClearErrno clear_error; + EXPECT_EQ(0, errno); + } + EXPECT_EQ(1, errno); +} + +TEST(ScopedClearErrno, TestError) { + errno = 1; + { + ScopedClearErrno clear_error; + errno = 2; + } + EXPECT_EQ(2, errno); +} + +} // namespace base diff --git a/base/stringprintf.cc b/base/stringprintf.cc index 79c6562..814fe99 100644 --- a/base/stringprintf.cc +++ b/base/stringprintf.cc @@ -6,6 +6,7 @@ #include <errno.h> +#include "base/scoped_clear_errno.h" #include "base/string_util.h" #include "base/utf_string_conversions.h" @@ -50,7 +51,7 @@ static void StringAppendVT(StringType* dst, GG_VA_COPY(ap_copy, ap); #if !defined(OS_WIN) - errno = 0; + ScopedClearErrno clear_errno; #endif int result = vsnprintfT(stack_buf, arraysize(stack_buf), format, ap_copy); va_end(ap_copy); diff --git a/base/stringprintf_unittest.cc b/base/stringprintf_unittest.cc index 305d24a..c04b17e 100644 --- a/base/stringprintf_unittest.cc +++ b/base/stringprintf_unittest.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include <errno.h> + #include "base/basictypes.h" #include "base/stringprintf.h" #include "testing/gtest/include/gtest/gtest.h" @@ -172,4 +174,14 @@ TEST(StringPrintfTest, PositionalParameters) { #endif } +// Test that StringPrintf and StringAppendV do not change errno. +TEST(StringPrintfTest, StringPrintfErrno) { + errno = 1; + EXPECT_EQ("", StringPrintf("%s", "")); + EXPECT_EQ(1, errno); + std::string out; + StringAppendVTestHelper(&out, "%d foo %s", 1, "bar"); + EXPECT_EQ(1, errno); +} + } // namespace base diff --git a/base/strings/string_number_conversions.cc b/base/strings/string_number_conversions.cc index c9b1115..39daece 100644 --- a/base/strings/string_number_conversions.cc +++ b/base/strings/string_number_conversions.cc @@ -12,6 +12,7 @@ #include <limits> #include "base/logging.h" +#include "base/scoped_clear_errno.h" #include "base/third_party/dmg_fp/dmg_fp.h" #include "base/utf_string_conversions.h" @@ -450,7 +451,9 @@ bool StringToSizeT(const StringPiece16& input, size_t* output) { } bool StringToDouble(const std::string& input, double* output) { - errno = 0; // Thread-safe? It is on at least Mac, Linux, and Windows. + // Thread-safe? It is on at least Mac, Linux, and Windows. + ScopedClearErrno clear_errno; + char* endptr = NULL; *output = dmg_fp::strtod(input.c_str(), &endptr); diff --git a/base/strings/string_number_conversions_unittest.cc b/base/strings/string_number_conversions_unittest.cc index 4d413d7..5ae59d3 100644 --- a/base/strings/string_number_conversions_unittest.cc +++ b/base/strings/string_number_conversions_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include <errno.h> #include <math.h> #include <limits> @@ -449,7 +450,10 @@ TEST(StringNumberConversionsTest, StringToDouble) { for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) { double output; + errno = 1; EXPECT_EQ(cases[i].success, StringToDouble(cases[i].input, &output)); + if (cases[i].success) + EXPECT_EQ(1, errno) << i; // confirm that errno is unchanged. EXPECT_DOUBLE_EQ(cases[i].output, output); } |