On 26/02/2016 10:13, Mike Williams wrote:
On 26/02/2016 09:41, Nikolay Aleksandrovich Pavlov wrote:
2016-02-26 12:34 GMT+03:00 Mike Williams <[email protected]>:
On 24/02/2016 23:21, Nikolay Aleksandrovich Pavlov wrote:

2016-02-22 19:57 GMT+03:00 Bram Moolenaar <[email protected]>:

This one too.

[...]
os_win32.c: In function 'read_console_input.constprop.22':
os_win32.c:299:28: warning: array subscript is above array bounds
[-Warray-bounds]
        s_irCache[i] = s_irCache[i + 1];
                                ^


Not sure how to fix this.  Or even if this is a real problem.


I do not know whether this is a real problem, but such things are best
written with memmove:

       memmove(s_irCache + head, s_irCache + head + 1, (size_t) (tail -
head) * sizeof(s_irCache[0]));


The warning will be because ultimately i is limited by s_dwMax which is
based on a value returned from ReadConsoleInputW() and the compiler cannot
tell if this will be larger that IRSIZE although we know it from the API
doc.  Limiting the range of s_dwMax with something like

    s_dwMax = min(dwEvents, IRSIZE);

may kill the warning without the code gymnastics above.

This is not code gymnastics. If one wants to shift an array he should
use memmove() because this is exactly the purpose of this function,
also slightly less code then a cycle. This should also run faster,
unless compiler was able to optimize cycle to the same code as
memmove() (which should be rather likely given how common this pattern
is).

The lack of diff hid the context, at least for me, sorry.  It is still
possible the compiler will warn about indexing beyond the end of s_irCache.

The whole double loop feels heavy - for every repeat of a the size event
the remainder of the buffer is moved down one place, even if there are
later repeats.  Since the buffer is only 10 elements it may be simpler
to read into a stack buffer and then copy into the static skipping runs
in a single loop.  Something like the following (ReadConsoleInputW()
changed to read into temporary irCache) - untested ;-):

if (s_dwMax > 1)
{
      /* Reduce sequences of buffer size events to just 1 */
      s_irCache[0] = irCache[0];
      for (head = 1, prev = 0; head < s_dwMax; head++)
      {
        if (s_irCache[prev].EventType == WINDOW_BUFFER_SIZE_EVENT &&
            irCache[head].EventType == WINDOW_BUFFER_SIZE_EVENT)
        {
          continue;
        }
        s_irCache[++prev] = irCache[head];
      }
      s_dwMax = prev + 1;
}

Of course, it can be done with just the single array. Attached is a patch that simplifies the code. Since it is a such a small array it is hard to justify writing extra code to optimise the copying being done. Builds for me but untested as I have Win7 and don't use IME, YMMV.

Mike
--
The first sign of maturity is the discovery that the volume knob also turns to the left.

--
--
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 --git a/src/os_win32.c b/src/os_win32.c
--- a/src/os_win32.c
+++ b/src/os_win32.c
@@ -256,9 +256,8 @@ read_console_input(
     static DWORD s_dwIndex = 0;
     static DWORD s_dwMax = 0;
     DWORD dwEvents;
-    int head;
-    int tail;
-    int i;
+    unsigned int next;
+    unsigned int tail;
 
     if (nLength == -2)
 	return (s_dwMax > 0) ? TRUE : FALSE;
@@ -283,24 +282,17 @@ read_console_input(
 	    *lpEvents = 0;
 	    return TRUE;
 	}
-
 	if (s_dwMax > 1)
 	{
-	    head = 0;
-	    tail = s_dwMax - 1;
-	    while (head != tail)
+	    /* Remove repeated buffer size events to reduce flicker */
+	    for (tail = 0, next = 1; next < s_dwMax; next++)
 	    {
-		if (s_irCache[head].EventType == WINDOW_BUFFER_SIZE_EVENT
-			&& s_irCache[head + 1].EventType
-						  == WINDOW_BUFFER_SIZE_EVENT)
+		if (s_irCache[tail].EventType == WINDOW_BUFFER_SIZE_EVENT &&
+		    s_irCache[next].EventType == WINDOW_BUFFER_SIZE_EVENT)
 		{
-		    /* Remove duplicate event to avoid flicker. */
-		    for (i = head; i < tail; ++i)
-			s_irCache[i] = s_irCache[i + 1];
-		    --tail;
 		    continue;
 		}
-		head++;
+		s_irCache[++tail] = s_irCache[next];
 	    }
 	    s_dwMax = tail + 1;
 	}

Raspunde prin e-mail lui