Here is some small improvements for the patch:
src/window.c, match_add():
- removed unnecessary (after alloc() to alloc_clear() transformation)
zero-initializations of m->pos fields,
- checking that li == NULL was moved inside for-loop body
- decrement i when the pos_list element is a number and has value 0 to be
symmetric with sublist counterpart
src/screen.c:
- simple code alignment
runtime/doc/eval.txt, matchaddpos():
this is a bit more important: actually column number and match length must
correspond to bytes as col() returns, not screen columns as virtcol()
returns! It does matter for Unicode texts like Cyrillics, i changed doc to
reflect this.
Cheers, Alexey.
2014-06-19 19:35 GMT+04:00 Alexey Radkov <[email protected]>:
> Using alloc_clear() instead alloc() in match_add() proved to be a very
> good fix: sometimes i could have a segfault on vim's start without it
> (today i was lucky to catch a core dump on an older patch and gdb showed
> that the reason was uninitialized m->pos array).
>
> Also this allows removing line
>
> m->pos.cur = 0;
>
> four lines below: perhaps this will make the code cleaner?
>
> Cheers, Alexey.
>
>
>
> 2014-06-18 15:01 GMT+04:00 Bram Moolenaar <[email protected]>:
>
>
>> Dominique wrote:
>>
>> [...]
>>
>> > > Compiling with gcc -O2, I also get this compilation warning
>> > > which also points to the same bug:
>> > >
>> > > screen.c:7471:3: warning: 'nmatched' may be used uninitialized in this
>> > > function [-Wmaybe-uninitialized]
>> > >
>> > > Bug is introduced in vim-7.4.330.
>> > >
>> > > Attached patch fixes it but but I don't understand this
>> > > code enough to tell whether it's correct, so please
>> > > review it. Patch also fixes a typo in :help matchaddpos().
>> > >
>> > > Regards
>> > > Dominique
>> >
>> >
>> > I just see that my patch fixes another bug with Vim-7.4.333:
>> >
>> > Type:
>> >
>> > $ vim vim/src/screen.c +312
>> >
>> > Then move cursor up and down with k j around
>> > the curly brace at line screen.c:312.
>> >
>> > Observe that the curly brace sometimes remains
>> > highlighted when moving above or below the curly brace.
>> > That's a bug. Curly brace should not be highlighted when
>> > moving above or below it. The proposed patch that fixes
>> > valgrind errors also happens to fix this bug with spurious
>> > highlight of parenthesis.
>>
>> Thanks for the fix. The patch is complicated and has only one small
>> test. Please look out for any further problems. It is a useful
>> addition, should make match highlighting much faster, worth taking some
>> risk.
>>
>> --
>> Yah, well, we had to carve our electrons out of driftwood we'd
>> find. In the winter. Uphill. Both ways.
>>
>> /// Bram Moolenaar -- [email protected] -- 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
>>
>> ---
>> You received this message because you are subscribed to the Google Groups
>> "vim_dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to [email protected].
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>
--
--
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
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
diff -r 4aa63564dd3f runtime/doc/eval.txt
--- a/runtime/doc/eval.txt Wed Jun 18 21:38:18 2014 +0200
+++ b/runtime/doc/eval.txt Fri Jun 20 19:09:05 2014 +0400
@@ -4391,17 +4391,17 @@
required, for example to highlight matching parentheses.
The list {pos} can contain one of these items:
- - A number. This while line will be highlighted. The first
+ - A number. This whole line will be highlighted. The first
line has number 1.
- A list with one number, e.g., [23]. The whole line with this
number will be highlighted.
- A list with two numbers, e.g., [23, 11]. The first number is
- the line number, the second one the column number (first
- column is 1). The character at this position will be
- highlighted.
+ the line number, the second one is the column number (first
+ column is 1, the value must correspond to the byte index as
+ |col()| would return). The character at this position will
+ be highlighted.
- A list with three numbers, e.g., [23, 11, 3]. As above, but
- the third number gives the length of the highlight in screen
- cells.
+ the third number gives the length of the highlight in bytes.
The maximum number of positions is 8.
diff -r 4aa63564dd3f src/screen.c
--- a/src/screen.c Wed Jun 18 21:38:18 2014 +0200
+++ b/src/screen.c Fri Jun 20 19:09:05 2014 +0400
@@ -7485,7 +7485,7 @@
colnr_T mincol; /* minimal column for a match */
{
int i;
- int bot = -1;
+ int bot = -1;
shl->lnum = 0;
for (i = posmatch->cur; i < MAXPOSMATCH; i++)
diff -r 4aa63564dd3f src/window.c
--- a/src/window.c Wed Jun 18 21:38:18 2014 +0200
+++ b/src/window.c Fri Jun 20 19:09:05 2014 +0400
@@ -6813,7 +6813,6 @@
m->id = id;
m->priority = prio;
m->pattern = pat == NULL ? NULL : vim_strsave(pat);
- m->pos.cur = 0;
m->hlg_id = hlg_id;
m->match.regprog = regprog;
m->match.rmm_ic = FALSE;
@@ -6827,7 +6826,7 @@
listitem_T *li;
int i;
- for (i = 0, li = pos_list->lv_first; i < MAXPOSMATCH;
+ for (i = 0, li = pos_list->lv_first; li != NULL && i < MAXPOSMATCH;
i++, li = li->li_next)
{
linenr_T lnum = 0;
@@ -6837,11 +6836,6 @@
listitem_T *subli;
int error = FALSE;
- if (li == NULL)
- {
- m->pos.pos[i].lnum = 0;
- break;
- }
if (li->li_tv.v_type == VAR_LIST)
{
subl = li->li_tv.vval.v_list;
@@ -6853,12 +6847,12 @@
lnum = get_tv_number_chk(&subli->li_tv, &error);
if (error == TRUE)
goto fail;
- m->pos.pos[i].lnum = lnum;
if (lnum == 0)
{
--i;
continue;
}
+ m->pos.pos[i].lnum = lnum;
subli = subli->li_next;
if (subli != NULL)
{
@@ -6879,7 +6873,10 @@
else if (li->li_tv.v_type == VAR_NUMBER)
{
if (li->li_tv.vval.v_number == 0)
+ {
+ --i;
continue;
+ }
m->pos.pos[i].lnum = li->li_tv.vval.v_number;
m->pos.pos[i].col = 0;
m->pos.pos[i].len = 0;
