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)

diff --git a/src/channel.c b/src/channel.c
index 3fdd312b38f45dd9ac5368342389b90c0a5eea3e..d5240fc25efff2642fa6c87d71741c133fa6c470 100644 (file)
--- a/src/channel.c
+++ b/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] =
-                      tv_get_string_buf_chk(item, opt->jo_io_name_buf[part]);
+               opt->jo_io_name[part] = tv_get_string_buf_chk(item,
+                                                  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);
-               if (p == NULL)
+               opt->jo_eof_chars = tv_get_string_buf_chk(item,
+                                                      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)
            {
@@ -5157,7 +5164,12 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2)
                    break;
                opt->jo_set2 |= JO2_TERM_API;
                opt->jo_term_api = tv_get_string_buf_chk(item,
-                                                        opt->jo_term_api_buf);
+                                                       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);
 }
 
 /*

diff --git a/src/proto/channel.pro b/src/proto/channel.pro
index fd40a3333c1ab2801dd70f5fed1ea25ce69575ce..a441a484f45dc8c7d87556c8a13e3a69627a8aee 100644 (file)
--- a/src/proto/channel.pro
+++ b/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);

diff --git a/src/structs.h b/src/structs.h
index 762cd867c7d056033070050b231150b2f02f5178..bb3e72f6354e83af449b62fb89447aa49d1c69b7 100644 (file)
--- a/src/structs.h
+++ b/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;
 

diff --git a/src/terminal.c b/src/terminal.c
index 3bf914ae9a9703cf6f4746d735ea754c0bae38fa..d789ab0563603dc6eb93da291a0e51a84698c1c3 100644 (file)
--- a/src/terminal.c
+++ b/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;
 

diff --git a/src/testdir/test_terminal.vim b/src/testdir/test_terminal.vim
index 0626253c334d3c03264b6365c5950653a45e66cd..431951bea9a1dc716031eb99ecb8866ed9a71ba6 100644 (file)
--- a/src/testdir/test_terminal.vim
+++ b/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
-  endif
-  new
+  " TODO: enable once writing to stdin works on MS-Windows
+  CheckNotMSWindows
+  CheckExecutable wc
+
   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
 
-  if executable('python')
-    new
-    call setline(1, ['print("hello")'])
-    1term ++eof=exit() 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
+func Test_terminal_eof_arg()
+  CheckExecutable python
 
-    if has('win32')
-      new
-      call setline(1, ['print("hello")'])
-      1term ++eof=<C-Z> python
-      call WaitForAssert({-> assert_match('Z', getline("$"))})
-      call assert_equal('hello', getline(line('$') - 1))
-      bwipe
-    endif
+  call setline(1, ['print("hello")'])
+  1term ++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
 
-  bwipe!
+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(line('$') - 1))})
+  call assert_match('\^Z', getline(line('$') - 1))
+  %bwipe!
+endfunc
+
+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()
-    return
-  endif
+  CheckRunVimInTerminal
 
   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')

diff --git a/src/version.c b/src/version.c
index 8f34555c7441b82d8a5162708faf693083571f74..acbc4f2955fa914b926671b700b025bf1a46ba4f 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -742,6 +742,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    181,
 /**/
     180,
 /**/