patch 8.2.0181: problems parsing :term arguments

Problem:    Problems parsing :term arguments.
Solution:   Improve parsing, fix memory leak, add tests. (Ozaki Kiichi,
            closes #5536)

src/channel.c

@@ -4787,8 +4787,8 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
 		if (!(supported & JO_OUT_IO))
 		    break;
 		opt->jo_set |= JO_OUT_NAME << (part - PART_OUT);
-		opt->jo_io_name[part] =
+		opt->jo_io_name[part] = tv_get_string_buf_chk(item,
-		       tv_get_string_buf_chk(item, opt->jo_io_name_buf[part]);
+						   opt->jo_io_name_buf[part]);
 	    }
 	    else if (STRCMP(hi->hi_key, "pty") == 0)
 	    {
@@ -4953,7 +4953,8 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
 		if (!(supported2 & JO2_TERM_NAME))
 		    break;
 		opt->jo_set2 |= JO2_TERM_NAME;
-		opt->jo_term_name = tv_get_string_chk(item);
+		opt->jo_term_name = tv_get_string_buf_chk(item,
+						       opt->jo_term_name_buf);
 		if (opt->jo_term_name == NULL)
 		{
 		    semsg(_(e_invargval), "term_name");
@@ -4980,7 +4981,8 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
 		if (!(supported2 & JO2_TERM_OPENCMD))
 		    break;
 		opt->jo_set2 |= JO2_TERM_OPENCMD;
-		p = opt->jo_term_opencmd = tv_get_string_chk(item);
+		p = opt->jo_term_opencmd = tv_get_string_buf_chk(item,
+						    opt->jo_term_opencmd_buf);
 		if (p != NULL)
 		{
 		    // Must have %d and no other %.
@@ -4997,13 +4999,12 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
 	    }
 	    else if (STRCMP(hi->hi_key, "eof_chars") == 0)
 	    {
-		char_u *p;
-
 		if (!(supported2 & JO2_EOF_CHARS))
 		    break;
 		opt->jo_set2 |= JO2_EOF_CHARS;
-		p = opt->jo_eof_chars = tv_get_string_chk(item);
+		opt->jo_eof_chars = tv_get_string_buf_chk(item,
-		if (p == NULL)
+						       opt->jo_eof_chars_buf);
+		if (opt->jo_eof_chars == NULL)
 		{
 		    semsg(_(e_invargval), "eof_chars");
 		    return FAIL;
@@ -5082,7 +5083,13 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
 		if (!(supported2 & JO2_TERM_KILL))
 		    break;
 		opt->jo_set2 |= JO2_TERM_KILL;
-		opt->jo_term_kill = tv_get_string_chk(item);
+		opt->jo_term_kill = tv_get_string_buf_chk(item,
+						       opt->jo_term_kill_buf);
+		if (opt->jo_term_kill == NULL)
+		{
+		    semsg(_(e_invargval), "term_kill");
+		    return FAIL;
+		}
 	    }
 	    else if (STRCMP(hi->hi_key, "tty_type") == 0)
 	    {
@@ -5158,6 +5165,11 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
 		opt->jo_set2 |= JO2_TERM_API;
 		opt->jo_term_api = tv_get_string_buf_chk(item,
 							opt->jo_term_api_buf);
+		if (opt->jo_term_api == NULL)
+		{
+		    semsg(_(e_invargval), "term_api");
+		    return FAIL;
+		}
 	    }
 #endif
 	    else if (STRCMP(hi->hi_key, "env") == 0)
@@ -5247,7 +5259,7 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
 		    break;
 		opt->jo_set |= JO_STOPONEXIT;
 		opt->jo_stoponexit = tv_get_string_buf_chk(item,
-							     opt->jo_soe_buf);
+						      opt->jo_stoponexit_buf);
 		if (opt->jo_stoponexit == NULL)
 		{
 		    semsg(_(e_invargval), "stoponexit");
@@ -5817,7 +5829,7 @@ job_start(
 	typval_T    *argvars,
 	char	    **argv_arg UNUSED,
 	jobopt_T    *opt_arg,
-	int	    is_terminal UNUSED)
+	job_T	    **term_job)
 {
     job_T	*job;
     char_u	*cmd = NULL;
@@ -5968,6 +5980,9 @@ job_start(
     // Save the command used to start the job.
     job->jv_argv = argv;
 
+    if (term_job != NULL)
+	*term_job = job;
+
 #ifdef USE_ARGV
     if (ch_log_active())
     {
@@ -5984,7 +5999,7 @@ job_start(
 	ch_log(NULL, "Starting job: %s", (char *)ga.ga_data);
 	ga_clear(&ga);
     }
-    mch_job_start(argv, job, &opt, is_terminal);
+    mch_job_start(argv, job, &opt, term_job != NULL);
 #else
     ch_log(NULL, "Starting job: %s", (char *)cmd);
     mch_job_start((char *)cmd, job, &opt);
@@ -6600,7 +6615,7 @@ f_job_start(typval_T *argvars, typval_T *rettv)
     rettv->v_type = VAR_JOB;
     if (check_restricted() || check_secure())
 	return;
-    rettv->vval.v_job = job_start(argvars, NULL, NULL, FALSE);
+    rettv->vval.v_job = job_start(argvars, NULL, NULL, NULL);
 }
 
 /*

src/proto/channel.pro

@@ -51,7 +51,7 @@ void job_set_options(job_T *job, jobopt_T *opt);
 void job_stop_on_exit(void);
 int has_pending_job(void);
 int job_check_ended(void);
-job_T *job_start(typval_T *argvars, char **argv_arg, jobopt_T *opt_arg, int is_terminal);
+job_T *job_start(typval_T *argvars, char **argv_arg, jobopt_T *opt_arg, job_T **term_job);
 char *job_status(job_T *job);
 int job_stop(job_T *job, typval_T *argvars, char *type);
 void invoke_prompt_callback(void);

src/structs.h

@@ -2106,7 +2106,7 @@ typedef struct
     int		jo_block_write;	// for testing only
     int		jo_part;
     int		jo_id;
-    char_u	jo_soe_buf[NUMBUFLEN];
+    char_u	jo_stoponexit_buf[NUMBUFLEN];
     char_u	*jo_stoponexit;
     dict_T	*jo_env;	// environment variables
     char_u	jo_cwd_buf[NUMBUFLEN];
@@ -2121,17 +2121,21 @@ typedef struct
     buf_T	*jo_bufnr_buf;
     int		jo_hidden;
     int		jo_term_norestore;
+    char_u	jo_term_name_buf[NUMBUFLEN];
     char_u	*jo_term_name;
+    char_u	jo_term_opencmd_buf[NUMBUFLEN];
     char_u	*jo_term_opencmd;
     int		jo_term_finish;
+    char_u	jo_eof_chars_buf[NUMBUFLEN];
     char_u	*jo_eof_chars;
+    char_u	jo_term_kill_buf[NUMBUFLEN];
     char_u	*jo_term_kill;
 # if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
     long_u	jo_ansi_colors[16];
 # endif
     int		jo_tty_type;	    // first character of "tty_type"
-    char_u	*jo_term_api;
     char_u	jo_term_api_buf[NUMBUFLEN];
+    char_u	*jo_term_api;
 #endif
 } jobopt_T;
 

src/terminal.c

@@ -595,9 +595,7 @@ term_start(
 #if defined(FEAT_SESSION)
     // Remember the command for the session file.
     if (opt->jo_term_norestore || argv != NULL)
-    {
 	term->tl_command = vim_strsave((char_u *)"NONE");
-    }
     else if (argvar->v_type == VAR_STRING)
     {
 	char_u	*cmd = argvar->vval.v_string;
@@ -646,7 +644,11 @@ term_start(
     }
 
     if (opt->jo_term_api != NULL)
-	term->tl_api = vim_strsave(opt->jo_term_api);
+    {
+	char_u *p = skiptowhite(opt->jo_term_api);
+
+	term->tl_api = vim_strnsave(opt->jo_term_api, p - opt->jo_term_api);
+    }
     else
 	term->tl_api = vim_strsave((char_u *)"Tapi_");
 
@@ -778,6 +780,7 @@ ex_terminal(exarg_T *eap)
 	    char_u *buf = NULL;
 	    char_u *keys;
 
+	    vim_free(opt.jo_eof_chars);
 	    p = skiptowhite(cmd);
 	    *p = NUL;
 	    keys = replace_termcodes(ep + 1, &buf,
@@ -6697,7 +6700,7 @@ term_and_job_init(
 #endif
 
     // This may change a string in "argvar".
-    term->tl_job = job_start(argvar, argv, opt, TRUE);
+    term->tl_job = job_start(argvar, argv, opt, &term->tl_job);
     if (term->tl_job != NULL)
 	++term->tl_job->jv_refcount;
 

src/testdir/test_terminal.vim

@@ -689,53 +689,70 @@ func Test_terminal_noblock()
 endfunc
 
 func Test_terminal_write_stdin()
-  if !executable('wc')
-    throw 'skipped: wc command not available'
-  endif
-  if has('win32')
   " TODO: enable once writing to stdin works on MS-Windows
-    return
+  CheckNotMSWindows
-  endif
+  CheckExecutable wc
-  new
+
   call setline(1, ['one', 'two', 'three'])
   %term wc
   call WaitForAssert({-> assert_match('3', getline("$"))})
   let nrs = split(getline('$'))
   call assert_equal(['3', '3', '14'], nrs)
-  bwipe
+  %bwipe!
 
-  new
   call setline(1, ['one', 'two', 'three', 'four'])
   2,3term wc
   call WaitForAssert({-> assert_match('2', getline("$"))})
   let nrs = split(getline('$'))
   call assert_equal(['2', '2', '10'], nrs)
-  bwipe
+  %bwipe!
+endfunc
+
+func Test_terminal_eof_arg()
+  CheckExecutable python
 
-  if executable('python')
-    new
   call setline(1, ['print("hello")'])
-    1term ++eof=exit() python
+  1term ++eof=exit(123) python
   " MS-Windows echoes the input, Unix doesn't.
-    call WaitFor('getline("$") =~ "exit" || getline(1) =~ "hello"')
-    if getline(1) =~ 'hello'
-      call assert_equal('hello', getline(1))
-    else
-      call assert_equal('hello', getline(line('$') - 1))
-    endif
-    bwipe
-
   if has('win32')
-      new
+    call WaitFor({-> getline('$') =~ 'exit(123)'})
+    call assert_equal('hello', getline(line('$') - 1))
+  else
+    call WaitFor({-> getline('$') =~ 'hello'})
+    call assert_equal('hello', getline('$'))
+  endif
+  call assert_equal(123, bufnr()->term_getjob()->job_info().exitval)
+  %bwipe!
+endfunc
+
+func Test_terminal_eof_arg_win32_ctrl_z()
+  CheckMSWindows
+  CheckExecutable python
+
   call setline(1, ['print("hello")'])
   1term ++eof=<C-Z> python
-      call WaitForAssert({-> assert_match('Z', getline("$"))})
+  call WaitForAssert({-> assert_match('\^Z', getline(line('$') - 1))})
-      call assert_equal('hello', getline(line('$') - 1))
+  call assert_match('\^Z', getline(line('$') - 1))
-      bwipe
+  %bwipe!
-    endif
+endfunc
-  endif
 
-  bwipe!
+func Test_terminal_duplicate_eof_arg()
+  CheckExecutable python
+
+  " Check the last specified ++eof arg is used and should not memory leak.
+  new
+  call setline(1, ['print("hello")'])
+  1term ++eof=<C-Z> ++eof=exit(123) python
+  " MS-Windows echoes the input, Unix doesn't.
+  if has('win32')
+    call WaitFor({-> getline('$') =~ 'exit(123)'})
+    call assert_equal('hello', getline(line('$') - 1))
+  else
+    call WaitFor({-> getline('$') =~ 'hello'})
+    call assert_equal('hello', getline('$'))
+  endif
+  call assert_equal(123, bufnr()->term_getjob()->job_info().exitval)
+  %bwipe!
 endfunc
 
 func Test_terminal_no_cmd()
@@ -2242,9 +2259,7 @@ func Test_terminal_shell_option()
 endfunc
 
 func Test_terminal_setapi_and_call()
-  if !CanRunVimInTerminal()
+  CheckRunVimInTerminal
-    return
-  endif
 
   call WriteApiCall('Tapi_TryThis')
   call ch_logfile('Xlog', 'w')
@@ -2252,17 +2267,52 @@ func Test_terminal_setapi_and_call()
   unlet! g:called_bufnum
   unlet! g:called_arg
 
-  let buf = RunVimInTerminal('-S Xscript', {'term_api': 0})
+  let buf = RunVimInTerminal('-S Xscript', {'term_api': ''})
   call WaitForAssert({-> assert_match('Unpermitted function: Tapi_TryThis', string(readfile('Xlog')))})
   call assert_false(exists('g:called_bufnum'))
   call assert_false(exists('g:called_arg'))
 
-  call term_setapi(buf, 'Tapi_TryThis')
+  eval buf->term_setapi('Tapi_')
   call term_sendkeys(buf, ":set notitle\<CR>")
   call term_sendkeys(buf, ":source Xscript\<CR>")
   call WaitFor({-> exists('g:called_bufnum')})
   call assert_equal(buf, g:called_bufnum)
   call assert_equal(['hello', 123], g:called_arg)
+
+  call StopVimInTerminal(buf)
+
+  call delete('Xscript')
+  call ch_logfile('')
+  call delete('Xlog')
+  unlet! g:called_bufnum
+  unlet! g:called_arg
+endfunc
+
+func Test_terminal_api_arg()
+  CheckRunVimInTerminal
+
+  call WriteApiCall('Tapi_TryThis')
+  call ch_logfile('Xlog', 'w')
+
+  unlet! g:called_bufnum
+  unlet! g:called_arg
+
+  execute 'term ++api= ' .. GetVimCommandCleanTerm() .. '-S Xscript'
+  let buf = bufnr('%')
+  call WaitForAssert({-> assert_match('Unpermitted function: Tapi_TryThis', string(readfile('Xlog')))})
+  call assert_false(exists('g:called_bufnum'))
+  call assert_false(exists('g:called_arg'))
+
+  call StopVimInTerminal(buf)
+
+  call ch_logfile('Xlog', 'w')
+
+  execute 'term ++api=Tapi_ ' .. GetVimCommandCleanTerm() .. '-S Xscript'
+  let buf = bufnr('%')
+  call WaitFor({-> exists('g:called_bufnum')})
+  call assert_equal(buf, g:called_bufnum)
+  call assert_equal(['hello', 123], g:called_arg)
+
   call StopVimInTerminal(buf)
 
   call delete('Xscript')

src/version.c

@@ -742,6 +742,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    181,
 /**/
     180,
 /**/