diff options
author | James McCoy <jamessan@jamessan.com> | 2021-05-05 21:26:51 -0400 |
---|---|---|
committer | James McCoy <jamessan@jamessan.com> | 2021-05-06 18:56:56 -0400 |
commit | 833a6fcb6095254abae92a2309d7c3ce0e603bf3 (patch) | |
tree | 426f41debd34231fb5e86a0d26bc845edf366db5 | |
parent | 71107e12c7b68d8faf1bcc1a5794a55b21e146f3 (diff) | |
download | rneovim-833a6fcb6095254abae92a2309d7c3ce0e603bf3.tar.gz rneovim-833a6fcb6095254abae92a2309d7c3ce0e603bf3.tar.bz2 rneovim-833a6fcb6095254abae92a2309d7c3ce0e603bf3.zip |
coverity/331378: Fix inserting new decor provider
Since the providers are ordered by ns_id, inserting a new provider may
require shifting existing providers around to maintain this ordering.
When this happens, we need to allocate a new element at the end of the
vector and then shift the larger elements to the right. Rather than
iterating (incorrectly) with a loop and copying each item, use memmove
to copy the entire block.
-rw-r--r-- | src/nvim/decoration.c | 19 | ||||
-rw-r--r-- | test/functional/ui/decorations_spec.lua | 12 |
2 files changed, 24 insertions, 7 deletions
diff --git a/src/nvim/decoration.c b/src/nvim/decoration.c index ca1d141dd8..f3000f4430 100644 --- a/src/nvim/decoration.c +++ b/src/nvim/decoration.c @@ -383,8 +383,9 @@ void decor_add_ephemeral(int start_row, int start_col, int end_row, int end_col, DecorProvider *get_decor_provider(NS ns_id, bool force) { - ssize_t i; - for (i = 0; i < (ssize_t)kv_size(decor_providers); i++) { + size_t i; + size_t len = kv_size(decor_providers); + for (i = 0; i < len; i++) { DecorProvider *item = &kv_A(decor_providers, i); if (item->ns_id == ns_id) { return item; @@ -397,12 +398,16 @@ DecorProvider *get_decor_provider(NS ns_id, bool force) return NULL; } - for (ssize_t j = (ssize_t)kv_size(decor_providers)-1; j >= i; j++) { - // allocates if needed: - (void)kv_a(decor_providers, (size_t)j+1); - kv_A(decor_providers, (size_t)j+1) = kv_A(decor_providers, j); + // Adding a new provider, so allocate room in the vector + (void)kv_a(decor_providers, len); + if (i < len) { + // New ns_id needs to be inserted between existing providers to maintain + // ordering, so shift other providers with larger ns_id + memmove(&kv_A(decor_providers, i + 1), + &kv_A(decor_providers, i), + (len - i) * sizeof(kv_a(decor_providers, i))); } - DecorProvider *item = &kv_a(decor_providers, (size_t)i); + DecorProvider *item = &kv_a(decor_providers, i); *item = DECORATION_PROVIDER_INIT(ns_id); return item; diff --git a/test/functional/ui/decorations_spec.lua b/test/functional/ui/decorations_spec.lua index 09638df6c5..98aafd8757 100644 --- a/test/functional/ui/decorations_spec.lua +++ b/test/functional/ui/decorations_spec.lua @@ -66,6 +66,18 @@ describe('decorations providers', function() expect_events(expected, actual, "beam trace") end + it('does not OOM when inserting, rather than appending, to the decoration provider vector', function() + -- Add a dummy decoration provider with a larger ns id than what setup_provider() creates. + -- This forces get_decor_provider() to insert into the providers vector, + -- rather than append, which used to spin in an infinite loop allocating + -- memory until nvim crashed/was killed. + setup_provider([[ + local ns2 = a.nvim_create_namespace "ns2" + a.nvim_set_decoration_provider(ns2, {}) + ]]) + helpers.assert_alive() + end) + it('leave a trace', function() insert(mulholland) |