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.

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;
-
+    // It is an error for the scope number to be less than `-1`.
     if (scope_number[i] < -1) {
       EMSG(_(e_invarg));
       return;
     }
   }
 
-  // Allocate and initialize the string to return.
+  // Normalize scope, the number of the new scope will be 0.
-  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;
-    }
-  }
-
-  // If the deepest scope number is `-1` advance the scope.
   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;
   }
 }

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;
-
+#define MIN_CD_SCOPE  kCdScopeWindow
-/// Last `:cd` scope defined.
 #define MAX_CD_SCOPE  kCdScopeGlobal
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS