Patch 8.0.1227

[Bram Moolenaar]

Patch 8.0.1227
Problem: Undefined left shift in readfile(). (Brian 'geeknik' Carpenter)
Solution: Add cast to unsigned. (Dominique Pelle, closes #2253)
Files: src/fileio.c


*** ../vim-8.0.1226/src/fileio.c 2017-10-19 18:35:46.094557713 +0200
--- src/fileio.c 2017-10-27 22:13:04.947264450 +0200
***************
*** 1956,1972 ****
{
if (fio_flags & FIO_ENDIAN_L)
{
! u8c = (*--p << 24);
! u8c += (*--p << 16);
! u8c += (*--p << 8);
u8c += *--p;
}
else /* big endian */
{
u8c = *--p;
! u8c += (*--p << 8);
! u8c += (*--p << 16);
! u8c += (*--p << 24);
}
}
else /* UTF-8 */
--- 1956,1972 ----
{
if (fio_flags & FIO_ENDIAN_L)
{
! u8c = (unsigned)*--p << 24;
! u8c += (unsigned)*--p << 16;
! u8c += (unsigned)*--p << 8;
u8c += *--p;
}
else /* big endian */
{
u8c = *--p;
! u8c += (unsigned)*--p << 8;
! u8c += (unsigned)*--p << 16;
! u8c += (unsigned)*--p << 24;
}
}
else /* UTF-8 */
*** ../vim-8.0.1226/src/version.c 2017-10-27 01:34:55.093306847 +0200
--- src/version.c 2017-10-27 22:14:50.206524087 +0200
***************
*** 763,764 ****
--- 763,766 ----
{ /* Add new patch number below this line */
+ /**/
+ 1227,
/**/

--
TALL KNIGHT: When you have found the shrubbery, then you must cut down the
mightiest tree in the forest ... with a herring.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

/// Bram Moolenaar -- Br...@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 ///

[Christian Brabandt]

On Fr, 27 Okt 2017, Bram Moolenaar wrote:

> Patch 8.0.1227
> Problem: Undefined left shift in readfile(). (Brian 'geeknik' Carpenter)
> Solution: Add cast to unsigned. (Dominique Pelle, closes #2253)
> Files: src/fileio.c

Can't we use the example file from #2253 for creating a test?

Christian
--
Manchmal habe ich ein Dejavu und eine Gedächnislücke gleichzeitig.
Dann denke ich: Boa, das hast Du doch schonmal vergessen.
-- Vicki Vomit (alias Jens Hellmann)

[Bram Moolenaar]

Christian Brabandt wrote:

> On Fr, 27 Okt 2017, Bram Moolenaar wrote:
>
> > Patch 8.0.1227
> > Problem: Undefined left shift in readfile(). (Brian 'geeknik' Carpenter)
> > Solution: Add cast to unsigned. (Dominique Pelle, closes #2253)
> > Files: src/fileio.c
>
> Can't we use the example file from #2253 for creating a test?

Feel free to make one. I can't reproduce the error.

--
Save the plankton - eat a whale.

[Christian Brabandt]

On Sa, 28 Okt 2017, Bram Moolenaar wrote:
> Feel free to make one.

I kind of knew that this answer would come :)

Okay, will look into that one and the other undefined behaviour patches.

> I can't reproduce the error.

I was thinking, it would still make sense to have them as test, even if
those don't trigger the error in every environment.

Christian
--
Vor zwei Dingen kann man sich nicht genug in Acht nehmen:
Beschränkt man sich in seinem Fach, vor Starrsinn, tritt man heraus,
vor Unzulänglichkeit.
-- Goethe, Maximen und Reflektionen, Nr. 1039

[Christian Brabandt]

On So, 29 Okt 2017, Christian Brabandt wrote:

> On Sa, 28 Okt 2017, Bram Moolenaar wrote:
> > Feel free to make one.
>
> I kind of knew that this answer would come :)
>
> Okay, will look into that one and the other undefined behaviour patches.
>
> > I can't reproduce the error.
>
> I was thinking, it would still make sense to have them as test, even if
> those don't trigger the error in every environment.

Okay, how about this patch:

diff --git a/src/testdir/test_normal.vim b/src/testdir/test_normal.vim
index f6b1a43b8..7d31373c9 100644
--- a/src/testdir/test_normal.vim
+++ b/src/testdir/test_normal.vim
@@ -1208,6 +1208,13 @@ func! Test_normal19_z_spell()
call assert_match("Word 'goood' added to ./Xspellfile2.add", a)
call assert_equal('goood', cnt[0])

+ " Test for :spellgood!
+ let temp=execute(':spe!0/0')
+ call assert_match('Invalid region', temp)
+ let spellfile=matchstr(temp, 'Invalid region nr in \zs.*\ze line \d: 0')
+ call assert_equal(['# goood', '# goood/!', '#oood', '0/0'], readfile(spellfile))
+ call delete(spellfile)
+
" clean up
exe "lang" oldlang
call delete("./Xspellfile.add")
diff --git a/src/testdir/test_search.vim b/src/testdir/test_search.vim
index b863fcbba..9e3b8783c 100644
--- a/src/testdir/test_search.vim
+++ b/src/testdir/test_search.vim
@@ -567,3 +567,22 @@ func Test_search_cmdline_incsearch_highlight_attr()

bwipe!
endfunc
+
+func Test_search_undefined_behaviour()
+ if !has("terminal")
+ return
+ endif
+ let h = winheight(0)
+ if h < 3
+ return
+ endif
+ " did cause an undefined left shift
+ let g:buf = term_start([GetVimProg(), '--clean', '-e', '-s', '-c', 'call search(getline("."))', 'samples/test000'], {'term_rows': 3})
+ call assert_equal([''], getline(1, '$'))
+ call term_sendkeys(g:buf, ":qa!\<cr>")
+ bwipe!
+endfunc
+
+func Test_search_undefined_behaviour2()
+ call assert_fails("call search('\\%UC0000000')", 'E486')
+endfu

This already includes a test for issue #2255, that currently fails.


Christian
--
Wie man sein Kind nicht nennen sollte:
Reiner Zufall

[Bram Moolenaar]

Thanks. I'll eave out the failing part.

--
Q: Why does /dev/null accept only integers?
A: You can't sink a float.

[Christian Brabandt]

Note, it needs the samples/test000 file from the referenced issue.

Christian
--
Strasser: Welche Nationalität haben Sie?
Rick: Ich bin Trinker.
Renault: Und damit ist er Weltbürger!
(aus "Casablanca")

[Christian Brabandt]

Bram,
please include this patch, to properly test this commit.

Christian
--
Wußten Sie schon...
100 DM = 51 Euro 13 Cent.
100 Euro = 195 DM 58 Pf.

[Bram Moolenaar]

Christian wrote:

> Bram,
> please include this patch, to properly test this commit.

I didn't receive a proper patch, only a git binary diff.
Can you send me the file as an attachment?

Hmm, how can this test pass without the file anyway?
That shouldn't happen.

--
BEDEVERE: And that, my lord, is how we know the Earth to be banana-shaped.

"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

[Christian Brabandt]

On Mo, 13 Nov 2017, Bram Moolenaar wrote:

> I didn't receive a proper patch, only a git binary diff.
> Can you send me the file as an attachment?

git apply should be able to apply it nevertheless. However the file is
this one:
https://github.com/vim/vim/issues/2253

> Hmm, how can this test pass without the file anyway?
> That shouldn't happen.

Because when the file does not exist, Vim tries to create this file. But
the test could be improved to make sure the file exists. That was my
fault, sorry for that.

Christian
--
Hast Du Luftschlösser gebaut,
so braucht Deine Arbeit
nicht verloren zu sein.
Eben dort sollten sie sein.
Jetzt lege das Fundament darunter.
-- Henry David Thoreau

[Bram Moolenaar]

Christian wrote:

> On Mo, 13 Nov 2017, Bram Moolenaar wrote:
>
> > I didn't receive a proper patch, only a git binary diff.
> > Can you send me the file as an attachment?
>
> git apply should be able to apply it nevertheless.

error: git diff header lacks filename information when removing 1 leading pathname component (line 14)

> However the file is
> this one:
> https://github.com/vim/vim/issues/2253

Hmm, is that just one zero character?
Does it matter that the line break is missing?

> > Hmm, how can this test pass without the file anyway?
> > That shouldn't happen.
>
> Because when the file does not exist, Vim tries to create this file. But
> the test could be improved to make sure the file exists. That was my
> fault, sorry for that.

It only fails under ASAN anyway.

--
Lower life forms have more fun!

[Christian Brabandt]

On Mo, 13 Nov 2017, Bram Moolenaar wrote:

>
> Christian wrote:
>
> > On Mo, 13 Nov 2017, Bram Moolenaar wrote:
> >
> > > I didn't receive a proper patch, only a git binary diff.
> > > Can you send me the file as an attachment?
> >
> > git apply should be able to apply it nevertheless.
>
> error: git diff header lacks filename information when removing 1 leading pathname component (line 14)

That is surprising. I attach the file here. Hopefully, sending it by
mail does not mangle it.

> > However the file is
> > this one:
> > https://github.com/vim/vim/issues/2253
>
> Hmm, is that just one zero character?

Yeah, that interesting part is that it is a ucs4-le file encoding, else
I would have written the test to create a sample file, but did not know
how to do it for ucs4-le.


Christian
--
Mißtrauen ist ein Zeichen von Schwäche.
-- Mahatma Gandhi

[Nikolay Aleksandrovich Pavlov]

`writefile(, , 'b')` has no problems with writing any kind of binary
data. To get a list for it given that you have an existing file
serialize `readfile(, 'b')` output. E.g. `writefile(["\n\n\n\n"],
'z4.bin', 'b')` will create a file with just four zero bytes inside.

>
>
> Christian
> --
> Mißtrauen ist ein Zeichen von Schwäche.
> -- Mahatma Gandhi
>

> --
> --
> 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 vim_dev+u...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Christian Brabandt]

On Di, 14 Nov 2017, Nikolay Aleksandrovich Pavlov wrote:

> `writefile(, , 'b')` has no problems with writing any kind of binary
> data. To get a list for it given that you have an existing file
> serialize `readfile(, 'b')` output. E.g. `writefile(["\n\n\n\n"],
> 'z4.bin', 'b')` will create a file with just four zero bytes inside.

Sure, but I did not know the details of the ucs4-le encoding, so I
thought it would be easier to use the sample provided, than to having to
study ucs4 encoding before that.

Christian
--
Einander kennenlernen heißt lernen, wie fremd man einander ist.
-- Christian Morgenstern

[Bram Moolenaar]

Christian wrote:

> > > On Mo, 13 Nov 2017, Bram Moolenaar wrote:
> > >
> > > > I didn't receive a proper patch, only a git binary diff.
> > > > Can you send me the file as an attachment?
> > >
> > > git apply should be able to apply it nevertheless.
> >
> > error: git diff header lacks filename information when removing 1 leading pathname component (line 14)
>
> That is surprising. I attach the file here. Hopefully, sending it by
> mail does not mangle it.

Yes, this works.

> > > However the file is
> > > this one:
> > > https://github.com/vim/vim/issues/2253
> >
> > Hmm, is that just one zero character?
>
> Yeah, that interesting part is that it is a ucs4-le file encoding, else
> I would have written the test to create a sample file, but did not know
> how to do it for ucs4-le.

Let's see what diff does with this binary file...

--
If your company is not involved in something called "ISO 9000" you probably
have no idea what it is. If your company _is_ involved in ISO 9000 then you
definitely have no idea what it is.
(Scott Adams - The Dilbert principle)