Merge pull request #1986 from libgit2/rb/error-handling-cleanups

Clean up some error handling and change callback error behavior

19 files changed, 176 insertions, 125 deletions

diff --git a/include/git2/attr.h b/include/git2/attr.h
index f256ff861..3adbb2c24 100644
--- a/include/git2/attr.h
+++ b/include/git2/attr.h
@@ -199,8 +199,9 @@ typedef int (*git_attr_foreach_cb)(const char *name, const char *value, void *pa
  *             only once per attribute name, even if there are multiple
  *             rules for a given file.  The highest priority rule will be
  *             used.  Return a non-zero value from this to stop looping.
+ *             The value will be returned from `git_attr_foreach`.
  * @param payload Passed on as extra parameter to callback function.
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_attr_foreach(
 	git_repository *repo,
diff --git a/include/git2/blob.h b/include/git2/blob.h
index dcab4fbe0..19ad4d949 100644
--- a/include/git2/blob.h
+++ b/include/git2/blob.h
@@ -159,37 +159,32 @@ typedef int (*git_blob_chunk_cb)(char *content, size_t max_length, void *payload
  * Write a loose blob to the Object Database from a
  * provider of chunks of data.
  *
- * Provided the `hintpath` parameter is filled, its value
- * will help to determine what git filters should be applied
- * to the object before it can be placed to the object database.
+ * If the `hintpath` parameter is filled, it will be used to determine
+ * what git filters should be applied to the object before it is written
+ * to the object database.
  *
+ * The implementation of the callback MUST respect the following rules:
  *
- * The implementation of the callback has to respect the
- * following rules:
+ *  - `content` must be filled by the callback. The maximum number of
+ *    bytes that the buffer can accept per call is defined by the
+ *    `max_length` parameter. Allocation and freeing of the buffer will
+ *    be taken care of by libgit2.
  *
- *  - `content` will have to be filled by the consumer. The maximum number
- * of bytes that the buffer can accept per call is defined by the
- * `max_length` parameter. Allocation and freeing of the buffer will be taken
- * care of by the function.
+ *  - The `callback` must return the number of bytes that have been
+ *    written to the `content` buffer.
  *
- *  - The callback is expected to return the number of bytes
- * that `content` have been filled with.
- *
- *  - When there is no more data to stream, the callback should
- * return 0. This will prevent it from being invoked anymore.
- *
- *  - When an error occurs, the callback should return -1.
+ *  - When there is no more data to stream, `callback` should return
+ *    0. This will prevent it from being invoked anymore.
  *
+ *  - If an error occurs, the callback should return a negative value.
+ *    This value will be returned to the caller.
  *
  * @param id Return the id of the written blob
- *
- * @param repo repository where the blob will be written.
- * This repository can be bare or not.
- *
- * @param hintpath if not NULL, will help selecting the filters
- * to apply onto the content of the blob to be created.
- *
- * @return 0 or an error code
+ * @param repo Repository where the blob will be written.
+ *        This repository can be bare or not.
+ * @param hintpath If not NULL, will be used to select data filters
+ *        to apply onto the content of the blob to be created.
+ * @return 0 or error code (from either libgit2 or callback function)
  */
 GIT_EXTERN(int) git_blob_create_fromchunks(
 	git_oid *id,
diff --git a/include/git2/checkout.h b/include/git2/checkout.h
index efafdc3e4..0e9d338c6 100644
--- a/include/git2/checkout.h
+++ b/include/git2/checkout.h
@@ -174,7 +174,12 @@ typedef enum {
  * - GIT_CHECKOUT_NOTIFY_IGNORED notifies about ignored files.
  *
  * Returning a non-zero value from this callback will cancel the checkout.
- * Notification callbacks are made prior to modifying any files on disk.
+ * The non-zero return value will be propagated back and returned by the
+ * git_checkout_... call.
+ *
+ * Notification callbacks are made prior to modifying any files on disk,
+ * so canceling on any notification will still happen prior to any files
+ * being modified.
  */
 typedef enum {
 	GIT_CHECKOUT_NOTIFY_NONE      = 0,
@@ -252,9 +257,9 @@ typedef struct git_checkout_opts {
  *
  * @param repo repository to check out (must be non-bare)
  * @param opts specifies checkout options (may be NULL)
- * @return 0 on success, GIT_EUNBORNBRANCH when HEAD points to a non existing
- * branch, GIT_ERROR otherwise (use giterr_last for information
- * about the error)
+ * @return 0 on success, GIT_EUNBORNBRANCH if HEAD points to a non
+ *         existing branch, non-zero value returned by `notify_cb`, or
+ *         other error code < 0 (use giterr_last for error details)
  */
 GIT_EXTERN(int) git_checkout_head(
 	git_repository *repo,
@@ -266,8 +271,8 @@ GIT_EXTERN(int) git_checkout_head(
  * @param repo repository into which to check out (must be non-bare)
  * @param index index to be checked out (or NULL to use repository index)
  * @param opts specifies checkout options (may be NULL)
- * @return 0 on success, GIT_ERROR otherwise (use giterr_last for information
- * about the error)
+ * @return 0 on success, non-zero return value from `notify_cb`, or error
+ *         code < 0 (use giterr_last for error details)
  */
 GIT_EXTERN(int) git_checkout_index(
 	git_repository *repo,
@@ -282,8 +287,8 @@ GIT_EXTERN(int) git_checkout_index(
  * @param treeish a commit, tag or tree which content will be used to update
  * the working directory (or NULL to use HEAD)
  * @param opts specifies checkout options (may be NULL)
- * @return 0 on success, GIT_ERROR otherwise (use giterr_last for information
- * about the error)
+ * @return 0 on success, non-zero return value from `notify_cb`, or error
+ *         code < 0 (use giterr_last for error details)
  */
 GIT_EXTERN(int) git_checkout_tree(
 	git_repository *repo,
diff --git a/include/git2/clone.h b/include/git2/clone.h
index 331cf92e7..59a73aa15 100644
--- a/include/git2/clone.h
+++ b/include/git2/clone.h
@@ -26,23 +26,25 @@ GIT_BEGIN_DECL
 /**
  * Clone options structure
  *
- * Use zeros to indicate default settings.  It's easiest to use the
- * `GIT_CLONE_OPTIONS_INIT` macro:
+ * Use the GIT_CLONE_OPTIONS_INIT to get the default settings, like this:
  *
  *		git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
  *
- * - `checkout_opts` is options for the checkout step.  To disable checkout,
- *   set the `checkout_strategy` to GIT_CHECKOUT_DEFAULT.
- * - `bare` should be set to zero to create a standard repo, non-zero for
- *   a bare repo
- * - `ignore_cert_errors` should be set to 1 if errors validating the remote host's
- *   certificate should be ignored.
+ * - `checkout_opts` are option passed to the checkout step.  To disable
+ *   checkout, set the `checkout_strategy` to GIT_CHECKOUT_NONE.
+ *   Generally you will want the use GIT_CHECKOUT_SAFE_CREATE to create
+ *   all files in the working directory for the newly cloned repository.
+ * - `bare` should be set to zero (false) to create a standard repo,
+ *   or non-zero for a bare repo
+ * - `ignore_cert_errors` should be set to 1 if errors validating the
+ *   remote host's certificate should be ignored.
  *
  *   ** "origin" remote options: **
- * - `remote_name` is the name given to the "origin" remote.  The default is
- *   "origin".
- * - `checkout_branch` gives the name of the branch to checkout. NULL means
- *   use the remote's HEAD.
+ *
+ * - `remote_name` is the name to be given to the "origin" remote.  The
+ *   default is "origin".
+ * - `checkout_branch` gives the name of the branch to checkout.  NULL
+ *   means use the remote's HEAD.
  */
 
 typedef struct git_clone_options {
@@ -70,16 +72,17 @@ typedef struct git_clone_options {
  * @param out pointer that will receive the resulting repository object
  * @param url the remote repository to clone
  * @param local_path local directory to clone to
- * @param options configuration options for the clone.  If NULL, the function
- * works as though GIT_OPTIONS_INIT were passed.
- * @return 0 on success, GIT_ERROR otherwise (use giterr_last for information
- * about the error)
+ * @param options configuration options for the clone.  If NULL, the
+ *        function works as though GIT_OPTIONS_INIT were passed.
+ * @return 0 on success, any non-zero return value from a callback
+ *         function, or a negative value to indicate an error (use
+ *         `giterr_last` for a detailed error message)
  */
 GIT_EXTERN(int) git_clone(
-		git_repository **out,
-		const char *url,
-		const char *local_path,
-		const git_clone_options *options);
+	git_repository **out,
+	const char *url,
+	const char *local_path,
+	const git_clone_options *options);
 
 /**
  * Clone into a repository
@@ -91,11 +94,17 @@ GIT_EXTERN(int) git_clone(
  * @param repo the repository to use
  * @param remote the remote repository to clone from
  * @param co_opts options to use during checkout
- * @param branch the branch to checkout after the clone, pass NULL for the remote's
- * default branch
- * @return 0 on success or an error code
+ * @param branch the branch to checkout after the clone, pass NULL for the
+ *        remote's default branch
+ * @return 0 on success, any non-zero return value from a callback
+ *         function, or a negative value to indicate an error (use
+ *         `giterr_last` for a detailed error message)
  */
-GIT_EXTERN(int) git_clone_into(git_repository *repo, git_remote *remote, const git_checkout_opts *co_opts, const char *branch);
+GIT_EXTERN(int) git_clone_into(
+	git_repository *repo,
+	git_remote *remote,
+	const git_checkout_opts *co_opts,
+	const char *branch);
 
 /** @} */
 GIT_END_DECL
diff --git a/include/git2/config.h b/include/git2/config.h
index 95da4bc03..f650f1b41 100644
--- a/include/git2/config.h
+++ b/include/git2/config.h
@@ -450,13 +450,13 @@ GIT_EXTERN(int) git_config_delete_multivar(git_config *cfg, const char *name, co
  *
  * The callback receives the normalized name and value of each variable
  * in the config backend, and the data pointer passed to this function.
- * As soon as one of the callback functions returns something other than 0,
- * this function stops iterating and returns `GIT_EUSER`.
+ * If the callback returns a non-zero value, the function stops iterating
+ * and returns that value to the caller.
  *
  * @param cfg where to get the variables from
  * @param callback the function to call on each variable
  * @param payload the data to pass to the callback
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_config_foreach(
 	const git_config *cfg,
@@ -612,8 +612,8 @@ GIT_EXTERN(int) git_config_parse_int64(int64_t *out, const char *value);
 GIT_EXTERN(int) git_config_backend_foreach_match(
 	git_config_backend *backend,
 	const char *regexp,
-	int (*fn)(const git_config_entry *, void *),
-	void *data);
+	git_config_foreach_cb callback,
+	void *payload);
 
 
 /** @} */
diff --git a/include/git2/diff.h b/include/git2/diff.h
index 315cc1215..76fb23654 100644
--- a/include/git2/diff.h
+++ b/include/git2/diff.h
@@ -468,7 +468,7 @@ typedef int (*git_diff_line_cb)(
  * Flags to control the behavior of diff rename/copy detection.
  */
 typedef enum {
-	/** Obey `diff.renames`. This is overridden by any other GIT_DIFF_FIND_ALL flag. */
+	/** Obey `diff.renames`. Overridden by any other GIT_DIFF_FIND_... flag. */
 	GIT_DIFF_FIND_BY_CONFIG = 0,
 
 	/** Look for renames? (`--find-renames`) */
@@ -577,9 +577,9 @@ typedef struct {
 	unsigned int version;
 
 	/**
-	 * Combination of git_diff_find_t values (default FIND_BY_CONFIG).
-	 * Note that if you don't explicitly set this, `diff.renames` could be set
-	 * to false, resulting in `git_diff_find_similar` doing nothing. 
+	 * Combination of git_diff_find_t values (default GIT_DIFF_FIND_BY_CONFIG).
+	 * NOTE: if you don't explicitly set this, `diff.renames` could be set
+	 * to false, resulting in `git_diff_find_similar` doing nothing.
 	 */
 	uint32_t flags;
 
@@ -870,7 +870,7 @@ GIT_EXTERN(int) git_diff_is_sorted_icase(const git_diff *diff);
  * files whose only changed is a file mode change.
  *
  * Returning a non-zero value from any of the callbacks will terminate
- * the iteration and cause this return `GIT_EUSER`.
+ * the iteration and return the value to the user.
  *
  * @param diff A git_diff generated by one of the above functions.
  * @param file_cb Callback function to make per file in the diff.
@@ -881,7 +881,7 @@ GIT_EXTERN(int) git_diff_is_sorted_icase(const git_diff *diff);
  *                same callback will be made for context lines, added, and
  *                removed lines, and even for a deleted trailing newline.
  * @param payload Reference pointer that will be passed to your callbacks.
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_diff_foreach(
 	git_diff *diff,
@@ -918,13 +918,13 @@ typedef enum {
  * Iterate over a diff generating formatted text output.
  *
  * Returning a non-zero value from the callbacks will terminate the
- * iteration and cause this return `GIT_EUSER`.
+ * iteration and return the non-zero value to the caller.
  *
  * @param diff A git_diff generated by one of the above functions.
  * @param format A git_diff_format_t value to pick the text format.
  * @param print_cb Callback to make per line of diff text.
  * @param payload Reference pointer that will be passed to your callback.
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_diff_print(
 	git_diff *diff,
@@ -964,7 +964,7 @@ GIT_EXTERN(int) git_diff_print(
  * @param hunk_cb Callback for each hunk in diff; can be NULL
  * @param line_cb Callback for each line in diff; can be NULL
  * @param payload Payload passed to each callback function
- * @return 0 on success, GIT_EUSER on non-zero callback return, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_diff_blobs(
 	const git_blob *old_blob,
@@ -999,7 +999,7 @@ GIT_EXTERN(int) git_diff_blobs(
  * @param hunk_cb Callback for each hunk in diff; can be NULL
  * @param line_cb Callback for each line in diff; can be NULL
  * @param payload Payload passed to each callback function
- * @return 0 on success, GIT_EUSER on non-zero callback return, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_diff_blob_to_buffer(
 	const git_blob *old_blob,
diff --git a/include/git2/errors.h b/include/git2/errors.h
index f1a8ea1ae..973d56003 100644
--- a/include/git2/errors.h
+++ b/include/git2/errors.h
@@ -8,7 +8,6 @@
 #define INCLUDE_git_errors_h__
 
 #include "common.h"
-#include "buffer.h"
 
 /**
  * @file git2/errors.h
@@ -20,25 +19,38 @@ GIT_BEGIN_DECL
 
 /** Generic return codes */
 typedef enum {
-	GIT_OK = 0,
-	GIT_ERROR = -1,
-	GIT_ENOTFOUND = -3,
-	GIT_EEXISTS = -4,
-	GIT_EAMBIGUOUS = -5,
-	GIT_EBUFS = -6,
-	GIT_EUSER = -7,
-	GIT_EBAREREPO = -8,
-	GIT_EUNBORNBRANCH = -9,
-	GIT_EUNMERGED = -10,
-	GIT_ENONFASTFORWARD = -11,
-	GIT_EINVALIDSPEC = -12,
-	GIT_EMERGECONFLICT = -13,
-	GIT_ELOCKED = -14,
+	GIT_OK         =  0,		/*< No error */
 
-	GIT_PASSTHROUGH = -30,
-	GIT_ITEROVER = -31,
+	GIT_ERROR      = -1,		/*< Generic error */
+	GIT_ENOTFOUND  = -3,		/*< Requested object could not be found */
+	GIT_EEXISTS    = -4,		/*< Object exists preventing operation */
+	GIT_EAMBIGUOUS = -5,		/*< More than one object matches */
+	GIT_EBUFS      = -6,		/*< Output buffer too short to hold data */
+
+	/* GIT_EUSER is a special error that is never generated by libgit2
+	 * code.  You can return it from a callback (e.g to stop an iteration)
+	 * to know that it was generated by the callback and not by libgit2.
+	 */
+	GIT_EUSER      = -7,
+
+	GIT_EBAREREPO       =  -8,	/*< Operation not allowed on bare repository */
+	GIT_EUNBORNBRANCH   =  -9,	/*< HEAD refers to branch with no commits */
+	GIT_EUNMERGED       = -10,	/*< Merge in progress prevented operation */
+	GIT_ENONFASTFORWARD = -11,	/*< Reference was not fast-forwardable */
+	GIT_EINVALIDSPEC    = -12,	/*< Name/ref spec was not in a valid format */
+	GIT_EMERGECONFLICT  = -13,	/*< Merge conflicts prevented operation */
+	GIT_ELOCKED         = -14,	/*< Lock file prevented operation */
+
+	GIT_PASSTHROUGH     = -30,	/*< Internal only */
+	GIT_ITEROVER        = -31,	/*< Signals end of iteration with iterator */
 } git_error_code;
 
+/**
+ * Structure to store extra details of the last error that occurred.
+ *
+ * This is kept on a per-thread basis if GIT_THREADS was defined when the
+ * library was build, otherwise one is kept globally for the library
+ */
 typedef struct {
 	char *message;
 	int klass;
@@ -72,6 +84,7 @@ typedef enum {
 	GITERR_SSH,
 	GITERR_FILTER,
 	GITERR_REVERT,
+	GITERR_CALLBACK,
 } git_error_t;
 
 /**
@@ -91,7 +104,7 @@ GIT_EXTERN(void) giterr_clear(void);
  * Get the last error data and clear it.
  *
  * This copies the last error into the given `git_error` struct
- * and returns 0 if the copy was successful, leaving the error 
+ * and returns 0 if the copy was successful, leaving the error
  * cleared as if `giterr_clear` had been called.
  *
  * If there was no existing error in the library, -1 will be returned
diff --git a/include/git2/index.h b/include/git2/index.h
index a60db370a..ffefad15d 100644
--- a/include/git2/index.h
+++ b/include/git2/index.h
@@ -493,7 +493,7 @@ GIT_EXTERN(int) git_index_remove_bypath(git_index *index, const char *path);
  * item in the working directory immediately *before* it is added to /
  * updated in the index.  Returning zero will add the item to the index,
  * greater than zero will skip the item, and less than zero will abort the
- * scan and cause GIT_EUSER to be returned.
+ * scan and return that value to the caller.
  *
  * @param index an existing index object
  * @param pathspec array of path patterns
@@ -502,7 +502,7 @@ GIT_EXTERN(int) git_index_remove_bypath(git_index *index, const char *path);
  *                 gets index of matching pathspec entry); can be NULL;
  *                 return 0 to add, >0 to skip, <0 to abort scan.
  * @param payload payload passed through to callback function
- * @return 0 or an error code
+ * @return 0 on success, negative callback return value, or error code
  */
 GIT_EXTERN(int) git_index_add_all(
 	git_index *index,
@@ -524,7 +524,7 @@ GIT_EXTERN(int) git_index_add_all(
  *                 gets index of matching pathspec entry); can be NULL;
  *                 return 0 to add, >0 to skip, <0 to abort scan.
  * @param payload payload passed through to callback function
- * @return 0 or an error code
+ * @return 0 on success, negative callback return value, or error code
  */
 GIT_EXTERN(int) git_index_remove_all(
 	git_index *index,
@@ -553,7 +553,7 @@ GIT_EXTERN(int) git_index_remove_all(
  *                 gets index of matching pathspec entry); can be NULL;
  *                 return 0 to add, >0 to skip, <0 to abort scan.
  * @param payload payload passed through to callback function
- * @return 0 or an error code
+ * @return 0 on success, negative callback return value, or error code
  */
 GIT_EXTERN(int) git_index_update_all(
 	git_index *index,
diff --git a/include/git2/notes.h b/include/git2/notes.h
index 76361633b..0cb6ce5f1 100644
--- a/include/git2/notes.h
+++ b/include/git2/notes.h
@@ -189,7 +189,7 @@ GIT_EXTERN(int) git_note_default_ref(const char **out, git_repository *repo);
  *
  * @param payload Extra parameter to callback function.
  *
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_note_foreach(
 	git_repository *repo,
diff --git a/include/git2/odb.h b/include/git2/odb.h
index ad56384f0..82df4d300 100644
--- a/include/git2/odb.h
+++ b/include/git2/odb.h
@@ -189,7 +189,7 @@ GIT_EXTERN(int) git_odb_refresh(struct git_odb *db);
  * @param db database to use
  * @param cb the callback to call for each object
  * @param payload data to pass to the callback
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_odb_foreach(git_odb *db, git_odb_foreach_cb cb, void *payload);
 
diff --git a/include/git2/pack.h b/include/git2/pack.h
index 88a2716bb..11bb559d8 100644
--- a/include/git2/pack.h
+++ b/include/git2/pack.h
@@ -52,7 +52,7 @@ typedef enum {
 	GIT_PACKBUILDER_ADDING_OBJECTS = 0,
 	GIT_PACKBUILDER_DELTAFICATION = 1,
 } git_packbuilder_stage_t;
-	
+
 /**
  * Initialize a new packbuilder
  *
@@ -143,6 +143,7 @@ GIT_EXTERN(int) git_packbuilder_write(
 GIT_EXTERN(const git_oid *) git_packbuilder_hash(git_packbuilder *pb);
 
 typedef int (*git_packbuilder_foreach_cb)(void *buf, size_t size, void *payload);
+
 /**
  * Create the new pack and pass each object to the callback
  *
diff --git a/include/git2/patch.h b/include/git2/patch.h
index 6a6ad92d7..e09f625c0 100644
--- a/include/git2/patch.h
+++ b/include/git2/patch.h
@@ -218,13 +218,13 @@ GIT_EXTERN(size_t) git_patch_size(
  * Serialize the patch to text via callback.
  *
  * Returning a non-zero value from the callback will terminate the iteration
- * and cause this return `GIT_EUSER`.
+ * and return that value to the caller.
  *
  * @param patch A git_patch representing changes to one file
  * @param print_cb Callback function to output lines of the patch.  Will be
  *                 called for file headers, hunk headers, and diff lines.
  * @param payload Reference pointer that will be passed to your callbacks.
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_patch_print(
 	git_patch *patch,
diff --git a/include/git2/push.h b/include/git2/push.h
index 77ef74039..12f0e7f2c 100644
--- a/include/git2/push.h
+++ b/include/git2/push.h
@@ -132,17 +132,19 @@ GIT_EXTERN(int) git_push_finish(git_push *push);
 GIT_EXTERN(int) git_push_unpack_ok(git_push *push);
 
 /**
- * Call callback `cb' on each status
+ * Invoke callback `cb' on each status entry
  *
  * For each of the updated references, we receive a status report in the
  * form of `ok refs/heads/master` or `ng refs/heads/master <msg>`.
  * `msg != NULL` means the reference has not been updated for the given
  * reason.
  *
+ * Return a non-zero value from the callback to stop the loop.
+ *
  * @param push The push object
  * @param cb The callback to call on each object
  *
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_push_status_foreach(git_push *push,
 			int (*cb)(const char *ref, const char *msg, void *data),
diff --git a/include/git2/refs.h b/include/git2/refs.h
index 4041947f6..e2bfa9615 100644
--- a/include/git2/refs.h
+++ b/include/git2/refs.h
@@ -310,20 +310,33 @@ typedef int (*git_reference_foreach_name_cb)(const char *name, void *payload);
  * Perform a callback on each reference in the repository.
  *
  * The `callback` function will be called for each reference in the
- * repository, receiving the name of the reference and the `payload` value
+ * repository, receiving the reference object and the `payload` value
  * passed to this method.  Returning a non-zero value from the callback
  * will terminate the iteration.
  *
  * @param repo Repository where to find the refs
  * @param callback Function which will be called for every listed ref
  * @param payload Additional data to pass to the callback
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_reference_foreach(
 	git_repository *repo,
 	git_reference_foreach_cb callback,
 	void *payload);
 
+/**
+ * Perform a callback on the fully-qualified name of each reference.
+ *
+ * The `callback` function will be called for each reference in the
+ * repository, receiving the name of the reference and the `payload` value
+ * passed to this method.  Returning a non-zero value from the callback
+ * will terminate the iteration.
+ *
+ * @param repo Repository where to find the refs
+ * @param callback Function which will be called for every listed ref name
+ * @param payload Additional data to pass to the callback
+ * @return 0 on success, non-zero callback return value, or error code
+ */
 GIT_EXTERN(int) git_reference_foreach_name(
 	git_repository *repo,
 	git_reference_foreach_name_cb callback,
diff --git a/include/git2/repository.h b/include/git2/repository.h
index c6bd4dca4..9f71d2959 100644
--- a/include/git2/repository.h
+++ b/include/git2/repository.h
@@ -503,14 +503,18 @@ typedef int (*git_repository_fetchhead_foreach_cb)(const char *ref_name,
 	void *payload);
 
 /**
- * Call callback 'callback' for each entry in the given FETCH_HEAD file.
+ * Invoke 'callback' for each entry in the given FETCH_HEAD file.
+ *
+ * Return a non-zero value from the callback to stop the loop.
  *
  * @param repo A repository object
  * @param callback Callback function
  * @param payload Pointer to callback data (optional)
- * @return 0 on success, GIT_ENOTFOUND, GIT_EUSER or error
+ * @return 0 on success, non-zero callback return value, GIT_ENOTFOUND if
+ *         there is no FETCH_HEAD file, or other error code.
  */
-GIT_EXTERN(int) git_repository_fetchhead_foreach(git_repository *repo,
+GIT_EXTERN(int) git_repository_fetchhead_foreach(
+	git_repository *repo,
 	git_repository_fetchhead_foreach_cb callback,
 	void *payload);
 
@@ -518,15 +522,19 @@ typedef int (*git_repository_mergehead_foreach_cb)(const git_oid *oid,
 	void *payload);
 
 /**
- * If a merge is in progress, call callback 'cb' for each commit ID in the
+ * If a merge is in progress, invoke 'callback' for each commit ID in the
  * MERGE_HEAD file.
  *
+ * Return a non-zero value from the callback to stop the loop.
+ *
  * @param repo A repository object
  * @param callback Callback function
  * @param payload Pointer to callback data (optional)
- * @return 0 on success, GIT_ENOTFOUND, GIT_EUSER or error
+ * @return 0 on success, non-zero callback return value, GIT_ENOTFOUND if
+ *         there is no MERGE_HEAD file, or other error code.
  */
-GIT_EXTERN(int) git_repository_mergehead_foreach(git_repository *repo,
+GIT_EXTERN(int) git_repository_mergehead_foreach(
+	git_repository *repo,
 	git_repository_mergehead_foreach_cb callback,
 	void *payload);
 
diff --git a/include/git2/stash.h b/include/git2/stash.h
index b48d33f5d..e2fe2cf0b 100644
--- a/include/git2/stash.h
+++ b/include/git2/stash.h
@@ -62,19 +62,15 @@ GIT_EXTERN(int) git_stash_save(
 	unsigned int flags);
 
 /**
- * When iterating over all the stashed states, callback that will be
- * issued per entry.
+ * This is a callback function you can provide to iterate over all the
+ * stashed states that will be invoked per entry.
  *
  * @param index The position within the stash list. 0 points to the
- * most recent stashed state.
- *
+ *              most recent stashed state.
  * @param message The stash message.
- *
  * @param stash_id The commit oid of the stashed state.
- *
  * @param payload Extra parameter to callback function.
- *
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 to continue iterating or non-zero to stop
  */
 typedef int (*git_stash_cb)(
 	size_t index,
@@ -89,12 +85,12 @@ typedef int (*git_stash_cb)(
  *
  * @param repo Repository where to find the stash.
  *
- * @param callback Callback to invoke per found stashed state. The most recent
- * stash state will be enumerated first.
+ * @param callback Callback to invoke per found stashed state. The most
+ *                 recent stash state will be enumerated first.
  *
  * @param payload Extra parameter to callback function.
  *
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_stash_foreach(
 	git_repository *repo,
diff --git a/include/git2/status.h b/include/git2/status.h
index 4ec3432df..aa68c0da0 100644
--- a/include/git2/status.h
+++ b/include/git2/status.h
@@ -203,12 +203,12 @@ typedef struct {
  * into this function.
  *
  * If the callback returns a non-zero value, this function will stop looping
- * and return GIT_EUSER.
+ * and return that value to caller.
  *
  * @param repo A repository object
  * @param callback The function to call on each file
  * @param payload Pointer to pass through to callback function
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_status_foreach(
 	git_repository *repo,
@@ -227,7 +227,7 @@ GIT_EXTERN(int) git_status_foreach(
  * @param opts Status options structure
  * @param callback The function to call on each file
  * @param payload Pointer to pass through to callback function
- * @return 0 on success, GIT_EUSER on non-zero callback, or error code
+ * @return 0 on success, non-zero callback return value, or error code
  */
 GIT_EXTERN(int) git_status_foreach_ext(
 	git_repository *repo,
diff --git a/include/git2/sys/filter.h b/include/git2/sys/filter.h
index 94ad3aed4..8fe21c9c0 100644
--- a/include/git2/sys/filter.h
+++ b/include/git2/sys/filter.h
@@ -149,6 +149,7 @@ typedef int (*git_filter_init_fn)(git_filter *self);
  * Specified as `filter.shutdown`, this is an optional callback invoked
  * when the filter is unregistered or when libgit2 is shutting down.  It
  * will be called once at most and should release resources as needed.
+ * This may be called even if the `initialize` callback was not made.
  *
  * Typically this function will free the `git_filter` object itself.
  */
diff --git a/include/git2/tree.h b/include/git2/tree.h
index d94b446c2..422365674 100644
--- a/include/git2/tree.h
+++ b/include/git2/tree.h
@@ -332,11 +332,18 @@ GIT_EXTERN(int) git_treebuilder_insert(
 GIT_EXTERN(int) git_treebuilder_remove(
 	git_treebuilder *bld, const char *filename);
 
+/**
+ * Callback for git_treebuilder_filter
+ *
+ * The return value is treated as a boolean, with zero indicating that the
+ * entry should be left alone and any non-zero value meaning that the
+ * entry should be removed from the treebuilder list (i.e. filtered out).
+ */
 typedef int (*git_treebuilder_filter_cb)(
 	const git_tree_entry *entry, void *payload);
 
 /**
- * Filter the entries in the tree
+ * Selectively remove entries in the tree
  *
  * The `filter` callback will be called for each entry in the tree with a
  * pointer to the entry and the provided `payload`; if the callback returns
@@ -344,7 +351,7 @@ typedef int (*git_treebuilder_filter_cb)(
  *
  * @param bld Tree builder
  * @param filter Callback to filter entries
- * @param payload Extra data to pass to filter
+ * @param payload Extra data to pass to filter callback
  */
 GIT_EXTERN(void) git_treebuilder_filter(
 	git_treebuilder *bld,