Fix fnamemodify()'s handling of :r after :e #5024
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
During `fnamemodify()`, for `":r"` we will iterate up the filename.
Ensuring that we don't go before the filename's first directory
separator (the tail) is insufficient in cases where we've already
handled a `":e"` modifier, for example:
```
"path/to/this.file.ext" :e:e:r:r
^ ^-------- *fnamep
+------------- tail
```
This means for a `":r"`, we'll go before `*fnamep`, and outside the
bounds of the filename. This is both incorrect and causes vim to attempt
to allocate a lot of memory, which will either fails and we'll continue
with a null string, or we'll get a runtime allocation error.
The large memory allocation comes from calculating `s - *fnamep`. Since
`s` is before `*fnamep`, we caluclate a negative length, which ends up
being interpreted as an amount to allocate, causing the above problem.
We must instead ensure we don't go before `*fnamep` nor `tail`. The
check for `tail` is still relevant, for example, here we don't want to
go before it:
```
"path/to/this.file.ext" :r:r:r
^ ^------------- tail
+--------------------- *fnamep
```
d761aef to
8d61b5d
Compare
|
If I'm reading the report right, I appear to have changed test coverage to uncover some lines in |
justinmk
added a commit
to neovim/neovim
that referenced
this pull request
Oct 11, 2019
Problem: Fnamemodify() fails when repeating :e. Solution: Do not go before the tail. (Rob Pilling, closes vim/vim#5024) vim/vim@b189295
manuelschiller
pushed a commit
to manuelschiller/vim
that referenced
this pull request
Nov 10, 2019
Problem: Fnamemodify() fails when repeating :e. Solution: Do not go before the tail. (Rob Pilling, closes vim#5024)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Durin 8000 g
fnamemodify(), for":r"we will iterate up the filename. Ensuring that we don't go before the filename's first directory separator (the tail) is insufficient in cases where we've already handled a":e"modifier, for example:This means for a
":r", we'll go before*fnamep, and outside the bounds of the filename. This is both incorrect and causes vim to attempt to allocate a lot of memory, which will either fails and we'll continue with a null string, or we'll get a runtime allocation error.The large memory allocation comes from calculating
s - *fnamep. Sincesis before*fnamep, we calculate a negative length, which ends up being interpreted as an amount to allocate, causing the above problem.We must instead ensure we don't go before
*fnamepnortail. The check fortailis still relevant, for example, here we don't want to go before it:(This is cloned this PR to neovim)