Fix coverity errors in haslocaldir() and getcwd.

The Vim function `haslocaldir()` would crash if the users called it with
the two arguments `-1, -1`. Now it returns `0` in that case.

The coverity issue was complaining about a NULL dereference, but there
can never be a case where the pointer `tp` is NULL and being
dereferenced. An assertion has been put in place to satisfy coverity.

Furthermore the functions themselves have been cleaned up. First of all
the documentation comment for the different scopes has been extended and
a macro for the minimum scope has been introduced. In both functions any
time a scope is used as a range (e.g. in a loop) macros instead of
actuals scopes are used, that makes the functions more robust if new
scopes are added.

Second, in the implementation of `getcwd()` there was a superfluous
loop, it has been removed completely. I also changed all `goto end` to
plaing `return` statements by moving the allocation of `cwd` down, that
way there is no need for `goto` anymore.

2 files changed, 32 insertions, 38 deletions

diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 0ff70df54d..5323e17158 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -9820,7 +9820,7 @@ static void f_getcmdwintype(typval_T *argvars, typval_T *rettv)
 static void f_getcwd(typval_T *argvars, typval_T *rettv)
 {
   // Possible scope of working directory to return.
-  CdScope scope = kCdScopeWindow;
+  CdScope scope = MIN_CD_SCOPE;
 
   // Numbers of the scope objects (window, tab) we want the working directory
   // of. A `-1` means to skip this scope, a `0` means the current object.
@@ -9839,7 +9839,8 @@ static void f_getcwd(typval_T *argvars, typval_T *rettv)
   rettv->vval.v_string = NULL;
 
   // Pre-conditions and scope extraction together
-  for (int i = 0; i < kCdScopeGlobal; i++) {
+  for (int i = MIN_CD_SCOPE; i < MAX_CD_SCOPE; i++) {
+    // If there is no argument there are no more scopes after it, break out.
     if (argvars[i].v_type == VAR_UNKNOWN) {
       break;
     }
@@ -9850,34 +9851,17 @@ static void f_getcwd(typval_T *argvars, typval_T *rettv)
     scope_number[i] = argvars[i].vval.v_number;
     // The scope is the current iteration step.
     scope = i;
-
-    if (scope_number[i] < -1) {
-      EMSG(_(e_invarg));
-      return;
-    }
-  }
-
-  // Allocate and initialize the string to return.
-  cwd = xmalloc(MAXPATHL);
-
-  // Get the scope and numbers from the arguments
-  for (int i = 0; i < MAX_CD_SCOPE; i++) {
-    // If there is no argument there are no more scopes after it, break out.
-    if (argvars[i].v_type == VAR_UNKNOWN) {
-      break;
-    }
-    scope_number[i] = argvars[i].vval.v_number;
-    // The scope is the current iteration step.
-    scope = i;
     // It is an error for the scope number to be less than `-1`.
     if (scope_number[i] < -1) {
       EMSG(_(e_invarg));
-      goto end;
+      return;
     }
   }
 
-  // If the deepest scope number is `-1` advance the scope.
+  // Normalize scope, the number of the new scope will be 0.
   if (scope_number[scope] < 0) {
+    // Arguments to `getcwd` always end at second-highest scope, so scope will
+    // always be <= `MAX_CD_SCOPE`.
     scope++;
   }
 
@@ -9888,7 +9872,7 @@ static void f_getcwd(typval_T *argvars, typval_T *rettv)
     tp = find_tabpage(scope_number[kCdScopeTab]);
     if (!tp) {
       EMSG(_("E5000: Cannot find tab number."));
-      goto end;
+      return;
     }
   }
 
@@ -9898,25 +9882,29 @@ static void f_getcwd(typval_T *argvars, typval_T *rettv)
   } else if (scope_number[kCdScopeWindow] >= 0) {
     if (!tp) {
       EMSG(_("E5001: Higher scope cannot be -1 if lower scope is >= 0."));
-      goto end;
+      return;
     }
 
     if (scope_number[kCdScopeWindow] > 0) {
       win = find_win_by_nr(&argvars[0], curtab);
       if (!win) {
         EMSG(_("E5002: Cannot find window number."));
-        goto end;
+        return;
       }
     }
   }
 
+  cwd = xmalloc(MAXPATHL);
+
   switch (scope) {
     case kCdScopeWindow:
+      assert(win);
       from = win->w_localdir;
       if (from) {
         break;
       }
     case kCdScopeTab:  // FALLTHROUGH
+      assert(tp);
       from = tp->localdir;
       if (from) {
         break;
@@ -9926,7 +9914,7 @@ static void f_getcwd(typval_T *argvars, typval_T *rettv)
       if (globaldir) {
         from = globaldir;
       } else {
-        // Copy the OS path directly into output string and jump to the end.
+        // We have to copy the OS path directly into output string.
         if (os_dirname(cwd, MAXPATHL) == FAIL) {
           EMSG(_("E41: Could not display path."));
           goto end;
@@ -9943,7 +9931,6 @@ static void f_getcwd(typval_T *argvars, typval_T *rettv)
 #ifdef BACKSLASH_IN_FILENAME
   slash_adjust(rettv->vval.v_string);
 #endif
-
 end:
   xfree(cwd);
 }
@@ -10779,7 +10766,7 @@ static void f_has_key(typval_T *argvars, typval_T *rettv)
 static void f_haslocaldir(typval_T *argvars, typval_T *rettv)
 {
   // Possible scope of working directory to return.
-  CdScope scope = kCdScopeWindow;
+  CdScope scope = MIN_CD_SCOPE;
 
   // Numbers of the scope objects (window, tab) we want the working directory
   // of. A `-1` means to skip this scope, a `0` means the current object.
@@ -10795,7 +10782,7 @@ static void f_haslocaldir(typval_T *argvars, typval_T *rettv)
   rettv->vval.v_number = 0;
 
   // Pre-conditions and scope extraction together
-  for (int i = 0; i < kCdScopeGlobal; i++) {
+  for (int i = MIN_CD_SCOPE; i < MAX_CD_SCOPE; i++) {
     if (argvars[i].v_type == VAR_UNKNOWN) {
       break;
     }
@@ -10812,9 +10799,11 @@ static void f_haslocaldir(typval_T *argvars, typval_T *rettv)
     }
   }
 
-  // It the deepest scope number is `-1` advance the scope by one.
+  // Normalize scope, the number of the new scope will be 0.
   if (scope_number[scope] < 0) {
-    ++scope;
+    // Arguments to `haslocaldir` always end at second-highest scope, so scope
+    // will always be <= `MAX_CD_SCOPE`.
+    scope++;
   }
 
   // Find the tabpage by number
@@ -10848,13 +10837,16 @@ static void f_haslocaldir(typval_T *argvars, typval_T *rettv)
 
   switch (scope) {
     case kCdScopeWindow:
+      assert(win);
       rettv->vval.v_number = win->w_localdir ? 1 : 0;
       break;
     case kCdScopeTab:
+      assert(tp);
       rettv->vval.v_number = tp->localdir ? 1 : 0;
       break;
     case kCdScopeGlobal:
-      assert(0);
+      // The global scope never has a local directory
+      rettv->vval.v_number = 0;
       break;
   }
 }
diff --git a/src/nvim/ex_docmd.h b/src/nvim/ex_docmd.h
index 7af3ee233c..dbfc64e2f1 100644
--- a/src/nvim/ex_docmd.h
+++ b/src/nvim/ex_docmd.h
@@ -19,16 +19,18 @@
 #define EXMODE_NORMAL           1
 #define EXMODE_VIM              2
 
-/// The scope of a command.
+/// The scope of a working-directory command like `:cd`.
 ///
-/// The lower a number, the deeper the scope.
+/// Scopes are enumerated from lowest to highest. When adding a scope make sure
+/// to update all functions using scopes as well, such as the implementation of
+/// `getcwd()`. When using scopes as limits (e.g. in loops) don't use the scopes
+/// directly, use `MIN_CD_SCOPE` and `MAX_CD_SCOPE` instead.
 typedef enum {
   kCdScopeWindow,  ///< Affects one window.
   kCdScopeTab,     ///< Affects one tab page.
-  kCdScopeGlobal,  ///< Affects the entire instance of NeoVim.
+  kCdScopeGlobal,  ///< Affects the entire instance of Neovim.
 } CdScope;
-
-/// Last `:cd` scope defined.
+#define MIN_CD_SCOPE  kCdScopeWindow
 #define MAX_CD_SCOPE  kCdScopeGlobal
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS