From 9d3f45339a6a29631e6148ff1da894b6fd28747b Mon Sep 17 00:00:00 2001
From: Christopher Ferris <cferris@google.com>
Date: Tue, 28 Aug 2018 14:35:38 -0700
Subject: [PATCH] Set errno for all error return cases.

Test: Updated unit tests pass.
Change-Id: I2c1f14ed44ce144a46cb2866c146fca89a05539e
---
 base/include/android-base/parseint.h | 22 +++++++++++++----
 base/parseint_test.cpp               | 36 ++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/base/include/android-base/parseint.h b/base/include/android-base/parseint.h
index 5bfa47e8e..9444fddf0 100644
--- a/base/include/android-base/parseint.h
+++ b/base/include/android-base/parseint.h
@@ -38,6 +38,7 @@ bool ParseUint(const char* s, T* out, T max = std::numeric_limits<T>::max(),
   }
 
   if (s[0] == '-') {
+    errno = EINVAL;
     return false;
   }
 
@@ -45,14 +46,22 @@ bool ParseUint(const char* s, T* out, T max = std::numeric_limits<T>::max(),
   errno = 0;
   char* end;
   unsigned long long int result = strtoull(s, &end, base);
-  if (errno != 0 || end == s) return false;
+  if (errno != 0) return false;
+  if (end == s) {
+    errno = EINVAL;
+    return false;
+  }
   if (*end != '\0') {
     const char* suffixes = "bkmgtpe";
     const char* suffix;
-    if (!allow_suffixes || (suffix = strchr(suffixes, tolower(*end))) == nullptr) return false;
-    if (__builtin_mul_overflow(result, 1ULL << (10 * (suffix - suffixes)), &result)) return false;
+    if ((!allow_suffixes || (suffix = strchr(suffixes, tolower(*end))) == nullptr) ||
+        __builtin_mul_overflow(result, 1ULL << (10 * (suffix - suffixes)), &result)) {
+      errno = EINVAL;
+      return false;
+    }
   }
   if (max < result) {
+    errno = ERANGE;
     return false;
   }
   if (out != nullptr) {
@@ -95,10 +104,15 @@ bool ParseInt(const char* s, T* out,
   errno = 0;
   char* end;
   long long int result = strtoll(s, &end, base);
-  if (errno != 0 || s == end || *end != '\0') {
+  if (errno != 0) {
+    return false;
+  }
+  if (s == end || *end != '\0') {
+    errno = EINVAL;
     return false;
   }
   if (result < min || max < result) {
+    errno = ERANGE;
     return false;
   }
   if (out != nullptr) {
diff --git a/base/parseint_test.cpp b/base/parseint_test.cpp
index b8cf65468..e449c3345 100644
--- a/base/parseint_test.cpp
+++ b/base/parseint_test.cpp
@@ -16,15 +16,22 @@
 
 #include "android-base/parseint.h"
 
+#include <errno.h>
+
 #include <gtest/gtest.h>
 
 TEST(parseint, signed_smoke) {
+  errno = 0;
   int i = 0;
   ASSERT_FALSE(android::base::ParseInt("x", &i));
+  ASSERT_EQ(EINVAL, errno);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseInt("123x", &i));
+  ASSERT_EQ(EINVAL, errno);
 
   ASSERT_TRUE(android::base::ParseInt("123", &i));
   ASSERT_EQ(123, i);
+  ASSERT_EQ(0, errno);
   i = 0;
   EXPECT_TRUE(android::base::ParseInt("  123", &i));
   EXPECT_EQ(123, i);
@@ -40,26 +47,43 @@ TEST(parseint, signed_smoke) {
 
   ASSERT_TRUE(android::base::ParseInt("12", &i, 0, 15));
   ASSERT_EQ(12, i);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseInt("-12", &i, 0, 15));
+  ASSERT_EQ(ERANGE, errno);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseInt("16", &i, 0, 15));
+  ASSERT_EQ(ERANGE, errno);
 
+  errno = 0;
   ASSERT_FALSE(android::base::ParseInt<int>("x", nullptr));
+  ASSERT_EQ(EINVAL, errno);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseInt<int>("123x", nullptr));
+  ASSERT_EQ(EINVAL, errno);
   ASSERT_TRUE(android::base::ParseInt<int>("1234", nullptr));
 }
 
 TEST(parseint, unsigned_smoke) {
+  errno = 0;
   unsigned int i = 0u;
   ASSERT_FALSE(android::base::ParseUint("x", &i));
+  ASSERT_EQ(EINVAL, errno);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseUint("123x", &i));
+  ASSERT_EQ(EINVAL, errno);
 
   ASSERT_TRUE(android::base::ParseUint("123", &i));
   ASSERT_EQ(123u, i);
+  ASSERT_EQ(0, errno);
   i = 0u;
   EXPECT_TRUE(android::base::ParseUint("  123", &i));
   EXPECT_EQ(123u, i);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseUint("-123", &i));
+  EXPECT_EQ(EINVAL, errno);
+  errno = 0;
   EXPECT_FALSE(android::base::ParseUint("  -123", &i));
+  EXPECT_EQ(EINVAL, errno);
 
   unsigned short s = 0u;
   ASSERT_TRUE(android::base::ParseUint("1234", &s));
@@ -67,16 +91,28 @@ TEST(parseint, unsigned_smoke) {
 
   ASSERT_TRUE(android::base::ParseUint("12", &i, 15u));
   ASSERT_EQ(12u, i);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseUint("-12", &i, 15u));
+  ASSERT_EQ(EINVAL, errno);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseUint("16", &i, 15u));
+  ASSERT_EQ(ERANGE, errno);
 
+  errno = 0;
   ASSERT_FALSE(android::base::ParseUint<unsigned short>("x", nullptr));
+  ASSERT_EQ(EINVAL, errno);
+  errno = 0;
   ASSERT_FALSE(android::base::ParseUint<unsigned short>("123x", nullptr));
+  ASSERT_EQ(EINVAL, errno);
   ASSERT_TRUE(android::base::ParseUint<unsigned short>("1234", nullptr));
 
+  errno = 0;
   unsigned long long int lli;
   EXPECT_FALSE(android::base::ParseUint("-123", &lli));
+  EXPECT_EQ(EINVAL, errno);
+  errno = 0;
   EXPECT_FALSE(android::base::ParseUint("  -123", &lli));
+  EXPECT_EQ(EINVAL, errno);
 }
 
 TEST(parseint, no_implicit_octal) {