[vim/vim] Fixed crash after OOM in expand_shellcmd() (#5449)

15 views
Skip to first unread message

Dominique Pellé

unread,
Jan 6, 2020, 4:59:04 PM1/6/20
to vim/vim, Subscribed

This fixes yet another crash after out-of-memory error in expand_shellcmd() in vim-8.2.94.

I think that there are probably many other places where failures to allocate are not gracefully handled.
I'm not sure whether it's really worth fixing them. They are very unlikely to happen.
But at least this one is simple to fix.


You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/5449

Commit Summary

  • Fixed crash after OOM in expand_shellcmd()

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Codecov

unread,
Jan 6, 2020, 5:20:15 PM1/6/20
to vim/vim, Subscribed

Codecov Report

Merging #5449 into master will decrease coverage by 4.49%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff            @@

##           master    #5449     +/-   ##

=========================================

- Coverage   82.85%   78.36%   -4.5%     

=========================================

  Files         134      134             

  Lines      147808   147116    -692     

=========================================

- Hits       122473   115294   -7179     

- Misses      25335    31822   +6487
Impacted Files Coverage Δ
src/cmdexpand.c 79.23% <33.33%> (-2.43%) ⬇️
src/libvterm/src/vterm_internal.h 50% <0%> (-50%) ⬇️
src/libvterm/src/rect.h 79.31% <0%> (-17.25%) ⬇️
src/pty.c 45% <0%> (-15%) ⬇️
src/sound.c 81.11% <0%> (-14.45%) ⬇️
src/libvterm/src/unicode.c 75% <0%> (-14.14%) ⬇️
src/gui_gtk_x11.c 44.39% <0%> (-13.67%) ⬇️
src/crypt.c 79.59% <0%> (-13.61%) ⬇️
src/libvterm/include/vterm.h 25% <0%> (-12.5%) ⬇️
src/arabic.c 85.36% <0%> (-12.2%) ⬇️
... and 120 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1860bde...8079a97. Read the comment docs.

Bram Moolenaar

unread,
Jan 7, 2020, 3:06:24 PM1/7/20
to vim/vim, Subscribed

Closed #5449 via 8b7aa2f.

Bram Moolenaar

unread,
Jan 7, 2020, 3:12:17 PM1/7/20
to vim...@googlegroups.com, Dominique Pellé

Dominique wrote:

> This fixes yet another crash after out-of-memory error in `expand_shellcmd()` in vim-8.2.94.
>
> I think that there are probably many other places where failures to allocate are not gracefully handled.
> I&#39;m not sure whether it&#39;s really worth fixing them. They are very unlikely to happen.
> But at least this one is simple to fix.

Most places are unlikely to actually run out of memory. But in some
places it is important to catch. E.g. when saving for undo or making a
complicated change that eats up lots of memory. It is hard to predict
where this happens, thus the best will be to fix all we can find.

Some engineers are lazy and say "you can't find and fix all, just let it
crash". Or, when working on code for the Amiga: "you have a whole
Megabyte of memory, you won't ever run out!".

On Linux some people say that because of overcommitting it will crash
anyway. That of course means you need to disable overcommitting, not
stop checking for running out of memory. Overcommitting was made for
servers where processes can be restarted without losing data, not for
running end-user commands.


--
Sometimes I think the surest sign that intelligent life exists elsewhere
in the universe is that none of it has tried to contact us. (Calvin)

/// 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 ///
Reply all
Reply to author
Forward
0 new messages