libgit2/libgit2 — pull request #2771 by carlosmn (2014-12-21 15:49:19+00:00)
Title: [WIP/RFC] Borrowing configuration entries

This code won't compile for now, as it would involve changes to every use of `_get_string()`.

We currently have "live" config objects and snapshots. The live ones are currently only useful for writing unless you're in a environment with absolutely no concurrency and you're careful. The snapshot is only useful for reading.

This makes reading a single entry a bit more hassle, as you need to take the snapshot and then free it just for the one entry. We have `git_repository_config_snapshot()` to save on temp vars, but you still need to remember to switch objects for writing. And id you use `git_repository_config()` and then perform a read, it may crash or the pointer it gives back may be already invalid.

So it'd be good to say how long an entry will be valid, and what better way than by refcounting everything. What I'm proposing here is that instead of an entry simply having the name, value and level, it should also have a way to say when we're done with it. The way I've implemented this is by using the refcount of the whole hashmap keeping the entries. Keeping one entry alive will keep everything else alive too, but you shouldn't be making use of them too long anyhow.

As part of this "making stuff safe", we cannot expect a `char *` to remain active for a live entry, so `_get_string()` uses a `git_buf` as a way to make a defensive copy which can be freed by the user. This is safe, which is good for live configs, but sucks for snapshots because now every use of `_get_string()` needs to keep track of yet another thing to free instead of relying on the snapshot, where we already know how long the string is going to live and creates a useless copy.

While it's not the end of the world, I'd like it if we could find a way to make this work by returning a `const char *` for the snapshot case but still use a `git_buf` for the live case. I've considered using two functions, and asserting that `_get_string(const char **out, ...)` can only be used for snapshot entries, but I'm not sure if the different APIs are worth the trouble.