From 26a9ae3f5f0f8a563ce108e757b327b0dd9beae7 Mon Sep 17 00:00:00 2001
From: Yi Jin <yijin@fb.com>
Date: Wed, 19 Dec 2018 16:45:42 -0800
Subject: [PATCH] refactor readU32FromChar(...), improve init_cLevel(...), and
 add env var ZSTD_CLEVEL tests

---
 programs/zstdcli.c | 57 ++++++++++++++++++++++++++++++++--------------
 tests/playTests.sh | 12 ++++++++++
 2 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/programs/zstdcli.c b/programs/zstdcli.c
index 9737ae021..d892c6aa7 100644
--- a/programs/zstdcli.c
+++ b/programs/zstdcli.c
@@ -234,32 +234,44 @@ static void errorOut(const char* msg)
     DISPLAY("%s \n", msg); exit(1);
 }
 
-/*! readU32FromChar() :
- * @return : unsigned integer value read from input in `char` format.
+/*! _readU32FromChar() :
+ * @return 0 if success, and store the result in *value.
  *  allows and interprets K, KB, KiB, M, MB and MiB suffix.
  *  Will also modify `*stringPtr`, advancing it to position where it stopped reading.
- *  Note : function will exit() program if digit sequence overflows */
-static unsigned readU32FromChar(const char** stringPtr)
+ * @return 1 if an overflow error occurs */
+static int _readU32FromChar(const char** stringPtr, unsigned* value)
 {
-    const char errorMsg[] = "error: numeric value too large";
+    static unsigned const max = (((unsigned)(-1)) / 10) - 1;
     unsigned result = 0;
     while ((**stringPtr >='0') && (**stringPtr <='9')) {
-        unsigned const max = (((unsigned)(-1)) / 10) - 1;
-        if (result > max) errorOut(errorMsg);
+        if (result > max) return 1; // overflow error
         result *= 10, result += **stringPtr - '0', (*stringPtr)++ ;
     }
     if ((**stringPtr=='K') || (**stringPtr=='M')) {
         unsigned const maxK = ((unsigned)(-1)) >> 10;
-        if (result > maxK) errorOut(errorMsg);
+        if (result > maxK) return 1; // overflow error
         result <<= 10;
         if (**stringPtr=='M') {
-            if (result > maxK) errorOut(errorMsg);
+            if (result > maxK) return 1; // overflow error
             result <<= 10;
         }
         (*stringPtr)++;  /* skip `K` or `M` */
         if (**stringPtr=='i') (*stringPtr)++;
         if (**stringPtr=='B') (*stringPtr)++;
     }
+    *value = result;
+    return 0;
+}
+
+/*! readU32FromChar() :
+ * @return : unsigned integer value read from input in `char` format.
+ *  allows and interprets K, KB, KiB, M, MB and MiB suffix.
+ *  Will also modify `*stringPtr`, advancing it to position where it stopped reading.
+ *  Note : function will exit() program if digit sequence overflows */
+static unsigned readU32FromChar(const char** stringPtr) {
+    static const char errorMsg[] = "error: numeric value too large";
+    unsigned result;
+    if (_readU32FromChar(stringPtr, &result)) { errorOut(errorMsg); }
     return result;
 }
 
@@ -458,18 +470,28 @@ static void printVersion(void)
 
 /* functions that pick up environment variables */
 int init_cLevel() {
-    const char *env = getenv(ENV_CLEVEL);
+    const char* const env = getenv(ENV_CLEVEL);
     if (env) {
+        const char *ptr = env;
         int sign = 1;
-        if (*env == '-') {
+        if (*ptr == '-') {
             sign = -1;
-            env++;
-        } else if (*env == '+') {
-            env++;
+            ptr++;
+        } else if (*ptr == '+') {
+            ptr++;
         }
 
-        if ((*env>='0') && (*env<='9'))
-            return sign * readU32FromChar(&env);
+        if ((*ptr>='0') && (*ptr<='9')) {
+            unsigned absLevel;
+            if (_readU32FromChar(&ptr, &absLevel)) { 
+                DISPLAYLEVEL(2, "Ignore environment variable %s=%s: numeric value too large\n", ENV_CLEVEL, env);
+                return ZSTDCLI_CLEVEL_DEFAULT;
+            } else if (*ptr == 0) {
+                return sign * absLevel;
+            }
+        }
+
+        DISPLAYLEVEL(2, "Ignore environment variable %s=%s: not a valid integer value\n", ENV_CLEVEL, env);
     }
 
     return ZSTDCLI_CLEVEL_DEFAULT;
@@ -490,6 +512,8 @@ typedef enum { zom_compress, zom_decompress, zom_test, zom_bench, zom_train, zom
 
 int main(int argCount, const char* argv[])
 {
+    g_displayOut = stderr;
+
     int argNb,
         followLinks = 0,
         forceStdout = 0,
@@ -550,7 +574,6 @@ int main(int argCount, const char* argv[])
     (void)memLimit;   /* not used when ZSTD_NODECOMPRESS set */
     if (filenameTable==NULL) { DISPLAY("zstd: %s \n", strerror(errno)); exit(1); }
     filenameTable[0] = stdinmark;
-    g_displayOut = stderr;
     programName = lastNameFromPath(programName);
 #ifdef ZSTD_MULTITHREAD
     nbWorkers = 1;
diff --git a/tests/playTests.sh b/tests/playTests.sh
index 7758f46e9..cb4c6d5ae 100755
--- a/tests/playTests.sh
+++ b/tests/playTests.sh
@@ -122,6 +122,18 @@ $ZSTD --fast=5000000000 -f tmp && die "too large numeric value : must fail"
 $ZSTD -c --fast=0 tmp > $INTOVOID && die "--fast must not accept value 0"
 $ECHO "test : too large numeric argument"
 $ZSTD --fast=9999999999 -f tmp  && die "should have refused numeric value"
+$ECHO "test : set compression level with environment variable ZSTD_CLEVEL"
+ZSTD_CLEVEL=12  $ZSTD -f tmp # positive compression level
+ZSTD_CLEVEL=-12 $ZSTD -f tmp # negative compression level
+ZSTD_CLEVEL=+12 $ZSTD -f tmp # valid: verbose '+' sign 
+ZSTD_CLEVEL=    $ZSTD -f tmp # empty env var, warn and revert to default setting
+ZSTD_CLEVEL=-   $ZSTD -f tmp # malformed env var, warn and revert to default setting
+ZSTD_CLEVEL=a   $ZSTD -f tmp # malformed env var, warn and revert to default setting
+ZSTD_CLEVEL=+a  $ZSTD -f tmp # malformed env var, warn and revert to default setting
+ZSTD_CLEVEL=3a7 $ZSTD -f tmp # malformed env var, warn and revert to default setting
+ZSTD_CLEVEL=50000000000  $ZSTD -f tmp # numeric value too large, warn and revert to default setting
+$ECHO "test : override ZSTD_CLEVEL with command line option"
+ZSTD_CLEVEL=12  $ZSTD --fast=3 -f tmp # overridden by command line option 
 $ECHO "test : compress to stdout"
 $ZSTD tmp -c > tmpCompressed
 $ZSTD tmp --stdout > tmpCompressed       # long command format