Broken Memory Management

[josephbirkner]

Hey, I would like to report that the memory management in this library is completely broken.

Examples

repository::open

repository repository::open(const std::string &path) {
  repository result(nullptr);
  if (git_repository_open(&result.c_ptr_, path.c_str()))
    throw git_exception();
  return result;
}

Here, repository wraps a c-struct pointer that is copied to the return value. As the result variable goes out of scope, it deletes the wrapped pointer, leaving the returned struct with a dangling c_ptr_. The main issue is a lack of move, move assignment and copy constructors. This problem affects at least 30 classes.

checkout::options::set_paths

void set_paths(const std::vector<std::string> &paths) {
  c_ptr_->paths = *(strarray(paths).c_ptr());
}

Here, a temporary strarray wrapper object is constructed to convert the vector into a c-struct. The internal c-struct pointer is then copied. However, the pointer is always deleted by the strarray wrapper class, leaving c_ptr_->paths dangling. The issue here is that the pointed-to obejct is not moved or copied out of strarray. This problem affects at least 10 usages of strarray::c_ptr().

Conclusion

Until these problems are fixed, it is quite dangerous to use this library.

I have a commit here that fixes at least the missing move, move assignment and copy ctors: 0fb889f

But the c_ptr() usage is still unfixed, and I am not sure which other c_ptr() apart from strarray have the same problem in their usage.