mirror of
https://github.com/neovim/neovim.git
synced 2024-12-20 03:05:11 -07:00
shada: Fix some memory leaks and completely ignore numbered mark names
Problems: - In two places in shada_read_when_writing() memory just was not freed. Both places were verified to cause test failures. - Numbered marks got assigned incorrect (off-by-one compared to position in the array) numbers in replace_numbered_mark. - It was possible to have non-continuously populated array of numbered marks which messed up code for merging them. (Note about tests: marks with additional data are always compared different when merging, that caused some confusion regarding why test did not work the way I expected.)
This commit is contained in:
ZyX
parent
f5373e2cdc
commit
dd1b493f75
@ -2255,6 +2255,7 @@ static inline ShaDaWriteResult shada_read_when_writing(
|
||||
entry.data.filemark.mark)
|
||||
&& strcmp(wms_entry.data.filemark.fname,
|
||||
entry.data.filemark.fname) == 0) {
|
||||
shada_free_shada_entry(&entry);
|
||||
processed_mark = true;
|
||||
break;
|
||||
}
|
||||
@ -2262,6 +2263,8 @@ static inline ShaDaWriteResult shada_read_when_writing(
|
||||
processed_mark = true;
|
||||
if (i < ARRAY_SIZE(wms->numbered_marks)) {
|
||||
replace_numbered_mark(wms, i, pfs_entry);
|
||||
} else {
|
||||
shada_free_shada_entry(&entry);
|
||||
}
|
||||
break;
|
||||
}
|
||||
@ -2539,7 +2542,7 @@ static inline void replace_numbered_mark(WriteMergerState *const wms,
|
||||
}
|
||||
for (size_t i = idx; i < ARRAY_SIZE(wms->numbered_marks) - 1; i++) {
|
||||
if (wms->numbered_marks[i].data.type == kSDItemGlobalMark) {
|
||||
wms->numbered_marks[i].data.data.filemark.name = '0' + (char)i;
|
||||
wms->numbered_marks[i].data.data.filemark.name = '0' + (char)i + 1;
|
||||
}
|
||||
}
|
||||
memmove(wms->numbered_marks + idx + 1, wms->numbered_marks + idx,
|
||||
@ -2781,6 +2784,7 @@ static ShaDaWriteResult shada_write(ShaDaWriteDef *const sd_writer,
|
||||
// Initialize global marks
|
||||
if (dump_global_marks) {
|
||||
const void *global_mark_iter = NULL;
|
||||
size_t digit_mark_idx = 0;
|
||||
do {
|
||||
char name = NUL;
|
||||
xfmark_T fm;
|
||||
@ -2803,24 +2807,26 @@ static ShaDaWriteResult shada_write(ShaDaWriteDef *const sd_writer,
|
||||
}
|
||||
fname = (const char *) buf->b_ffname;
|
||||
}
|
||||
*(ascii_isdigit(name)
|
||||
? &wms->numbered_marks[name - '0']
|
||||
: &wms->global_marks[mark_global_index(name)]) = (
|
||||
(PossiblyFreedShadaEntry) {
|
||||
.can_free_entry = false,
|
||||
.data = {
|
||||
.type = kSDItemGlobalMark,
|
||||
.timestamp = fm.fmark.timestamp,
|
||||
.data = {
|
||||
.filemark = {
|
||||
.mark = fm.fmark.mark,
|
||||
.name = name,
|
||||
.additional_data = fm.fmark.additional_data,
|
||||
.fname = (char *)fname,
|
||||
}
|
||||
}
|
||||
},
|
||||
});
|
||||
const PossiblyFreedShadaEntry pf_entry = {
|
||||
.can_free_entry = false,
|
||||
.data = {
|
||||
.type = kSDItemGlobalMark,
|
||||
.timestamp = fm.fmark.timestamp,
|
||||
.data = {
|
||||
.filemark = {
|
||||
.mark = fm.fmark.mark,
|
||||
.name = name,
|
||||
.additional_data = fm.fmark.additional_data,
|
||||
.fname = (char *)fname,
|
||||
}
|
||||
}
|
||||
},
|
||||
};
|
||||
if (ascii_isdigit(name)) {
|
||||
replace_numbered_mark(wms, digit_mark_idx++, pf_entry);
|
||||
} else {
|
||||
wms->global_marks[mark_global_index(name)] = pf_entry;
|
||||
}
|
||||
} while (global_mark_iter != NULL);
|
||||
}
|
||||
|
||||
|
||||
@ -526,7 +526,7 @@ describe('ShaDa marks support code', function()
|
||||
end)
|
||||
|
||||
it('can merge with file with mark 9 as the only numeric mark', function()
|
||||
wshada('\007\001\018\131\162mX\195\161f\196\006' .. mock_file_path .. '-\161n9')
|
||||
wshada('\007\001\014\130\161f\196\006' .. mock_file_path .. '-\161n9')
|
||||
eq(0, exc_exec(sdrcmd()))
|
||||
nvim_command('normal! `9oabc')
|
||||
eq('-', funcs.fnamemodify(curbufmeths.get_name(), ':t'))
|
||||
@ -541,6 +541,69 @@ describe('ShaDa marks support code', function()
|
||||
eq({['0']=1, ['1']=1}, found)
|
||||
end)
|
||||
|
||||
it('removes duplicates while merging', function()
|
||||
wshada('\007\001\014\130\161f\196\006' .. mock_file_path .. '-\161n9'
|
||||
.. '\007\001\014\130\161f\196\006' .. mock_file_path .. '-\161n9')
|
||||
eq(0, exc_exec(sdrcmd()))
|
||||
eq(0, exc_exec('wshada ' .. shada_fname))
|
||||
local found = 0
|
||||
for _, v in ipairs(read_shada_file(shada_fname)) do
|
||||
if v.type == 7 and v.value.f == mock_file_path .. '-' then
|
||||
print(require('test.helpers').format_luav(v))
|
||||
found = found + 1
|
||||
end
|
||||
end
|
||||
eq(1, found)
|
||||
end)
|
||||
|
||||
it('does not leak when no append is performed due to too many marks',
|
||||
function()
|
||||
wshada('\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'a\161n0'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'b\161n1'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'c\161n2'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'd\161n3'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'e\161n4'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'f\161n5'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'g\161n6'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'h\161n7'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'i\161n8'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'j\161n9'
|
||||
.. '\007\001\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'k\161n9')
|
||||
eq(0, exc_exec(sdrcmd()))
|
||||
eq(0, exc_exec('wshada ' .. shada_fname))
|
||||
local found = {}
|
||||
for _, v in ipairs(read_shada_file(shada_fname)) do
|
||||
if v.type == 7 and v.value.f:sub(1, #mock_file_path) == mock_file_path then
|
||||
found[#found + 1] = v.value.f:sub(#v.value.f)
|
||||
end
|
||||
end
|
||||
eq({'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'}, found)
|
||||
end)
|
||||
|
||||
it('does not leak when last mark in file removes some of the earlier ones',
|
||||
function()
|
||||
wshada('\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'a\161n0'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'b\161n1'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'c\161n2'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'd\161n3'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'e\161n4'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'f\161n5'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'g\161n6'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'h\161n7'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'i\161n8'
|
||||
.. '\007\002\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'j\161n9'
|
||||
.. '\007\003\018\131\162mX\195\161f\196\006' .. mock_file_path .. 'k\161n9')
|
||||
eq(0, exc_exec(sdrcmd()))
|
||||
eq(0, exc_exec('wshada ' .. shada_fname))
|
||||
local found = {}
|
||||
for _, v in ipairs(read_shada_file(shada_fname)) do
|
||||
if v.type == 7 and v.value.f:sub(1, #mock_file_path) == mock_file_path then
|
||||
found[#found + 1] = v.value.f:sub(#v.value.f)
|
||||
end
|
||||
end
|
||||
eq({'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'k'}, found)
|
||||
end)
|
||||
|
||||
it('uses last A mark with gt timestamp from file when reading with !',
|
||||
function()
|
||||
wshada('\007\001\018\131\162mX\195\161f\196\006' .. mock_file_path .. '-\161nA')
|
||||
|
||||
Loading…
Reference in New Issue
Block a user