From 9f725bee1f59df961e93e3d2392bc779281d7de7 Mon Sep 17 00:00:00 2001
From: Kevin Ballard <kevin@sb.org>
Date: Sun, 13 Jul 2014 19:08:15 -0700
Subject: [PATCH] set: Print an error when setting `umask` to a bad value

Repurpose the ENV_INVALID return value for env_set(), which wasn't
currently used by anything. When a bad value is passed for the 'umask'
key, return ENV_INVALID to signal this and print a good error message
from the `set` builtin.

This makes `set umask foo` properly produce an error.
---
 builtin_set.cpp | 2 +-
 env.cpp         | 6 ++++--
 env.h           | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin_set.cpp b/builtin_set.cpp
index af257d160..eb4f5eda5 100644
--- a/builtin_set.cpp
+++ b/builtin_set.cpp
@@ -166,7 +166,7 @@ static int my_env_set(const wchar_t *key, const wcstring_list_t &val, int scope)
 
         case ENV_INVALID:
         {
-            append_format(stderr_buffer, _(L"%ls: Unknown error"), L"set");
+            append_format(stderr_buffer, _(L"%ls: Tried to set the special variable '%ls' to an invalid value\n"), L"set", key);
             retcode=1;
             break;
         }
diff --git a/env.cpp b/env.cpp
index 88d61c857..91bc7ac80 100644
--- a/env.cpp
+++ b/env.cpp
@@ -655,10 +655,12 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode)
             if (!errno && (!*end) && (mask <= 0777) && (mask >= 0))
             {
                 umask(mask);
+                /* Do not actually create a umask variable, on env_get, it will be calculated dynamically */
+                return 0;
             }
         }
-        /* Do not actually create a umask variable, on env_get, it will be calculated dynamically */
-        return 0;
+
+        return ENV_INVALID;
     }
 
     /*
diff --git a/env.h b/env.h
index 7afa9b9cd..8d4a41a9c 100644
--- a/env.h
+++ b/env.h
@@ -80,7 +80,7 @@ void env_init(const struct config_paths_t *paths = NULL);
 
    * ENV_PERM, can only be returned when setting as a user, e.g. ENV_USER is set. This means that the user tried to change a read-only variable.
    * ENV_SCOPE, the variable cannot be set in the given scope. This applies to readonly/electric variables set from the local or universal scopes, or set as exported.
-   * ENV_INVALID, the variable name or mode was invalid
+   * ENV_INVALID, the variable value was invalid. This applies only to special variables.
 */
 
 int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t mode);