Require libgit2 1.4.0+. Remove data_buffer methods

This change is slightly backward incompatible. libgit2 1.4.0 made a
chagne to the `git_buf` structure and deprecated a bunch of associated
functions, to the effect that `git_buf` is no longer modifiable by
users. It is expected that `git_buf` should only be set by libgit2
itself as a return from various functions.

So this change to the `data_buffer` class removes all modifier methods.
A data_buffer is returned where libgit2 returns a `git_buf`, but we no
longer allow the caller to modify the buffer data or grow the buffer.

related: libgit2/libgit2#6078

Perhaps we should change methods returning a `data_buffer` to return a
`std::string` instead?

Author: craigds

CMakeLists.txt

@@ -12,7 +12,7 @@ if(LIBGIT2_USE_VENDORED)
 else()
   find_package(PkgConfig)
   if(PkgConfig_FOUND)
-    PKG_CHECK_MODULES(LIBGIT2 libgit2>=0.99)
+    PKG_CHECK_MODULES(LIBGIT2 libgit2>=1.4.0)
   endif()
 endif()
 

README.md

@@ -870,10 +870,7 @@ virtual const char *what() const throw() { return message_; }
 | --- | --- |
 | `git_buf_contains_nul` |  `data_buffer::contains_nul` |
 | `git_buf_dispose` | `data_buffer::~data_buffer` |
-| `git_buf_free` | `data_buffer::~data_buffer`  |
-| `git_buf_grow` | `data_buffer::grow_to_size` |
 | `git_buf_is_binary` | `data_buffer::is_binary` |
-| `git_buf_set` | `data_buffer::set_buffer` |
 
 ### checkout
 

ext/libgit2

@@ -13,10 +13,6 @@ class data_buffer : public libgit2_api {
   // Default construct a data buffer using GIT_BUF_INIT
   data_buffer();
 
-  // Construct buffer of size n
-  // Contains a git_buf with char buffer of size n
-  data_buffer(size_t n);
-
   // Construct buffer from libgit2 C ptr
   data_buffer(const git_buf *c_ptr);
 
@@ -32,16 +28,9 @@ class data_buffer : public libgit2_api {
   // Check quickly if buffer contains a NUL byte
   bool contains_nul() const;
 
-  // Resize buffer allocation to make more space
-  // Currently, this will never shrink a buffer, only expand it
-  void grow_to_size(size_t target_size);
-
   // Check quickly if buffer looks like it contains binary data
   bool is_binary() const;
 
-  // Set buffer to a copy of some raw data
-  void set_buffer(const std::string &buffer);
-
   // Get string representation of data buffer
   std::string to_string() const;
 

include/cppgit2/data_buffer.hpp

@@ -3,53 +3,34 @@
 
 namespace cppgit2 {
 
-/* Used as default value for git_buf->ptr so that people can always
- * assume ptr is non-NULL and zero terminated even for new git_bufs.
- */
-char git_buf__initbuf[1];
-/* Use to initialize buffer structure when git_buf is on stack */
-#define GIT_BUF_INIT                                                           \
-  { git_buf__initbuf, 0, 0 }
-
-data_buffer::data_buffer() { c_struct_ = GIT_BUF_INIT; }
-
-data_buffer::data_buffer(size_t n) {
-  c_struct_.ptr = (char *)malloc(n * sizeof(char));
-  if (c_struct_.ptr)
-    memset(c_struct_.ptr, '\0', n * sizeof(char));
-  c_struct_.asize = n;
-  c_struct_.size = 0;
-}
 
 data_buffer::data_buffer(const git_buf *c_ptr) {
-  c_struct_.ptr = (char *)malloc(c_ptr->asize * sizeof(char));
-  c_struct_.asize = c_ptr->asize;
+  c_struct_ = GIT_BUF_INIT;
   c_struct_.size = c_ptr->size;
-  if (c_struct_.ptr)
-    strncpy(c_struct_.ptr, c_ptr->ptr, c_ptr->asize);
+  c_struct_.ptr = c_ptr->ptr;
+  c_struct_.reserved = c_ptr->reserved;
 }
 
 data_buffer::~data_buffer() {
-  if (c_struct_.size)
-    git_buf_dispose(&c_struct_);
+  git_buf_dispose(&c_struct_);
 }
 
 data_buffer::data_buffer(data_buffer&& other) {
   c_struct_.size = other.c_struct_.size;
   c_struct_.ptr = other.c_struct_.ptr;
-  c_struct_.asize = other.c_struct_.asize;
+  c_struct_.reserved = other.c_struct_.reserved;
   other.c_struct_.size = 0;
-  other.c_struct_.asize = 0;
+  other.c_struct_.reserved = 0;
   other.c_struct_.ptr = nullptr;
 }
 
 data_buffer& data_buffer::operator= (data_buffer&& other) {
   if (other.c_struct_.ptr != c_struct_.ptr) {
     c_struct_.size = other.c_struct_.size;
     c_struct_.ptr = other.c_struct_.ptr;
-    c_struct_.asize = other.c_struct_.asize;
+    c_struct_.reserved = other.c_struct_.reserved;
     other.c_struct_.size = 0;
-    other.c_struct_.asize = 0;
+    other.c_struct_.reserved = 0;
     other.c_struct_.ptr = nullptr;
   }
   return *this;
@@ -59,18 +40,8 @@ bool data_buffer::contains_nul() const {
   return git_buf_contains_nul(&c_struct_);
 }
 
-void data_buffer::grow_to_size(size_t target_size) {
-  if (git_buf_grow(&c_struct_, target_size))
-    throw git_exception();
-}
-
 bool data_buffer::is_binary() const { return git_buf_is_binary(&c_struct_); }
 
-void data_buffer::set_buffer(const std::string &buffer) {
-  if (git_buf_set(&c_struct_, buffer.c_str(), buffer.size()))
-    throw git_exception();
-}
-
 std::string data_buffer::to_string() const {
   if (c_struct_.size)
     return std::string(c_struct_.ptr);

src/data_buffer.cpp

@@ -88,11 +88,11 @@ char diff::status_char(delta::type status) const {
 }
 
 std::string diff::to_string(diff::format format_type) const {
-  // TODO: Check this hardcoded size
-  data_buffer result;
-  if (git_diff_to_buf(result.c_ptr(), c_ptr_,
+  git_buf result_buf = GIT_BUF_INIT;
+  if (git_diff_to_buf(&result_buf, c_ptr_,
                       static_cast<git_diff_format_t>(format_type)))
     throw git_exception();
+  auto result = data_buffer(&result_buf);
   return result.to_string();
 }
 
@@ -106,9 +106,10 @@ diff::stats diff::diff_stats() const {
 }
 
 data_buffer diff::format_email(const format_email_options &options) {
-  data_buffer result;
-  if (git_diff_format_email(result.c_ptr(), c_ptr_, options.c_ptr()))
+  git_buf result_buf = GIT_BUF_INIT;
+  if (git_diff_format_email(&result_buf, c_ptr_, options.c_ptr()))
     throw git_exception();
+  auto result = data_buffer(&result_buf);
   return result;
 }
 

src/diff.cpp

@@ -132,10 +132,10 @@ void pack_builder::write(
 }
 
 data_buffer pack_builder::write_to_buffer() {
-  data_buffer result;
-  if (git_packbuilder_write_buf(result.c_ptr(), c_ptr_))
+  git_buf result = GIT_BUF_INIT;
+  if (git_packbuilder_write_buf(&result, c_ptr_))
     throw git_exception();
-  return result;
+  return data_buffer(&result);
 }
 
 size_t pack_builder::written() const { return git_packbuilder_written(c_ptr_); }

src/pack_builder.cpp

@@ -133,10 +133,10 @@ size_t patch::size(bool include_context, bool include_hunk_headers,
 }
 
 data_buffer patch::to_buffer() {
-  data_buffer result;
-  if (git_patch_to_buf(result.c_ptr(), c_ptr_))
+  git_buf result = GIT_BUF_INIT;
+  if (git_patch_to_buf(&result, c_ptr_))
     throw git_exception();
-  return result;
+  return data_buffer(&result);
 }
 
 const git_patch *patch::c_ptr() const { return c_ptr_; }

src/patch.cpp

@@ -50,10 +50,10 @@ refspec refspec::parse(const std::string &input, bool is_fetch) {
 
 data_buffer
 refspec::transform_target_to_source_reference(const std::string &name) {
-  data_buffer result;
-  if (git_refspec_rtransform(result.c_ptr(), c_ptr_, name.c_str()))
+  git_buf result = GIT_BUF_INIT;
+  if (git_refspec_rtransform(&result, c_ptr_, name.c_str()))
     throw git_exception();
-  return result;
+  return data_buffer(&result);
 }
 
 std::string refspec::source() const {
@@ -71,10 +71,10 @@ std::string refspec::to_string() const {
 }
 
 data_buffer refspec::transform_reference(const std::string &name) {
-  data_buffer result;
-  if (git_refspec_transform(result.c_ptr(), c_ptr_, name.c_str()))
+  git_buf result;
+  if (git_refspec_transform(&result, c_ptr_, name.c_str()))
     throw git_exception();
-  return result;
+  return data_buffer(&result);
 }
 
 const git_refspec *refspec::c_ptr() const { return c_ptr_; }

src/refspec.cpp

@@ -73,10 +73,10 @@ remote remote::create_remote(const std::string &url,
 }
 
 data_buffer remote::default_branch() const {
-  data_buffer result;
-  if (git_remote_default_branch(result.c_ptr(), c_ptr_))
+  git_buf result = GIT_BUF_INIT;
+  if (git_remote_default_branch(&result, c_ptr_))
     throw git_exception();
-  return result;
+  return data_buffer(&result);
 }
 
 void remote::disconnect() {

src/remote.cpp

@@ -133,12 +133,11 @@ void repository::detach_head() const {
 std::string repository::discover_path(const std::string &start_path,
                                       bool across_fs,
                                       const std::string &ceiling_dirs) {
-  // TODO: Update this hardcoded size
-  data_buffer buffer;
-  if (git_repository_discover(buffer.c_ptr(), start_path.c_str(), across_fs,
+  git_buf buffer = GIT_BUF_INIT;
+  if (git_repository_discover(&buffer, start_path.c_str(), across_fs,
                               ceiling_dirs.c_str()))
     throw git_exception();
-  return buffer.to_string();
+  return data_buffer(&buffer).to_string();
 }
 
 std::string repository::discover_path(const std::string &start_path) {
@@ -269,18 +268,19 @@ cppgit2::index repository::index() const {
 }
 
 std::string repository::path(repository::item item) const {
-  data_buffer buffer;
-  if (git_repository_item_path(buffer.c_ptr(), c_ptr_,
+  git_buf buffer = GIT_BUF_INIT;
+  if (git_repository_item_path(&buffer, c_ptr_,
                                static_cast<git_repository_item_t>(item)))
     throw git_exception();
-  return buffer.to_string();
+  return data_buffer(&buffer).to_string();
 }
 
 std::string repository::message() const {
-  data_buffer buffer;
-  if (git_repository_message(buffer.c_ptr(), c_ptr_))
+  git_buf buffer = GIT_BUF_INIT;
+  if (git_repository_message(&buffer, c_ptr_))
     throw git_exception();
-  return buffer.to_string();
+    
+  return data_buffer(&buffer).to_string();
 }
 
 void repository::remove_message() const { git_repository_message_remove(c_ptr_); }
@@ -652,10 +652,10 @@ std::string repository::branch_name(const reference &branch) const {
 }
 
 std::string repository::branch_remote_name(const std::string &refname) const {
-  data_buffer result;
-  if (git_branch_remote_name(result.c_ptr(), c_ptr_, refname.c_str()))
+  git_buf buf = GIT_BUF_INIT;
+  if (git_branch_remote_name(&buf, c_ptr_, refname.c_str()))
     throw git_exception();
-  return result.to_string();
+  return data_buffer(&buf).to_string();
 }
 
 void repository::set_branch_upstream(const reference &ref,
@@ -693,18 +693,18 @@ reference repository::branch_upstream(const std::string &local_branch_name) cons
 }
 
 std::string repository::branch_upstream_name(const std::string &refname) const {
-  data_buffer result;
-  if (git_branch_upstream_name(result.c_ptr(), c_ptr_, refname.c_str()))
+  git_buf buf = GIT_BUF_INIT;
+  if (git_branch_upstream_name(&buf, c_ptr_, refname.c_str()))
     throw git_exception();
-  return result.to_string();
+  return data_buffer(&buf).to_string();
 }
 
 std::string
 repository::branch_upstream_remote(const std::string &refname) const {
-  data_buffer result;
-  if (git_branch_upstream_remote(result.c_ptr(), c_ptr_, refname.c_str()))
+  git_buf buf = GIT_BUF_INIT;
+  if (git_branch_upstream_remote(&buf, c_ptr_, refname.c_str()))
     throw git_exception();
-  return result.to_string();
+  return data_buffer(&buf).to_string();
 }
 
 reference repository::lookup_branch(const std::string &branch_name,
@@ -797,19 +797,19 @@ data_buffer repository::create_commit(const signature &author,
                                       const std::string &message,
                                       const tree &tree,
                                       const std::vector<commit> &parents) const {
-  data_buffer result;
+  git_buf buf = GIT_BUF_INIT;
   const char *message_encoding_c =
       message_encoding == "" ? NULL : message_encoding.c_str();
   std::vector<const git_commit *> parents_c;
   for (auto &p : parents) {
     parents_c.push_back(p.c_ptr());
   }
-  if (git_commit_create_buffer(result.c_ptr(), c_ptr_, author.c_ptr(),
+  if (git_commit_create_buffer(&buf, c_ptr_, author.c_ptr(),
                                committer.c_ptr(), message_encoding_c,
                                message.c_str(), tree.c_ptr(), parents.size(),
                                parents_c.data()))
     throw git_exception();
-  return result;
+  return data_buffer(&buf);
 }
 
 oid repository::create_commit(const std::string &commit_content,
@@ -829,11 +829,11 @@ oid repository::create_commit(const std::string &commit_content,
 std::pair<data_buffer, data_buffer>
 repository::extract_signature_from_commit(oid id,
                                           const std::string &signature_field) const {
-  data_buffer sig, signed_data;
-  if (git_commit_extract_signature(sig.c_ptr(), signed_data.c_ptr(), c_ptr_,
+  git_buf sig, signed_data;
+  if (git_commit_extract_signature(&sig, &signed_data, c_ptr_,
                                    id.c_ptr(), signature_field.c_str()))
     throw git_exception();
-  return std::pair<data_buffer, data_buffer>{std::move(sig), std::move(signed_data)};
+  return std::pair<data_buffer, data_buffer>{std::move(data_buffer(&sig)), std::move(data_buffer(&signed_data))};
 }
 
 commit repository::lookup_commit(const oid &id) const {
@@ -904,12 +904,12 @@ void repository::add_ondisk_config_file(const cppgit2::config &cfg,
 data_buffer repository::create_diff_commit_as_email(
     const commit &commit, size_t patch_no, size_t total_patches,
     diff::format_email_flag flags, const diff::options &options) const {
-  data_buffer result;
-  if (git_diff_commit_as_email(result.c_ptr(), c_ptr_, commit.c_ptr_, patch_no,
+  git_buf buf = GIT_BUF_INIT;
+  if (git_diff_commit_as_email(&buf, c_ptr_, commit.c_ptr_, patch_no,
                                total_patches, static_cast<uint32_t>(flags),
                                options.c_ptr()))
     throw git_exception();
-  return result;
+  return data_buffer(&buf);
 }
 
 diff repository::create_diff_index_to_index(const cppgit2::index &old_index,
@@ -1217,10 +1217,10 @@ oid repository::remove_note(const commit &notes_commit, const signature &author,
 }
 
 data_buffer repository::detault_notes_reference() const {
-  data_buffer result;
-  if (git_note_default_ref(result.c_ptr(), c_ptr_))
+  git_buf buf = GIT_BUF_INIT;
+  if (git_note_default_ref(&buf, c_ptr_))
     throw git_exception();
-  return result;
+  return data_buffer(&buf);
 }
 
 void repository::for_each_note(
@@ -1774,10 +1774,10 @@ submodule repository::lookup_submodule(const std::string &name) const {
 }
 
 data_buffer repository::resolve_submodule_url(const std::string &url) const {
-  data_buffer result;
-  if (git_submodule_resolve_url(result.c_ptr(), c_ptr_, url.c_str()))
+  git_buf buf = GIT_BUF_INIT;
+  if (git_submodule_resolve_url(&buf, c_ptr_, url.c_str()))
     throw git_exception();
-  return result;
+  return data_buffer(&buf);
 }
 
 void repository::set_submodule_branch(const std::string &submodule_name,