win/pyt: cleanup

2 files changed, 173 insertions, 113 deletions

diff --git a/src/nvim/os/pty_process_win.c b/src/nvim/os/pty_process_win.c
index f3efe87d78..6a08e2868b 100644
--- a/src/nvim/os/pty_process_win.c
+++ b/src/nvim/os/pty_process_win.c
@@ -12,19 +12,6 @@
 # include "os/pty_process_win.c.generated.h"
 #endif
 
-static void wait_eof_timer_cb(uv_timer_t *wait_eof_timer)
-  FUNC_ATTR_NONNULL_ALL
-{
-  PtyProcess *ptyproc =
-    (PtyProcess *)((uv_handle_t *)wait_eof_timer->data);
-  Process *proc = (Process *)ptyproc;
-
-  if (!proc->out || !uv_is_readable(proc->out->uvstream)) {
-    uv_timer_stop(&ptyproc->wait_eof_timer);
-    pty_process_finish2(ptyproc);
-  }
-}
-
 static void CALLBACK pty_process_finish1(void *context, BOOLEAN unused)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -36,6 +23,7 @@ static void CALLBACK pty_process_finish1(void *context, BOOLEAN unused)
   uv_timer_start(&ptyproc->wait_eof_timer, wait_eof_timer_cb, 200, 200);
 }
 
+/// @returns zero on sucess, or error code of winpty or MultiByteToWideChar.
 int pty_process_spawn(PtyProcess *ptyproc)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -44,34 +32,39 @@ int pty_process_spawn(PtyProcess *ptyproc)
   winpty_error_ptr_t err = NULL;
   winpty_config_t *cfg = NULL;
   winpty_spawn_config_t *spawncfg = NULL;
-  winpty_t *wp = NULL;
-  char *in_name = NULL, *out_name = NULL;
+  winpty_t *winpty_object = NULL;
+  char *in_name = NULL;
+  char *out_name = NULL;
   HANDLE process_handle = NULL;
-  uv_connect_t *in_req = NULL, *out_req = NULL;
-  wchar_t *cmdline = NULL, *cwd = NULL;
+  uv_connect_t *in_req = NULL;
+  uv_connect_t *out_req = NULL;
+  wchar_t *cmd_line = NULL;
+  wchar_t *cwd = NULL;
 
   assert(!proc->err);
 
-  if (!(cfg = winpty_config_new(
-      WINPTY_FLAG_ALLOW_CURPROC_DESKTOP_CREATION, &err))) {
+  cfg = winpty_config_new(WINPTY_FLAG_ALLOW_CURPROC_DESKTOP_CREATION, &err);
+  if (cfg == NULL) {
     goto cleanup;
   }
-  winpty_config_set_initial_size(
-      cfg,
-      ptyproc->width,
-      ptyproc->height);
 
-  if (!(wp = winpty_open(cfg, &err))) {
+  winpty_config_set_initial_size(cfg, ptyproc->width, ptyproc->height);
+  winpty_object = winpty_open(cfg, &err);
+  if (winpty_object == NULL) {
     goto cleanup;
   }
 
-  if ((status = utf16_to_utf8(winpty_conin_name(wp), &in_name))) {
+  status = utf16_to_utf8(winpty_conin_name(winpty_object), &in_name);
+  if (status != 0) {
     goto cleanup;
   }
-  if ((status = utf16_to_utf8(winpty_conout_name(wp), &out_name))) {
+
+  status = utf16_to_utf8(winpty_conout_name(winpty_object), &out_name);
+  if (status != 0) {
     goto cleanup;
   }
-  if (proc->in) {
+
+  if (proc->in != NULL) {
     in_req = xmalloc(sizeof(uv_connect_t));
     uv_pipe_connect(
         in_req,
@@ -79,7 +72,8 @@ int pty_process_spawn(PtyProcess *ptyproc)
         in_name,
         pty_process_connect_cb);
   }
-  if (proc->out) {
+
+  if (proc->out != NULL) {
     out_req = xmalloc(sizeof(uv_connect_t));
     uv_pipe_connect(
         out_req,
@@ -88,46 +82,65 @@ int pty_process_spawn(PtyProcess *ptyproc)
         pty_process_connect_cb);
   }
 
-  if (proc->cwd != NULL && (status = utf8_to_utf16(proc->cwd, &cwd))) {
-    goto cleanup;
+  if (proc->cwd != NULL) {
+    status = utf8_to_utf16(proc->cwd, &cwd);
+    if (status != 0) {
+      goto cleanup;
+    }
   }
-  if ((status = build_cmdline(proc->argv, &cmdline))) {
+
+  status = build_cmd_line(proc->argv, &cmd_line);
+  if (status != 0) {
     goto cleanup;
   }
-  if (!(spawncfg = winpty_spawn_config_new(
+
+  spawncfg = winpty_spawn_config_new(
       WINPTY_SPAWN_FLAG_AUTO_SHUTDOWN,
-      NULL, cmdline, cwd, NULL, &err))) {
+      NULL,  // Optional application name
+      cmd_line,
+      cwd,
+      NULL,  // Optional environment variables
+      &err);
+  if (spawncfg == NULL) {
     goto cleanup;
   }
-  if (!winpty_spawn(wp, spawncfg, &process_handle, NULL, NULL, &err)) {
+
+  if (!winpty_spawn(winpty_object,
+                    spawncfg,
+                    &process_handle,
+                    NULL,  // Optional thread handle
+                    NULL,  // Optional create process error
+                    &err)) {
     goto cleanup;
   }
   proc->pid = GetProcessId(process_handle);
 
   if (!RegisterWaitForSingleObject(
       &ptyproc->finish_wait,
-      process_handle, pty_process_finish1, ptyproc,
-      INFINITE, WT_EXECUTEDEFAULT | WT_EXECUTEONLYONCE)) {
+      process_handle,
+      pty_process_finish1,
+      ptyproc,
+      INFINITE,
+      WT_EXECUTEDEFAULT | WT_EXECUTEONLYONCE)) {
     abort();
   }
 
-  while ((in_req && in_req->handle) || (out_req && out_req->handle)) {
+  // Wait until pty_process_connect_cb is called.
+  while ((in_req != NULL && in_req->handle != NULL)
+         || (out_req != NULL && out_req->handle != NULL)) {
     uv_run(&proc->loop->uv, UV_RUN_ONCE);
   }
 
-  ptyproc->wp = wp;
+  ptyproc->winpty_object = winpty_object;
   ptyproc->process_handle = process_handle;
-  wp = NULL;
+  winpty_object = NULL;
   process_handle = NULL;
 
 cleanup:
-  if (err != NULL) {
-    status = (int)winpty_error_code(err);
-  }
   winpty_error_free(err);
   winpty_config_free(cfg);
   winpty_spawn_config_free(spawncfg);
-  winpty_free(wp);
+  winpty_free(winpty_object);
   xfree(in_name);
   xfree(out_name);
   if (process_handle != NULL) {
@@ -135,7 +148,7 @@ cleanup:
   }
   xfree(in_req);
   xfree(out_req);
-  xfree(cmdline);
+  xfree(cmd_line);
   xfree(cwd);
   return status;
 }
@@ -144,8 +157,8 @@ void pty_process_resize(PtyProcess *ptyproc, uint16_t width,
                         uint16_t height)
   FUNC_ATTR_NONNULL_ALL
 {
-  if (ptyproc->wp != NULL) {
-    winpty_set_size(ptyproc->wp, width, height, NULL);
+  if (ptyproc->winpty_object != NULL) {
+    winpty_set_size(ptyproc->winpty_object, width, height, NULL);
   }
 }
 
@@ -164,9 +177,9 @@ void pty_process_close(PtyProcess *ptyproc)
 void pty_process_close_master(PtyProcess *ptyproc)
   FUNC_ATTR_NONNULL_ALL
 {
-  if (ptyproc->wp != NULL) {
-    winpty_free(ptyproc->wp);
-    ptyproc->wp = NULL;
+  if (ptyproc->winpty_object != NULL) {
+    winpty_free(ptyproc->winpty_object);
+    ptyproc->winpty_object = NULL;
   }
 }
 
@@ -182,6 +195,19 @@ static void pty_process_connect_cb(uv_connect_t *req, int status)
   req->handle = NULL;
 }
 
+static void wait_eof_timer_cb(uv_timer_t *wait_eof_timer)
+  FUNC_ATTR_NONNULL_ALL
+{
+  PtyProcess *ptyproc =
+    (PtyProcess *)((uv_handle_t *)wait_eof_timer->data);
+  Process *proc = (Process *)ptyproc;
+
+  if (!proc->out || !uv_is_readable(proc->out->uvstream)) {
+    uv_timer_stop(&ptyproc->wait_eof_timer);
+    pty_process_finish2(ptyproc);
+  }
+}
+
 static void pty_process_finish2(PtyProcess *ptyproc)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -200,99 +226,132 @@ static void pty_process_finish2(PtyProcess *ptyproc)
   proc->internal_exit_cb(proc);
 }
 
-static int build_cmdline(char **argv, wchar_t **cmdline)
+/// Build the command line to pass to CreateProcessW.
+///
+/// @param[in]  argv  Array with string arguments.
+/// @param[out]  cmd_line  Location where saved bulded cmd line.
+///
+/// @returns zero on sucess, or error code of MultiByteToWideChar function.
+///
+static int build_cmd_line(char **argv, wchar_t **cmd_line)
   FUNC_ATTR_NONNULL_ALL
 {
-  char *args = NULL;
-  size_t args_len = 0, argc = 0;
-  int ret;
-  QUEUE q;
-  QUEUE_INIT(&q);
+  size_t utf8_cmd_line_len = 0;
+  size_t argc = 0;
+  QUEUE args_q;
 
+  QUEUE_INIT(&args_q);
   while (*argv) {
     size_t buf_len = strlen(*argv) * 2 + 3;
-    arg_T *arg = xmalloc(sizeof(arg_T));
-    arg->arg = (char *)xmalloc(buf_len);
-    quote_cmd_arg(arg->arg, buf_len, *argv);
-    args_len += strlen(arg->arg);
-    QUEUE_INIT(&arg->node);
-    QUEUE_INSERT_TAIL(&q, &arg->node);
+    ArgNode *arg_node = xmalloc(sizeof(*arg_node));
+    arg_node->arg = xmalloc(buf_len);
+    quote_cmd_arg(arg_node->arg, buf_len, *argv);
+    utf8_cmd_line_len += strlen(arg_node->arg);
+    QUEUE_INIT(&arg_node->node);
+    QUEUE_INSERT_TAIL(&args_q, &arg_node->node);
     argc++;
     argv++;
   }
-  args_len += argc;
-  args = xmalloc(args_len);
-  *args = NUL;
+
+  utf8_cmd_line_len += argc;
+  char *utf8_cmd_line = xmalloc(utf8_cmd_line_len);
+  *utf8_cmd_line = NUL;
   while (1) {
-    QUEUE *head = QUEUE_HEAD(&q);
+    QUEUE *head = QUEUE_HEAD(&args_q);
     QUEUE_REMOVE(head);
-    arg_T *arg = QUEUE_DATA(head, arg_T, node);
-    xstrlcat(args, arg->arg, args_len);
-    xfree(arg->arg);
-    xfree(arg);
-    if (QUEUE_EMPTY(&q)) {
+    ArgNode *arg_node = QUEUE_DATA(head, ArgNode, node);
+    xstrlcat(utf8_cmd_line, arg_node->arg, utf8_cmd_line_len);
+    xfree(arg_node->arg);
+    xfree(arg_node);
+    if (QUEUE_EMPTY(&args_q)) {
       break;
     } else {
-      xstrlcat(args, " ", args_len);
+      xstrlcat(utf8_cmd_line, " ", utf8_cmd_line_len);
     }
   }
-  ret = utf8_to_utf16(args, cmdline);
-  xfree(args);
-  return ret;
+
+  int result = utf8_to_utf16(utf8_cmd_line, cmd_line);
+  if (result != 0) {
+  }
+  xfree(utf8_cmd_line);
+  return result;
 }
 
-// Emulate quote_cmd_arg of libuv and quotes command line arguments
-static void quote_cmd_arg(char *target, size_t remain, const char *source)
+/// Emulate quote_cmd_arg of libuv and quotes command line argument.
+/// Most of the code came from libuv.
+///
+/// @param[out]  dist  Location where saved quotes argument.
+/// @param  dist_remaining  Deistnation buffer size.
+/// @param[in]  src Pointer to argument.
+///
+static void quote_cmd_arg(char *dist, size_t dist_remaining, const char *src)
   FUNC_ATTR_NONNULL_ALL
 {
-  size_t src_len = strlen(source);
-  size_t i;
+  size_t src_len = strlen(src);
   bool quote_hit = true;
-  char *start = target;
-  char tmp;
+  char *start = dist;
 
   if (src_len == 0) {
-    snprintf(target, remain, "\"\"");
+    // Need double quotation for empty argument.
+    snprintf(dist, dist_remaining, "\"\"");
     return;
   }
 
-  if (NULL == strpbrk(source, " \t\"")) {
-    xstrlcpy(target, source, remain);
+  if (NULL == strpbrk(src, " \t\"")) {
+    // No quotation needed.
+    xstrlcpy(dist, src, dist_remaining);
     return;
   }
 
-  if (NULL == strpbrk(source, "\"\\")) {
-    snprintf(target, remain, "\"%s\"", source);
+  if (NULL == strpbrk(src, "\"\\")) {
+    // No embedded double quotes or backlashes, so I can just wrap quote marks.
+    // around the whole thing.
+    snprintf(dist, dist_remaining, "\"%s\"", src);
     return;
   }
 
-  assert(remain--);
-  *(target++) = NUL;
-  assert(remain--);
-  *(target++) = '"';
-  for (i = src_len; i > 0; i--) {
-    assert(remain--);
-    *(target++) = source[i - 1];
-
-    if (quote_hit && source[i - 1] == '\\') {
-      assert(remain--);
-      *(target++) = '\\';
-    } else if (source[i - 1] == '"') {
+  // Expected input/output:
+  //   input : hello"world
+  //   output: "hello\"world"
+  //   input : hello""world
+  //   output: "hello\"\"world"
+  //   input : hello\world
+  //   output: hello\world
+  //   input : hello\\world
+  //   output: hello\\world
+  //   input : hello\"world
+  //   output: "hello\\\"world"
+  //   input : hello\\"world
+  //   output: "hello\\\\\"world"
+  //   input : hello world\
+  //   output: "hello world\\"
+
+  assert(dist_remaining--);
+  *(dist++) = NUL;
+  assert(dist_remaining--);
+  *(dist++) = '"';
+  for (size_t i = src_len; i > 0; i--) {
+    assert(dist_remaining--);
+    *(dist++) = src[i - 1];
+    if (quote_hit && src[i - 1] == '\\') {
+      assert(dist_remaining--);
+      *(dist++) = '\\';
+    } else if (src[i - 1] == '"') {
       quote_hit = true;
-      assert(remain--);
-      *(target++) = '\\';
+      assert(dist_remaining--);
+      *(dist++) = '\\';
     } else {
       quote_hit = false;
     }
   }
-  assert(remain);
-  *target = '"';
-  while (start < target) {
-    tmp = *start;
-    *start = *target;
-    *target = tmp;
+  assert(dist_remaining);
+  *dist = '"';
+
+  while (start < dist) {
+    char tmp = *start;
+    *start = *dist;
+    *dist = tmp;
     start++;
-    target--;
+    dist--;
   }
-  return;
 }
diff --git a/src/nvim/os/pty_process_win.h b/src/nvim/os/pty_process_win.h
index 806857f130..59e0fad7f7 100644
--- a/src/nvim/os/pty_process_win.h
+++ b/src/nvim/os/pty_process_win.h
@@ -12,16 +12,17 @@ typedef struct pty_process {
   Process process;
   char *term_name;
   uint16_t width, height;
-  winpty_t *wp;
+  winpty_t *winpty_object;
   HANDLE finish_wait;
   HANDLE process_handle;
   uv_timer_t wait_eof_timer;
 } PtyProcess;
 
-typedef struct arg_S {
-  char *arg;
-  QUEUE node;
-} arg_T;
+// Structure used by build_cmd_line()
+typedef struct arg_node {
+  char *arg;  // pointer to argument.
+  QUEUE node;  // QUEUE structure.
+} ArgNode;
 
 static inline PtyProcess pty_process_init(Loop *loop, void *data)
 {
@@ -30,7 +31,7 @@ static inline PtyProcess pty_process_init(Loop *loop, void *data)
   rv.term_name = NULL;
   rv.width = 80;
   rv.height = 24;
-  rv.wp = NULL;
+  rv.winpty_object = NULL;
   rv.finish_wait = NULL;
   rv.process_handle = NULL;
   return rv;