Fix soundness bug on Multicursor

[yjh0502]

MultiCursor is currently unsound because of wrong lifetime. For example following example crashes with use-after-free.

    let multi = {
        let mut cursor = db.cursor();
        cursor.get_trail(0).unwrap();
        let cursors = vec![std::cell::RefCell::new(cursor)];
        traildb::MultiCursor::new(&cursors)
    };

    for _ev in multi {
        //
    }

with valgrind output

==1193941== Invalid read of size 8
==1193941==    at 0x15F269: tdb_cursor_next (traildb.h:304)
==1193941==    by 0x15F269: tdb_multi_cursor_next (tdb_multi_cursor.c:190)
==1193941==    by 0x14E536: <traildb::MultiCursor as core::iter::traits::iterator::Iterator>::next (lib.rs:519)
==1193941==    by 0x11AC82: simple::main (simple.rs:52)
==1193941==    by 0x11CFDF: std::rt::lang_start::{{closure}} (rt.rs:67)
==1193941==    by 0x16A322: {{closure}} (rt.rs:52)
==1193941==    by 0x16A322: std::panicking::try::do_call (panicking.rs:305)
==1193941==    by 0x16C266: __rust_maybe_catch_panic (lib.rs:86)
==1193941==    by 0x16ACFF: try<i32,closure-0> (panicking.rs:281)
==1193941==    by 0x16ACFF: catch_unwind<closure-0,i32> (panic.rs:394)
==1193941==    by 0x16ACFF: std::rt::lang_start_internal (rt.rs:51)
==1193941==    by 0x11CFB8: std::rt::lang_start (rt.rs:67)
==1193941==    by 0x11B119: main (in /home/jihyun/r/g/traildb-rust/target/debug/examples/simple)
==1193941==  Address 0x73f85e0 is 16 bytes inside a block of size 24 free'd
==1193941==    at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1193941==    by 0x14E1E1: <traildb::Cursor as core::ops::drop::Drop>::drop (lib.rs:460)
==1193941==    by 0x1534DE: core::ptr::drop_in_place (mod.rs:174)
==1193941==    by 0x11853E: core::ptr::drop_in_place (mod.rs:174)
==1193941==    by 0x118526: core::ptr::drop_in_place (mod.rs:174)
==1193941==    by 0x1188D4: core::ptr::drop_in_place (mod.rs:174)
==1193941==    by 0x116119: <alloc::vec::Vec<T> as core::ops::drop::Drop>::drop (vec.rs:2320)
==1193941==    by 0x118680: core::ptr::drop_in_place (mod.rs:174)
==1193941==    by 0x11AC3E: simple::main (simple.rs:50)
==1193941==    by 0x11CFDF: std::rt::lang_start::{{closure}} (rt.rs:67)
==1193941==    by 0x16A322: {{closure}} (rt.rs:52)
==1193941==    by 0x16A322: std::panicking::try::do_call (panicking.rs:305)
==1193941==    by 0x16C266: __rust_maybe_catch_panic (lib.rs:86)

I make following changes

• fix lifetime annotation on MultiCursor::new
• remove std::cell::RefCell from MultiCursor::new. I'm not sure why it uses RefCell, but it seems it's okay without RefCell.
• rustfmt
• add MultiCursor::from_iter. It allows users to write more ergonomic code like,

let cursors = vec![cursor];
let multi: traildb::MultiCursor = cursors.iter().collect(); // could be shorter if compiler could infer correct type for multi

[19h]

Thanks @yjh0502.

I'm a bit unhappy that there were so many formatting-only changes that it took some time to spot the actual changes you made. There are some things I'd have mentioned in this comment but I will do a proper code-review instead and annotate the patch.

[19h]

Is there a reason you put the FromIterator inclusion into the method itself?

[yjh0502]

The trait is only used for the method, so I thought it's better to include the trait inside the method. We could also use cursors.into_iter().collect() instead of Self::from_iter(cursors). Does it sound better?

[19h]

This is so much better..

[19h]

Is there a particular rustfmt configuration you used for the reformatted methods or did you do that manually? If it was for me I'd apply that formatting project-wide.

[19h]

I'll keep this pending for a few days to give @knutin the ability to chime in.

[yjh0502]

Sorry for noisy patch. I should made seperated rustfmt commit.
I'm using default rustfmt options without any customization. It seems most options included in rustfmt.toml are deprecated on latest rustfmt.

jihyun@dev:~/r/traildb-rust$ cat rustfmt.toml
fn_brace_style = "PreferSameLine"
struct_trailing_comma = "Always"
enum_trailing_comma = true
fn_empty_single_line = false
impl_empty_single_line = false
jihyun@dev:~/r/traildb-rust$ cargo fmt
Warning: Unknown configuration option `enum_trailing_comma`
Warning: Unknown configuration option `fn_brace_style`
Warning: Unknown configuration option `fn_empty_single_line`
Warning: Unknown configuration option `impl_empty_single_line`
Warning: Unknown configuration option `struct_trailing_comma`
Warning: Unknown configuration option `enum_trailing_comma`
Warning: Unknown configuration option `fn_brace_style`
Warning: Unknown configuration option `fn_empty_single_line`
Warning: Unknown configuration option `impl_empty_single_line`
Warning: Unknown configuration option `struct_trailing_comma`
Warning: Unknown configuration option `enum_trailing_comma`
Warning: Unknown configuration option `fn_brace_style`
Warning: Unknown configuration option `fn_empty_single_line`
Warning: Unknown configuration option `impl_empty_single_line`
Warning: Unknown configuration option `struct_trailing_comma`
Warning: Unknown configuration option `enum_trailing_comma`
Warning: Unknown configuration option `fn_brace_style`
Warning: Unknown configuration option `fn_empty_single_line`
Warning: Unknown configuration option `impl_empty_single_line`
Warning: Unknown configuration option `struct_trailing_comma`
Warning: Unknown configuration option `enum_trailing_comma`
Warning: Unknown configuration option `fn_brace_style`
Warning: Unknown configuration option `fn_empty_single_line`
Warning: Unknown configuration option `impl_empty_single_line`
Warning: Unknown configuration option `struct_trailing_comma`

[knutin]

Looks good, thanks for the patch @yjh0502!

Maybe we should just remove the rustfmt.toml and apply rustfmt to everything with the new default settings.

[JayKickliter]

Maybe we should just remove the rustfmt.toml and apply rustfmt to everything with the new default settings.

Yeah, sorry. I started this project when I had lots of opinions and little experience. These days I never customize rustfmt.