Re: [patch] Fixes wanted-column bug in cursorbind feature

2012-03-28 Fir de Conversatie Bram Moolenaar

Gary Johnson wrote:

 There is a bug in Vim's tracking of the wanted cursor column in the
 cursorbind feature.  I believe that the attached patch fixes it.
 
 Vim keeps track of the desired or wanted cursor column as well as
 the actual cursor column.  When the user moves the cursor to a
 different line and the wanted cursor column value is not stale, Vim
 tries to move the cursor to that wanted column.  When 'cursorbind'
 is set, that wanted cursor column information is not being copied
 from the active window to the other cursorbound windows, resulting
 in the cursor jumping to seemingly random columns when moving the
 cursor up or down following a jump to a different window.
 
 To demonstrate this, start vim as vim -N -u NONE and execute the
 following.
 
 :set cursorbind
 a123456789Esc
 Yp
 :vnew
 PP
 
 Now move the cursor to column 5 and move the cursor to the other
 window:
 
 4l
 C-WC-W
 
 The cursor will be at column 5 of the other window, as expected.
 Now move the cursor to column 6 and move back to the original
 window.
 
 l
 C-WC-W
 
 The cursor will be at column 6, again as expected.  Now move the
 cursor down a line.
 
 j
 
 The cursor will move to row 2 but to column 5 instead of column 6.

Thanks for the patch.  I'll include it.

-- 
God made the integers; all else is the work of Man.
-- Kronecker

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php


[patch] Fixes wanted-column bug in cursorbind feature

2012-03-26 Fir de Conversatie Gary Johnson
There is a bug in Vim's tracking of the wanted cursor column in the
cursorbind feature.  I believe that the attached patch fixes it.

Vim keeps track of the desired or wanted cursor column as well as
the actual cursor column.  When the user moves the cursor to a
different line and the wanted cursor column value is not stale, Vim
tries to move the cursor to that wanted column.  When 'cursorbind'
is set, that wanted cursor column information is not being copied
from the active window to the other cursorbound windows, resulting
in the cursor jumping to seemingly random columns when moving the
cursor up or down following a jump to a different window.

To demonstrate this, start vim as vim -N -u NONE and execute the
following.

:set cursorbind
a123456789Esc
Yp
:vnew
PP

Now move the cursor to column 5 and move the cursor to the other
window:

4l
C-WC-W

The cursor will be at column 5 of the other window, as expected.
Now move the cursor to column 6 and move back to the original
window.

l
C-WC-W

The cursor will be at column 6, again as expected.  Now move the
cursor down a line.

j

The cursor will move to row 2 but to column 5 instead of column 6.

Regards,
Gary

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
*** -	2012-03-26 10:46:17.264254238 -0700
--- src/move.c	2012-03-23 13:49:00.146246000 -0700
***
*** 2847,2852 
--- 2847,2854 
  # ifdef FEAT_VIRTUALEDIT
  colnr_T	coladd = curwin-w_cursor.coladd;
  # endif
+ colnr_T	curswant = curwin-w_curswant;
+ int		set_curswant = curwin-w_set_curswant;
  win_T	*old_curwin = curwin;
  buf_T	*old_curbuf = curbuf;
  int		restart_edit_save;
***
*** 2881,2886 
--- 2883,2890 
  # ifdef FEAT_VIRTUALEDIT
  	curwin-w_cursor.coladd = coladd;
  # endif
+ 	curwin-w_curswant = curswant;
+ 	curwin-w_set_curswant = set_curswant;
  
  	/* Make sure the cursor is in a valid position.  Temporarily set
  	 * restart_edit to allow the cursor to be beyond the EOL. */


Re: [patch] Fixes wanted-column bug in cursorbind feature

2012-03-26 Fir de Conversatie Gary Johnson
On 2012-03-26, Gary Johnson wrote:
 There is a bug in Vim's tracking of the wanted cursor column in the
 cursorbind feature.  I believe that the attached patch fixes it.

Patch is against version 7.3.475.

Regards,
Gary

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php


Re: [patch] Fixes bug in cursorbind feature when ve=all

2012-03-10 Fir de Conversatie Bram Moolenaar

Gary Johnson wrote:

 There is a bug in the 'cursorbind' feature of Vim 7.3.470 when
 'virtualedit' is all.  The cursor position in the inactive
 window(s) is supposed to track the cursor position in the active
 window so that when one of the inactive windows is entered, the
 cursor position in that window will match the position in the
 previous active window.
 
 What happens instead is that if the user has been moving the cursor
 around in the region to the right of the ends of the lines, entering
 another window will cause the cursor to jump to a column far to the
 right of the correct column, often to a column number in the
 hundreds or thousands.
 
 One way to observe this bug is with the following command.
 
 vim -O2 -N -u NONE -c 'set ruler' -c 'windo set cursorbind ve=all' -c 
 'normal Iline'
 
 Scroll the cursor back and forth in the empty region to the right of
 line and observe the rulers in the two status lines.  The column
 number in the right window will accurately follow the cursor
 position while the column number in the left window will continually
 increase.
 
 When Vim updates the cursor location, it keeps separate values for
 where the cursor would be in the actual line and where it is in the
 virtual line.  One of Vim's functions was not properly copying
 this latter value from the active window to the inactive windows.
 The attached patch fixes this problem.
 
 The patch is in two formats.  The move.c.hgdiff file is the output
 of hg diff and the move.c.diff-c file is a context diff.

Thanks, I'll look into it soon.

-- 
Keep America beautiful.  Swallow your beer cans.

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php


Re: is this a bug or a feature??

2007-10-20 Fir de Conversatie Ilya Sher

denis wrote:
 
 
 On Oct 13, 4:41 am, Ilya Sher [EMAIL PROTECTED] wrote:
 denis wrote:
 [snip]
 eh? is this a bug or a feature?
 It's a feature.
 The point is that terminal might be resized when running
 the external command so VIM will try to re-detect it
 after executing any external command.

 Detection of the terminal size is described in
 :he window-size
 
 Thanks. so, this somewhat makes sense. However, I am starting a vim -
 g,
 which is not running in the terminal. It sounds like in my case
 expected behavior should be different?
I'm sorry but I don't know about that.
 
 -d
 
 
  


-- 
Beating the averages: http://www.paulgraham.com/avg.html
Python paradox:   http://www.paulgraham.com/pypar.html
Bad Vista:http://badvista.org/
XML sucks:http://c2.com/cgi/wiki?XmlSucks
For robots (please don't mail me there): [EMAIL PROTECTED]

--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: is this a bug or a feature??

2007-10-15 Fir de Conversatie denis



On Oct 13, 4:41 am, Ilya Sher [EMAIL PROTECTED] wrote:
 denis wrote:
[snip]
  eh? is this a bug or a feature?

 It's a feature.
 The point is that terminal might be resized when running
 the external command so VIM will try to re-detect it
 after executing any external command.

 Detection of the terminal size is described in
 :he window-size

Thanks. so, this somewhat makes sense. However, I am starting a vim -
g,
which is not running in the terminal. It sounds like in my case
expected behavior should be different?

-d


--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: is this a bug or a feature??

2007-10-13 Fir de Conversatie Ilya Sher

denis wrote:
 Hello,
 
 I am seeing a problem where execution of seemingly unrelated commands
 causes a problem with resetting 'lines' variable
 
  here, lines is set to 23
 let g:foo = tempname()
 call system('touch ' . g:foo)
  here it is reset to the height of my xterm - in this case 50
 
 eh? is this a bug or a feature?

It's a feature.
The point is that terminal might be resized when running
the external command so VIM will try to re-detect it
after executing any external command.

Detection of the terminal size is described in
:he window-size


-- 
Beating the averages: http://www.paulgraham.com/avg.html
Python paradox:   http://www.paulgraham.com/pypar.html
Bad Vista:http://badvista.org/
XML sucks:http://c2.com/cgi/wiki?XmlSucks
For robots (please don't mail me there): [EMAIL PROTECTED]

--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: is this a bug or a feature??

2007-10-11 Fir de Conversatie Charles E Campbell Jr

denis wrote:

Hello,

I am seeing a problem where execution of seemingly unrelated commands
causes a problem with resetting 'lines' variable

 here, lines is set to 23
let g:foo = tempname()
call system('touch ' . g:foo)
 here it is reset to the height of my xterm - in this case 50

eh? is this a bug or a feature?

Below please find a minimal .vimrc as well as the output when starting
gvim from an xterm on ubuntu.

Further below is my :version information.

---
[20:41:16 [EMAIL PROTECTED] ~
$ cat .vimrc-test
if has(gui_running)
  set lines=23
endif

echo lines
let g:foo = tempname()
echo g:foo
echo lines
call system('touch ' . g:foo)
echo lines


[20:41:20 [EMAIL PROTECTED] ~
$ vim -g -u .vimrc-test
23
/tmp/v102439/0
23
50
---
  

Hello!

Well, I don't see that behavior.   Do you have a .gvimrc file, and 
what's in it?

Regards,
Chip Campbell


--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: is this a bug or a feature??

2007-10-11 Fir de Conversatie denis


On Oct 11, 6:00 am, Charles E Campbell Jr [EMAIL PROTECTED]
wrote:
 denis wrote:
 I am seeing a problem where execution of seemingly unrelated commands
 causes a problem with resetting 'lines' variable

 Well, I don't see that behavior.   Do you have a .gvimrc file, and
 what's in it?

That's extremely strange :( I don't seem to have a .gvimrc, but just
to make sure I started vim -g like so:

$ vim -g -u .vimrc-test -U NONE

The result is still the same!

-d


--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: patch to fix bug in cscope feature

2007-08-19 Fir de Conversatie Bram Moolenaar


Dominique Pelle wrote:

  I don't think that skipping white space before the pattern is a problem.
  I would rather call it an improvement.  However, I think your solution
  also has the effect that it's not possible to have a space in the
  pattern.  That is undesired.
 
 I found another way to fix it, and preserve old behavior, so that it
 allows to search for pattern which contains, start or ends with
 space(s).  For example :cs find e  a  will successfully match
 strings  a  (space, a, space).
 
 I attach the patch to this email.  Patch also contain fixes for
 a couple of typos.

Hmm, strtok() doesn't make it easy for us.  Apparently there is no way
to get the pointer of where it currently is looking for the next token
without changing the string.

Your solution depends on strtok() changing the space after the command to
a NUL.  What if there are several spaces:  :cs find   e a?  I don't
think it is defined what strtok() does with the extra spaces.  They
could be replaced with NUL or not.

Another solution, which is not clean either, is to store the length of
eap-arg in a global variable, before the first call to strtok().

-- 
hundred-and-one symptoms of being an internet addict:
196. Your computer costs more than your car.

 /// Bram Moolenaar -- [EMAIL PROTECTED] -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\download, build and distribute -- http://www.A-A-P.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: patch to fix bug in cscope feature

2007-08-19 Fir de Conversatie Dominique Pelle
 Your solution depends on strtok() changing the space after the command to
 a NUL.  What if there are several spaces:  :cs find   e a?  I don't
 think it is defined what strtok() does with the extra spaces.  They
 could be replaced with NUL or not.

 Another solution, which is not clean either, is to store the length of
 eap-arg in a global variable, before the first call to strtok().

Yes. That assumption bothered me a bit too. Changing eap-arg
after call to strtok(eap-arg,  ) may be asking for trouble.  But
information is just missing in cs_find() to implement a fix without
either:
- passing an extra parameter to the function (but it's used
  as function pointer, so we'd need to change other functions
  too (so I don't like it)
- adding a field in exarg_T with length of eap-arg, but type
  is used in tons of other places (so I don't like it)
- using a global static variable as you suggest to store length
  of eap-arg (maybe not clean, but most pragmatic solution)

So I attach a patch which uses a global static variable.
It is more portable and also simpler than my previous patch.

-- Dominique

--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---

Index: if_cscope.c
===
RCS file: /cvsroot/vim/vim7/src/if_cscope.c,v
retrieving revision 1.22
diff -c -r1.22 if_cscope.c
*** if_cscope.c 11 Mar 2007 14:48:29 -  1.22
--- if_cscope.c 19 Aug 2007 15:35:11 -
***
*** 71,77 
  static char * cs_resolve_file __ARGS((int, char *));
  static intcs_show __ARGS((exarg_T *eap));
  
! 
  static csinfo_T   csinfo[CSCOPE_MAX_CONNECTIONS];
  static cscmd_Tcs_cmds[] =
  {
--- 71,77 
  static char * cs_resolve_file __ARGS((int, char *));
  static intcs_show __ARGS((exarg_T *eap));
  
! static intlen_eap_arg;/* store length of eap-arg */
  static csinfo_T   csinfo[CSCOPE_MAX_CONNECTIONS];
  static cscmd_Tcs_cmds[] =
  {
***
*** 386,392 
   * PRIVATE: cs_add
   *
   * add cscope database or a directory name (to look for cscope.out)
!  * the the cscope connection list
   *
   * MAXPATHL 256
   */
--- 386,392 
   * PRIVATE: cs_add
   *
   * add cscope database or a directory name (to look for cscope.out)
!  * to the cscope connection list
   *
   * MAXPATHL 256
   */
***
*** 966,972 
  }
  
  pat = opt + strlen(opt) + 1;
! if (pat == NULL || (pat != NULL  pat[0] == '\0'))
  {
cs_usage_msg(Find);
return FALSE;
--- 966,972 
  }
  
  pat = opt + strlen(opt) + 1;
! if (pat = (char *)eap-arg + len_eap_arg)
  {
cs_usage_msg(Find);
return FALSE;
***
*** 1317,1323 
  #else
/* compare pathnames first */
 ((fullpathcmp(csinfo[j].fname, fname, FALSE)  FPC_SAME)
!   /* if not Windows 9x, test index file atributes too */
|| (!mch_windows95()
 csinfo[j].nVolume == bhfi.dwVolumeSerialNumber
 csinfo[j].nIndexHigh == bhfi.nFileIndexHigh
--- 1317,1323 
  #else
/* compare pathnames first */
 ((fullpathcmp(csinfo[j].fname, fname, FALSE)  FPC_SAME)
!   /* if not Windows 9x, test index file attributes too */
|| (!mch_windows95()
 csinfo[j].nVolume == bhfi.dwVolumeSerialNumber
 csinfo[j].nIndexHigh == bhfi.nFileIndexHigh
***
*** 1401,1406 
--- 1401,1408 
  if (eap-arg == NULL)
return NULL;
  
+ /* Store length of eap-arg before it gets modified by strtok() */
+ len_eap_arg = STRLEN(eap-arg);
  if ((stok = strtok((char *)(eap-arg), (const char *) )) == NULL)
return NULL;
  
***
*** 2195,2201 
cs_add_common(dblist[i], pplist[i], fllist[i]);
if (p_csverbose)
{
!   /* dont' use smsg_attr because want to display
 * connection number in the same line as
 * Added cscope database...
 */
--- 2197,2203 
cs_add_common(dblist[i], pplist[i], fllist[i]);
if (p_csverbose)
{
!   /* don't use smsg_attr because want to display
 * connection number in the same line as
 * Added cscope database...
 */


patch to fix bug in cscope feature

2007-08-18 Fir de Conversatie Dominique Pelle
Hi

Valgrind memory checker detects a bug when entering the command :cs find g.
(requires support for cscope in vim)

==19660== Conditional jump or move depends on uninitialised value(s)
==19660==at 0x80D27F4: cs_find (if_cscope.c:969)
==19660==by 0x80D13E6: do_cscope_general (if_cscope.c:135)
==19660==by 0x80D1415: do_cscope (if_cscope.c:150)
==19660==by 0x809A1FB: do_one_cmd (ex_docmd.c:2622)
==19660==by 0x8097B18: do_cmdline (ex_docmd.c:1100)
==19660==by 0x8113F5C: nv_colon (normal.c:5168)
==19660==by 0x810E0B3: normal_cmd (normal.c:1141)
==19660==by 0x80D5939: main_loop (main.c:1180)
==19660==by 0x80D554D: main (main.c:939)

Relevant code in if_cscope.c is:

   951 cs_find(eap)
   952 exarg_T *eap;
   953 {
   954 char *opt, *pat;
   955
   956 if (cs_check_for_connections() == FALSE)
   957 {
   958 (void)EMSG(_(E567: no cscope connections));
   959 return FALSE;
   960 }
   961
   962 if ((opt = strtok((char *)NULL, (const char *) )) == NULL)
   963 {
   964 cs_usage_msg(Find);
   965 return FALSE;
   966 }
   967
   968 pat = opt + strlen(opt) + 1;
!! 969 if (pat == NULL || (pat != NULL  pat[0] == '\0'))
   970 {
   971 cs_usage_msg(Find);
   972 return FALSE;
   973 }
   974
   975 return cs_find_common(opt, pat, eap-forceit, TRUE,
   976   eap-cmdidx == CMD_lcscope);
   977 } /* cs_find */

Vim accesses uninitialized memory after the string :cs find g
at line 969.  As a result, it may display usage or may call
(incorrectly) cs_find_common().

Bug happens because command :cs find g is illegal (pattern
to search is missing).  Correct command would be something like
:cs find some_pattern. However, entering an incorrect command
should not cause vim to to access uninitialized memory.  It
should display the usage of command.

I attach a patch which fixes it. It calls strtok() another time
to find the pattern after cs find g.  Note that patch changes
the behavior slightly when entering several spaces before pattern.
With patch, :cs find   pattern will search for pattern.
Without patch, :cs find   pattern will search for   pattern.
I'm not sure whether that's a better behavior or not.  I personally
prefer behavior after patch.  At least it fixes the access to
uninitialized memory. If original behavior must be preserved,
then bug needs to be fixed differently.

I was using vim-7.1 (patches 1-79) built with +cscope feature
on Linux x86.

-- Dominique

--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---

Index: if_cscope.c
===
RCS file: /cvsroot/vim/vim7/src/if_cscope.c,v
retrieving revision 1.22
diff -c -r1.22 if_cscope.c
*** if_cscope.c 11 Mar 2007 14:48:29 -  1.22
--- if_cscope.c 18 Aug 2007 16:08:22 -
***
*** 952,957 
--- 952,958 
  exarg_T *eap;
  {
  char *opt, *pat;
+ int jump;
  
  if (cs_check_for_connections() == FALSE)
  {
***
*** 964,979 
cs_usage_msg(Find);
return FALSE;
  }
  
! pat = opt + strlen(opt) + 1;
! if (pat == NULL || (pat != NULL  pat[0] == '\0'))
  {
cs_usage_msg(Find);
return FALSE;
  }
  
! return cs_find_common(opt, pat, eap-forceit, TRUE,
! eap-cmdidx == CMD_lcscope);
  } /* cs_find */
  
  
--- 965,982 
cs_usage_msg(Find);
return FALSE;
  }
+ opt = strdup(opt);
  
! if ((pat = strtok((char *)NULL, (const char *) )) == NULL)
  {
cs_usage_msg(Find);
return FALSE;
  }
  
! jump = cs_find_common(opt, pat, eap-forceit, TRUE,
! eap-cmdidx == CMD_lcscope);
! vim_free(opt);
! return jump;
  } /* cs_find */
  
  


Re: patch to fix bug in cscope feature

2007-08-18 Fir de Conversatie Dominique Pelle

I just noticed that there was a memory leak
in the patch I sent in my previous email.
So here it is again without the memory leak.


retrieving revision 1.22
diff -c -r1.22 if_cscope.c
*** if_cscope.c 11 Mar 2007 14:48:29 -  1.22
--- if_cscope.c 18 Aug 2007 17:48:54 -
***
*** 952,957 
--- 952,958 
  exarg_T *eap;
  {
  char *opt, *pat;
+ int jump;

  if (cs_check_for_connections() == FALSE)
  {
***
*** 964,979 
cs_usage_msg(Find);
return FALSE;
  }

! pat = opt + strlen(opt) + 1;
! if (pat == NULL || (pat != NULL  pat[0] == '\0'))
  {
cs_usage_msg(Find);
return FALSE;
  }

! return cs_find_common(opt, pat, eap-forceit, TRUE,
! eap-cmdidx == CMD_lcscope);
  } /* cs_find */


--- 965,983 
cs_usage_msg(Find);
return FALSE;
  }
+ opt = strdup(opt);

! if ((pat = strtok((char *)NULL, (const char *) )) == NULL)
  {
cs_usage_msg(Find);
+   vim_free(opt);
return FALSE;
  }

! jump = cs_find_common(opt, pat, eap-forceit, TRUE,
! eap-cmdidx == CMD_lcscope);
! vim_free(opt);
! return jump;
  } /* cs_find */


--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: patch to fix bug in cscope feature

2007-08-18 Fir de Conversatie Bram Moolenaar


Dominique Pelle wrote:

 Valgrind memory checker detects a bug when entering the command :cs find g.
 (requires support for cscope in vim)
 
 ==19660== Conditional jump or move depends on uninitialised value(s)
 ==19660==at 0x80D27F4: cs_find (if_cscope.c:969)
 ==19660==by 0x80D13E6: do_cscope_general (if_cscope.c:135)
 ==19660==by 0x80D1415: do_cscope (if_cscope.c:150)
 ==19660==by 0x809A1FB: do_one_cmd (ex_docmd.c:2622)
 ==19660==by 0x8097B18: do_cmdline (ex_docmd.c:1100)
 ==19660==by 0x8113F5C: nv_colon (normal.c:5168)
 ==19660==by 0x810E0B3: normal_cmd (normal.c:1141)
 ==19660==by 0x80D5939: main_loop (main.c:1180)
 ==19660==by 0x80D554D: main (main.c:939)
 
 Relevant code in if_cscope.c is:
 
951 cs_find(eap)
952 exarg_T *eap;
953 {
954 char *opt, *pat;
955
956 if (cs_check_for_connections() == FALSE)
957 {
958 (void)EMSG(_(E567: no cscope connections));
959 return FALSE;
960 }
961
962 if ((opt = strtok((char *)NULL, (const char *) )) == NULL)
963 {
964 cs_usage_msg(Find);
965 return FALSE;
966 }
967
968 pat = opt + strlen(opt) + 1;
 !! 969 if (pat == NULL || (pat != NULL  pat[0] == '\0'))
970 {
971 cs_usage_msg(Find);
972 return FALSE;
973 }
974
975 return cs_find_common(opt, pat, eap-forceit, TRUE,
976   eap-cmdidx == CMD_lcscope);
977 } /* cs_find */
 
 Vim accesses uninitialized memory after the string :cs find g
 at line 969.  As a result, it may display usage or may call
 (incorrectly) cs_find_common().
 
 Bug happens because command :cs find g is illegal (pattern
 to search is missing).  Correct command would be something like
 :cs find some_pattern. However, entering an incorrect command
 should not cause vim to to access uninitialized memory.  It
 should display the usage of command.
 
 I attach a patch which fixes it. It calls strtok() another time
 to find the pattern after cs find g.  Note that patch changes
 the behavior slightly when entering several spaces before pattern.
 With patch, :cs find   pattern will search for pattern.
 Without patch, :cs find   pattern will search for   pattern.
 I'm not sure whether that's a better behavior or not.  I personally
 prefer behavior after patch.  At least it fixes the access to
 uninitialized memory. If original behavior must be preserved,
 then bug needs to be fixed differently.
 
 I was using vim-7.1 (patches 1-79) built with +cscope feature
 on Linux x86.

Thanks for locating this problem and suggesting a solution.

I don't think that skipping white space before the pattern is a problem.
I would rather call it an improvement.  However, I think your solution
also has the effect that it's not possible to have a space in the
pattern.  That is undesired.

Can you try that this is solved by passing an empty string to strtok()?

An alternative is to do away with strtok() and use skipwhite() and
skiptowhite().  That avoids the need to duplicate opt.

-- 
Press any key to continue, press any other key to quit.

 /// Bram Moolenaar -- [EMAIL PROTECTED] -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\download, build and distribute -- http://www.A-A-P.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: patch to fix bug in cscope feature

2007-08-18 Fir de Conversatie Dominique Pelle
 I don't think that skipping white space before the pattern is a problem.
 I would rather call it an improvement.  However, I think your solution
 also has the effect that it's not possible to have a space in the
 pattern.  That is undesired.

I found another way to fix it, and preserve old behavior, so that it
allows to search for pattern which contains, start or ends with
space(s).  For example :cs find e  a  will successfully match
strings  a  (space, a, space).

I attach the patch to this email.  Patch also contain fixes for
a couple of typos.

-- Dominique

--~--~-~--~~~---~--~~
You received this message from the vim_dev maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---

Index: if_cscope.c
===
RCS file: /cvsroot/vim/vim7/src/if_cscope.c,v
retrieving revision 1.22
diff -c -r1.22 if_cscope.c
*** if_cscope.c 11 Mar 2007 14:48:29 -  1.22
--- if_cscope.c 18 Aug 2007 22:49:36 -
***
*** 386,392 
   * PRIVATE: cs_add
   *
   * add cscope database or a directory name (to look for cscope.out)
!  * the the cscope connection list
   *
   * MAXPATHL 256
   */
--- 386,392 
   * PRIVATE: cs_add
   *
   * add cscope database or a directory name (to look for cscope.out)
!  * to the cscope connection list
   *
   * MAXPATHL 256
   */
***
*** 952,957 
--- 952,958 
  exarg_T *eap;
  {
  char *opt, *pat;
+ int len;
  
  if (cs_check_for_connections() == FALSE)
  {
***
*** 959,964 
--- 960,966 
return FALSE;
  }
  
+ len = STRLEN(eap-arg);
  if ((opt = strtok((char *)NULL, (const char *) )) == NULL)
  {
cs_usage_msg(Find);
***
*** 966,972 
  }
  
  pat = opt + strlen(opt) + 1;
! if (pat == NULL || (pat != NULL  pat[0] == '\0'))
  {
cs_usage_msg(Find);
return FALSE;
--- 968,974 
  }
  
  pat = opt + strlen(opt) + 1;
! if (pat = (char *)eap-arg + len)
  {
cs_usage_msg(Find);
return FALSE;
***
*** 1317,1323 
  #else
/* compare pathnames first */
 ((fullpathcmp(csinfo[j].fname, fname, FALSE)  FPC_SAME)
!   /* if not Windows 9x, test index file atributes too */
|| (!mch_windows95()
 csinfo[j].nVolume == bhfi.dwVolumeSerialNumber
 csinfo[j].nIndexHigh == bhfi.nFileIndexHigh
--- 1319,1325 
  #else
/* compare pathnames first */
 ((fullpathcmp(csinfo[j].fname, fname, FALSE)  FPC_SAME)
!   /* if not Windows 9x, test index file attributes too */
|| (!mch_windows95()
 csinfo[j].nVolume == bhfi.dwVolumeSerialNumber
 csinfo[j].nIndexHigh == bhfi.nFileIndexHigh
***
*** 1396,1406 
  {
  cscmd_T *cmdp;
  char *stok;
! size_t len;
  
  if (eap-arg == NULL)
return NULL;
  
  if ((stok = strtok((char *)(eap-arg), (const char *) )) == NULL)
return NULL;
  
--- 1398,1409 
  {
  cscmd_T *cmdp;
  char *stok;
! size_t len, len_arg;
  
  if (eap-arg == NULL)
return NULL;
  
+ len_arg = STRLEN(eap-arg);
  if ((stok = strtok((char *)(eap-arg), (const char *) )) == NULL)
return NULL;
  
***
*** 1408,1416 
  for (cmdp = cs_cmds; cmdp-name != NULL; ++cmdp)
  {
if (strncmp((const char *)(stok), cmdp-name, len) == 0)
!   return (cmdp);
  }
! return NULL;
  } /* cs_lookup_cmd */
  
  
--- 1411,1423 
  for (cmdp = cs_cmds; cmdp-name != NULL; ++cmdp)
  {
if (strncmp((const char *)(stok), cmdp-name, len) == 0)
!   break;
  }
! 
! if (len  len_arg)
! stok[len] = ' ';/* restore space to keep eap-arg pristine */
! 
! return cmdp-name == NULL ? NULL : cmdp;
  } /* cs_lookup_cmd */
  
  
***
*** 2195,2201 
cs_add_common(dblist[i], pplist[i], fllist[i]);
if (p_csverbose)
{
!   /* dont' use smsg_attr because want to display
 * connection number in the same line as
 * Added cscope database...
 */
--- 2202,2208 
cs_add_common(dblist[i], pplist[i], fllist[i]);
if (p_csverbose)
{
!   /* don't use smsg_attr because want to display
 * connection number in the same line as
 * Added cscope database...
 */