Segmentation fault when collecting config entries in vector

[fin-ger]

Hi,

the below code snipped creates a segmentation fault:

fn main() {
    let repo = git2::Repository::open(".").unwrap();
    let config = repo.config().unwrap();
    let entries_iter = config.entries(None).unwrap();
    let entries: Vec<_> = entries_iter.map(Result::unwrap).collect();

    for entry in &entries {
        println!("name: {}", entry.name().unwrap());
    }
}

Output:

$ ./target/debug/git-segfault
[1]    498461 segmentation fault (core dumped)

The issue appears when collecting the entries of ConfigEntries and then iterating over them. The following code works just fine:

fn main() {
    let repo = git2::Repository::open(".").unwrap();
    let config = repo.config().unwrap();

    for entry in &config.entries(None).unwrap() {
        println!("name: {}", entry.unwrap().name().unwrap());
    }
}

Thank you for any help!

[ehuss]

Thanks for the report. I have filed an upstream issue at libgit2/libgit2#6289 with what I think is the issue. Either we are misusing the git iterator API, or there is an issue in its implementation.

[ehuss]

@alexcrichton Looks like upstream has defined it that config entries cannot live between successive calls to next. Unfortunately, AFAIK, that is incompatible with Rust's iterator interface. Do you have any ideas or preferences on how to fix this? Some thoughts that I've had:

• Add a new entries_cb function to Config that takes a callback instead of returning an iterator.
• Use something like streaming_iterator (I don't want to add a dependency for this, just tossing out options)
• Wait for GATs to stabilize…
• Mark entries as unsafe and document the restriction.
• Clone the name and value.

None of these seem particularly appealing to me (most are not serious suggestions).

[alexcrichton]

Could the iterator returned perhaps get its Iterator removed but still have a next method with a "fixed signature"? (e.g. it borrows the &mut self and threads that through to the ConfigEntry, preventing next again if ConfigEntry is active)

That at least paves the way for someone to later build any of the above interfaces (minus the unsafe variant). This is a pretty low-level library so while ergonomics are nice they don't seem like they're of the utmost importance.