From 2031760c626711cc69b4d63ac9798ff333583ca0 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 26 Jul 2012 16:10:22 -0700 Subject: [PATCH 1/3] Fix git_tree_walk to return user error This makes sure that an error code returned by the callback function of `git_tree_walk` will stop the iteration and get propagated back to the caller verbatim. Also, this adds a minor helper function `git_tree_entry_byoid` that searches a `git_tree` for an entry with the given OID. This isn't a fast function, but it's easier than writing the loop yourself as an external user of the library. --- include/git2/tree.h | 11 +++++++++++ src/tree.c | 34 ++++++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/include/git2/tree.h b/include/git2/tree.h index f12b15e2e..5c42f3957 100644 --- a/include/git2/tree.h +++ b/include/git2/tree.h @@ -128,6 +128,17 @@ GIT_EXTERN(const git_tree_entry *) git_tree_entry_byname(git_tree *tree, const c */ GIT_EXTERN(const git_tree_entry *) git_tree_entry_byindex(git_tree *tree, unsigned int idx); +/** + * Lookup a tree entry by SHA value. + * + * Warning: this must examine every entry in the tree, so it is not fast. + * + * @param tree a previously loaded tree. + * @param oid the sha being looked for + * @return the tree entry; NULL if not found + */ +GIT_EXTERN(const git_tree_entry *) git_tree_entry_byoid(git_tree *tree, const git_oid *oid); + /** * Get the UNIX file attributes of a tree entry * diff --git a/src/tree.c b/src/tree.c index 9d793cbb8..422e62b28 100644 --- a/src/tree.c +++ b/src/tree.c @@ -240,6 +240,21 @@ const git_tree_entry *git_tree_entry_byindex(git_tree *tree, unsigned int idx) return git_vector_get(&tree->entries, idx); } +const git_tree_entry *git_tree_entry_byoid(git_tree *tree, const git_oid *oid) +{ + unsigned int i; + git_tree_entry *e; + + assert(tree); + + git_vector_foreach(&tree->entries, i, e) { + if (memcmp(&e->oid.id, &oid->id, sizeof(oid->id)) == 0) + return e; + } + + return NULL; +} + int git_tree__prefix_position(git_tree *tree, const char *path) { git_vector *entries = &tree->entries; @@ -724,7 +739,7 @@ int git_tree_entry_bypath( } switch (path[filename_len]) { - case '/': + case '/': /* If there are more components in the path... * then this entry *must* be a tree */ if (!git_tree_entry__is_tree(entry)) { @@ -772,8 +787,9 @@ static int tree_walk( for (i = 0; i < tree->entries.length; ++i) { git_tree_entry *entry = tree->entries.contents[i]; - if (preorder && callback(path->ptr, entry, payload) < 0) - return -1; + if (preorder && + (error = callback(path->ptr, entry, payload)) != 0) + break; if (git_tree_entry__is_tree(entry)) { git_tree *subtree; @@ -790,18 +806,20 @@ static int tree_walk( if (git_buf_oom(path)) return -1; - if (tree_walk(subtree, callback, path, payload, preorder) < 0) - return -1; + error = tree_walk(subtree, callback, path, payload, preorder); + if (error != 0) + break; git_buf_truncate(path, path_len); git_tree_free(subtree); } - if (!preorder && callback(path->ptr, entry, payload) < 0) - return -1; + if (!preorder && + (error = callback(path->ptr, entry, payload)) != 0) + break; } - return 0; + return error; } int git_tree_walk(git_tree *tree, git_treewalk_cb callback, int mode, void *payload) From 5dca201072724e4230141796d7c9f8836a277de8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 3 Aug 2012 17:08:01 -0700 Subject: [PATCH 2/3] Update iterators for consistency across library This updates all the `foreach()` type functions across the library that take callbacks from the user to have a consistent behavior. The rules are: * A callback terminates the loop by returning any non-zero value * Once the callback returns non-zero, it will not be called again (i.e. the loop stops all iteration regardless of state) * If the callback returns non-zero, the parent fn returns GIT_EUSER * Although the parent returns GIT_EUSER, no error will be set in the library and `giterr_last()` will return NULL if called. This commit makes those changes across the library and adds tests for most of the iteration APIs to make sure that they follow the above rules. --- include/git2/attr.h | 21 +++--- include/git2/branch.h | 4 +- include/git2/config.h | 4 +- include/git2/diff.h | 18 +++++ include/git2/errors.h | 1 + include/git2/notes.h | 16 +++-- include/git2/odb.h | 7 +- include/git2/refs.h | 5 +- include/git2/remote.h | 5 +- include/git2/status.h | 4 +- src/attr.c | 11 ++-- src/config_file.c | 4 +- src/diff_output.c | 101 +++++++++++++++++------------ src/notes.c | 45 ++++++------- src/odb.c | 5 +- src/odb_loose.c | 14 ++-- src/odb_pack.c | 9 ++- src/pack.c | 11 ++-- src/path.h | 1 + src/refs.c | 22 +++++-- src/status.c | 22 +++++-- src/transports/git.c | 6 +- src/transports/http.c | 6 +- src/transports/local.c | 4 +- tests-clar/attr/repo.c | 22 +++++++ tests-clar/config/read.c | 2 +- tests-clar/diff/index.c | 50 ++++++++++++++ tests-clar/notes/notes.c | 30 ++++++++- tests-clar/odb/foreach.c | 18 +++++ tests-clar/refs/branches/foreach.c | 25 +++++++ tests-clar/refs/foreachglob.c | 22 +++++++ tests-clar/status/worktree.c | 33 ++++++++-- 32 files changed, 401 insertions(+), 147 deletions(-) diff --git a/include/git2/attr.h b/include/git2/attr.h index fad7183da..73a625a7e 100644 --- a/include/git2/attr.h +++ b/include/git2/attr.h @@ -172,18 +172,17 @@ GIT_EXTERN(int) git_attr_get_many( * * @param repo The repository containing the path. * @param flags A combination of GIT_ATTR_CHECK... flags. - * @param path The path inside the repo to check attributes. This - * does not have to exist, but if it does not, then - * it will be treated as a plain file (i.e. not a directory). - * @param callback The function that will be invoked on each attribute - * and attribute value. The name parameter will be the name - * of the attribute and the value will be the value it is - * set to, including possibly NULL if the attribute is - * explicitly set to UNSPECIFIED using the ! sign. This - * will be invoked only once per attribute name, even if - * there are multiple rules for a given file. The highest - * priority rule will be used. + * @param path Path inside the repo to check attributes. This does not have + * to exist, but if it does not, then it will be treated as a + * plain file (i.e. not a directory). + * @param callback Function to invoke on each attribute name and value. The + * value may be NULL is the attribute is explicitly set to + * UNSPECIFIED using the '!' sign. Callback will be invoked + * 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. * @param payload Passed on as extra parameter to callback function. + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_attr_foreach( git_repository *repo, diff --git a/include/git2/branch.h b/include/git2/branch.h index 8884df15a..c8f2d8f5f 100644 --- a/include/git2/branch.h +++ b/include/git2/branch.h @@ -74,6 +74,8 @@ GIT_EXTERN(int) git_branch_delete( /** * Loop over all the branches and issue a callback for each one. * + * If the callback returns a non-zero value, this will stop looping. + * * @param repo Repository where to find the branches. * * @param list_flags Filtering flags for the branch @@ -84,7 +86,7 @@ GIT_EXTERN(int) git_branch_delete( * * @param payload Extra parameter to callback function. * - * @return 0 or an error code. + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_branch_foreach( git_repository *repo, diff --git a/include/git2/config.h b/include/git2/config.h index c46e7fc9d..8a36885c7 100644 --- a/include/git2/config.h +++ b/include/git2/config.h @@ -302,12 +302,12 @@ GIT_EXTERN(int) git_config_delete(git_config *cfg, const char *name); * 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 returns that value. + * this function stops iterating and returns `GIT_EUSER`. * * @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 or the return value of the callback which didn't return 0 + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_config_foreach( git_config *cfg, diff --git a/include/git2/diff.h b/include/git2/diff.h index 85727d969..79ef7a49b 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -332,6 +332,9 @@ GIT_EXTERN(int) git_diff_merge( * callbacks will not be invoked for binary files on the diff list or for * 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`. + * * @param diff A git_diff_list generated by one of the above functions. * @param cb_data Reference pointer that will be passed to your callbacks. * @param file_cb Callback function to make per file in the diff. @@ -341,6 +344,7 @@ GIT_EXTERN(int) git_diff_merge( * @param line_cb Optional callback to make per line of diff text. This * same callback will be made for context lines, added, and * removed lines, and even for a deleted trailing newline. + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_diff_foreach( git_diff_list *diff, @@ -351,6 +355,14 @@ GIT_EXTERN(int) git_diff_foreach( /** * Iterate over a diff generating text output like "git diff --name-status". + * + * Returning a non-zero value from the callbacks will terminate the + * iteration and cause this return `GIT_EUSER`. + * + * @param diff A git_diff_list generated by one of the above functions. + * @param cb_data Reference pointer that will be passed to your callback. + * @param print_cb Callback to make per line of diff text. + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_diff_print_compact( git_diff_list *diff, @@ -362,6 +374,9 @@ GIT_EXTERN(int) git_diff_print_compact( * * This is a super easy way to generate a patch from a diff. * + * Returning a non-zero value from the callbacks will terminate the + * iteration and cause this return `GIT_EUSER`. + * * @param diff A git_diff_list generated by one of the above functions. * @param cb_data Reference pointer that will be passed to your callbacks. * @param print_cb Callback function to output lines of the diff. This @@ -369,6 +384,7 @@ GIT_EXTERN(int) git_diff_print_compact( * headers, and diff lines. Fortunately, you can probably * use various GIT_DIFF_LINE constants to determine what * text you are given. + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_diff_print_patch( git_diff_list *diff, @@ -393,6 +409,8 @@ GIT_EXTERN(int) git_diff_print_patch( * When at least one of the blobs being dealt with is binary, the * `git_diff_delta` binary attribute will be set to 1 and no call to the * hunk_cb nor line_cb will be made. + * + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_diff_blobs( git_blob *old_blob, diff --git a/include/git2/errors.h b/include/git2/errors.h index ca7f0de6e..2ab1da403 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -25,6 +25,7 @@ enum { GIT_EEXISTS = -4, GIT_EAMBIGUOUS = -5, GIT_EBUFS = -6, + GIT_EUSER = -7, GIT_PASSTHROUGH = -30, GIT_REVWALKOVER = -31, diff --git a/include/git2/notes.h b/include/git2/notes.h index 19073abd1..b4839bec3 100644 --- a/include/git2/notes.h +++ b/include/git2/notes.h @@ -119,19 +119,21 @@ typedef struct { * * @param repo Repository where to find the notes. * - * @param notes_ref OID reference to read from (optional); defaults to "refs/notes/commits". + * @param notes_ref OID reference to read from (optional); defaults to + * "refs/notes/commits". * - * @param note_cb Callback to invoke per found annotation. + * @param note_cb Callback to invoke per found annotation. Return non-zero + * to stop looping. * * @param payload Extra parameter to callback function. * - * @return 0 or an error code. + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_note_foreach( - git_repository *repo, - const char *notes_ref, - int (*note_cb)(git_note_data *note_data, void *payload), - void *payload + git_repository *repo, + const char *notes_ref, + int (*note_cb)(git_note_data *note_data, void *payload), + void *payload ); /** @} */ diff --git a/include/git2/odb.h b/include/git2/odb.h index dac9e06a9..1f25db463 100644 --- a/include/git2/odb.h +++ b/include/git2/odb.h @@ -176,13 +176,14 @@ GIT_EXTERN(int) git_odb_exists(git_odb *db, const git_oid *id); * List all objects available in the database * * The callback will be called for each object available in the - * database. Note that the objects are likely to be returned in the - * index order, which would make accessing the objects in that order - * inefficient. + * database. Note that the objects are likely to be returned in the index + * order, which would make accessing the objects in that order inefficient. + * Return a non-zero value from the callback to stop looping. * * @param db database to use * @param cb the callback to call for each object * @param data data to pass to the callback + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_odb_foreach(git_odb *db, int (*cb)(git_oid *oid, void *data), void *data); diff --git a/include/git2/refs.h b/include/git2/refs.h index b119e90b1..dbd9b7151 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -268,14 +268,15 @@ GIT_EXTERN(int) git_reference_list(git_strarray *array, git_repository *repo, un * * The `callback` function will be called for each of the references * in the repository, and will receive the name of the reference and - * the `payload` value passed to this method. + * 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 list_flags Filtering flags for the reference * listing. * @param callback Function which will be called for every listed ref * @param payload Additional data to pass to the callback - * @return 0 or an error code + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_reference_foreach(git_repository *repo, unsigned int list_flags, int (*callback)(const char *, void *), void *payload); diff --git a/include/git2/remote.h b/include/git2/remote.h index 5c01949d2..02b93e099 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -133,9 +133,12 @@ GIT_EXTERN(int) git_remote_connect(git_remote *remote, int direction); * The remote (or more exactly its transport) must be connected. The * memory belongs to the remote. * + * If you a return a non-zero value from the callback, this will stop + * looping over the refs. + * * @param refs where to store the refs * @param remote the remote - * @return 0 or an error code + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_remote_ls(git_remote *remote, git_headlist_cb list_cb, void *payload); diff --git a/include/git2/status.h b/include/git2/status.h index 9e7b5de4a..cc94d7680 100644 --- a/include/git2/status.h +++ b/include/git2/status.h @@ -38,11 +38,11 @@ enum { * * The callback is passed the path of the file, the status and the data * pointer passed to this function. If the callback returns something other - * than 0, this function will return that value. + * than 0, this function will stop looping and return GIT_EUSER. * * @param repo a repository object * @param callback the function to call on each file - * @return 0 on success or the return value of the callback that was non-zero + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ GIT_EXTERN(int) git_status_foreach( git_repository *repo, diff --git a/src/attr.c b/src/attr.c index 6fbd005d5..c58a1f045 100644 --- a/src/attr.c +++ b/src/attr.c @@ -163,11 +163,14 @@ int git_attr_foreach( continue; git_strmap_insert(seen, assign->name, assign, error); - if (error >= 0) - error = callback(assign->name, assign->value, payload); - - if (error != 0) + if (error < 0) goto cleanup; + + error = callback(assign->name, assign->value, payload); + if (error) { + error = GIT_EUSER; + goto cleanup; + } } } } diff --git a/src/config_file.c b/src/config_file.c index 7ced1e5ba..80c63d2a3 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -218,8 +218,10 @@ static int file_foreach( continue; /* abort iterator on non-zero return value */ - if ((result = fn(key, var->value, data)) != 0) + if (fn(key, var->value, data)) { + result = GIT_EUSER; goto cleanup; + } } ); diff --git a/src/diff_output.c b/src/diff_output.c index f6650b345..9f8779787 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -23,6 +23,7 @@ typedef struct { unsigned int index; git_diff_delta *delta; git_diff_range range; + int error; } diff_output_info; static int read_next_int(const char **str, int *value) @@ -49,25 +50,24 @@ static int diff_output_cb(void *priv, mmbuffer_t *bufs, int len) /* expect something of the form "@@ -%d[,%d] +%d[,%d] @@" */ if (*scan != '@') - return -1; + info->error = -1; + else if (read_next_int(&scan, &range.old_start) < 0) + info->error = -1; + else if (*scan == ',' && read_next_int(&scan, &range.old_lines) < 0) + info->error = -1; + else if (read_next_int(&scan, &range.new_start) < 0) + info->error = -1; + else if (*scan == ',' && read_next_int(&scan, &range.new_lines) < 0) + info->error = -1; + else if (range.old_start < 0 || range.new_start < 0) + info->error = -1; + else { + memcpy(&info->range, &range, sizeof(git_diff_range)); - if (read_next_int(&scan, &range.old_start) < 0) - return -1; - if (*scan == ',' && read_next_int(&scan, &range.old_lines) < 0) - return -1; - - if (read_next_int(&scan, &range.new_start) < 0) - return -1; - if (*scan == ',' && read_next_int(&scan, &range.new_lines) < 0) - return -1; - - if (range.old_start < 0 || range.new_start < 0) - return -1; - - memcpy(&info->range, &range, sizeof(git_diff_range)); - - return info->hunk_cb( - info->cb_data, info->delta, &range, bufs[0].ptr, bufs[0].size); + if (info->hunk_cb( + info->cb_data, info->delta, &range, bufs[0].ptr, bufs[0].size)) + info->error = GIT_EUSER; + } } if ((len == 2 || len == 3) && info->line_cb) { @@ -80,23 +80,24 @@ static int diff_output_cb(void *priv, mmbuffer_t *bufs, int len) GIT_DIFF_LINE_CONTEXT; if (info->line_cb( - info->cb_data, info->delta, &info->range, origin, bufs[1].ptr, bufs[1].size) < 0) - return -1; + info->cb_data, info->delta, &info->range, origin, bufs[1].ptr, bufs[1].size)) + info->error = GIT_EUSER; /* This should only happen if we are adding a line that does not * have a newline at the end and the old code did. In that case, * we have a ADD with a DEL_EOFNL as a pair. */ - if (len == 3) { + else if (len == 3) { origin = (origin == GIT_DIFF_LINE_ADDITION) ? GIT_DIFF_LINE_DEL_EOFNL : GIT_DIFF_LINE_ADD_EOFNL; - return info->line_cb( - info->cb_data, info->delta, &info->range, origin, bufs[2].ptr, bufs[2].size); + if (info->line_cb( + info->cb_data, info->delta, &info->range, origin, bufs[2].ptr, bufs[2].size)) + info->error = GIT_EUSER; } } - return 0; + return info->error; } #define BINARY_DIFF_FLAGS (GIT_DIFF_FILE_BINARY|GIT_DIFF_FILE_NOT_BINARY) @@ -318,6 +319,7 @@ int git_diff_foreach( xdemitconf_t xdiff_config; xdemitcb_t xdiff_callback; + memset(&info, 0, sizeof(info)); info.diff = diff; info.cb_data = data; info.hunk_cb = hunk_cb; @@ -422,11 +424,11 @@ int git_diff_foreach( * diffs to tell if a file has really been changed. */ - if (file_cb != NULL) { - error = file_cb( - data, delta, (float)info.index / diff->deltas.length); - if (error < 0) - goto cleanup; + if (file_cb != NULL && + file_cb(data, delta, (float)info.index / diff->deltas.length)) + { + error = GIT_EUSER; + goto cleanup; } /* don't do hunk and line diffs if file is binary */ @@ -451,6 +453,7 @@ int git_diff_foreach( xdl_diff(&old_xdiff_data, &new_xdiff_data, &xdiff_params, &xdiff_config, &xdiff_callback); + error = info.error; cleanup: release_content(&delta->old_file, &old_data, old_blob); @@ -524,7 +527,11 @@ static int print_compact(void *data, git_diff_delta *delta, float progress) if (git_buf_oom(pi->buf)) return -1; - return pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_FILE_HDR, git_buf_cstr(pi->buf), git_buf_len(pi->buf)); + if (pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_FILE_HDR, + git_buf_cstr(pi->buf), git_buf_len(pi->buf))) + return GIT_EUSER; + + return 0; } int git_diff_print_compact( @@ -586,7 +593,6 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress) const char *oldpath = delta->old_file.path; const char *newpfx = pi->diff->opts.new_prefix; const char *newpath = delta->new_file.path; - int result; GIT_UNUSED(progress); @@ -619,9 +625,8 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress) if (git_buf_oom(pi->buf)) return -1; - result = pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_FILE_HDR, git_buf_cstr(pi->buf), git_buf_len(pi->buf)); - if (result < 0) - return result; + if (pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_FILE_HDR, git_buf_cstr(pi->buf), git_buf_len(pi->buf))) + return GIT_EUSER; if (delta->binary != 1) return 0; @@ -633,7 +638,11 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress) if (git_buf_oom(pi->buf)) return -1; - return pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_BINARY, git_buf_cstr(pi->buf), git_buf_len(pi->buf)); + if (pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_BINARY, + git_buf_cstr(pi->buf), git_buf_len(pi->buf))) + return GIT_EUSER; + + return 0; } static int print_patch_hunk( @@ -649,7 +658,11 @@ static int print_patch_hunk( if (git_buf_printf(pi->buf, "%.*s", (int)header_len, header) < 0) return -1; - return pi->print_cb(pi->cb_data, d, r, GIT_DIFF_LINE_HUNK_HDR, git_buf_cstr(pi->buf), git_buf_len(pi->buf)); + if (pi->print_cb(pi->cb_data, d, r, GIT_DIFF_LINE_HUNK_HDR, + git_buf_cstr(pi->buf), git_buf_len(pi->buf))) + return GIT_EUSER; + + return 0; } static int print_patch_line( @@ -674,7 +687,11 @@ static int print_patch_line( if (git_buf_oom(pi->buf)) return -1; - return pi->print_cb(pi->cb_data, delta, range, line_origin, git_buf_cstr(pi->buf), git_buf_len(pi->buf)); + if (pi->print_cb(pi->cb_data, delta, range, line_origin, + git_buf_cstr(pi->buf), git_buf_len(pi->buf))) + return GIT_EUSER; + + return 0; } int git_diff_print_patch( @@ -763,11 +780,8 @@ int git_diff_blobs( if (file_is_binary_by_content(&delta, &old_map, &new_map) < 0) return -1; - if (file_cb != NULL) { - int error = file_cb(cb_data, &delta, 1); - if (error < 0) - return error; - } + if (file_cb != NULL && file_cb(cb_data, &delta, 1)) + return GIT_EUSER; /* don't do hunk and line diffs if the two blobs are identical */ if (delta.status == GIT_DELTA_UNMODIFIED) @@ -777,6 +791,7 @@ int git_diff_blobs( if (delta.binary == 1) return 0; + memset(&info, 0, sizeof(info)); info.diff = NULL; info.delta = δ info.cb_data = cb_data; @@ -790,5 +805,5 @@ int git_diff_blobs( xdl_diff(&old_data, &new_data, &xdiff_params, &xdiff_config, &xdiff_callback); - return 0; + return info.error; } diff --git a/src/notes.c b/src/notes.c index 7813e9985..212413a5a 100644 --- a/src/notes.c +++ b/src/notes.c @@ -522,13 +522,13 @@ static int process_entry_path( int (*note_cb)(git_note_data *note_data, void *payload), void *payload) { - int i = 0, j = 0, error = -1, len; + int i = 0, j = 0, error, len; git_buf buf = GIT_BUF_INIT; git_note_data note_data; - if (git_buf_puts(&buf, entry_path) < 0) + if ((error = git_buf_puts(&buf, entry_path)) < 0) goto cleanup; - + len = git_buf_len(&buf); while (i < len) { @@ -536,10 +536,9 @@ static int process_entry_path( i++; continue; } - + if (git__fromhex(buf.ptr[i]) < 0) { /* This is not a note entry */ - error = 0; goto cleanup; } @@ -555,16 +554,17 @@ static int process_entry_path( if (j != GIT_OID_HEXSZ) { /* This is not a note entry */ - error = 0; goto cleanup; } - if (git_oid_fromstr(¬e_data.annotated_object_oid, buf.ptr) < 0) - return -1; + if ((error = git_oid_fromstr( + ¬e_data.annotated_object_oid, buf.ptr)) < 0) + goto cleanup; git_oid_cpy(¬e_data.blob_oid, note_oid); - error = note_cb(¬e_data, payload); + if (note_cb(¬e_data, payload)) + error = GIT_EUSER; cleanup: git_buf_free(&buf); @@ -577,34 +577,27 @@ int git_note_foreach( int (*note_cb)(git_note_data *note_data, void *payload), void *payload) { - int error = -1; + int error; git_iterator *iter = NULL; git_tree *tree = NULL; git_commit *commit = NULL; const git_index_entry *item; - if ((error = retrieve_note_tree_and_commit(&tree, &commit, repo, ¬es_ref)) < 0) - goto cleanup; + if (!(error = retrieve_note_tree_and_commit( + &tree, &commit, repo, ¬es_ref)) && + !(error = git_iterator_for_tree(&iter, repo, tree))) + error = git_iterator_current(iter, &item); - if (git_iterator_for_tree(&iter, repo, tree) < 0) - goto cleanup; + while (!error && item) { + error = process_entry_path(item->path, &item->oid, note_cb, payload); - if (git_iterator_current(iter, &item) < 0) - goto cleanup; - - while (item) { - if (process_entry_path(item->path, &item->oid, note_cb, payload) < 0) - goto cleanup; - - if (git_iterator_advance(iter, &item) < 0) - goto cleanup; + if (!error) + error = git_iterator_advance(iter, &item); } - error = 0; - -cleanup: git_iterator_free(iter); git_tree_free(tree); git_commit_free(commit); + return error; } diff --git a/src/odb.c b/src/odb.c index 493c8292a..db2f03c9e 100644 --- a/src/odb.c +++ b/src/odb.c @@ -609,9 +609,12 @@ int git_odb_foreach(git_odb *db, int (*cb)(git_oid *oid, void *data), void *data { unsigned int i; backend_internal *internal; + git_vector_foreach(&db->backends, i, internal) { git_odb_backend *b = internal->backend; - b->foreach(b, cb, data); + int error = b->foreach(b, cb, data); + if (error < 0) + return error; } return 0; diff --git a/src/odb_loose.c b/src/odb_loose.c index 2197a4264..ccb899e8c 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -680,6 +680,7 @@ struct foreach_state { size_t dir_len; int (*cb)(git_oid *oid, void *data); void *data; + int cb_error; }; GIT_INLINE(int) filename_to_oid(git_oid *oid, const char *ptr) @@ -718,8 +719,10 @@ static int foreach_object_dir_cb(void *_state, git_buf *path) if (filename_to_oid(&oid, path->ptr + state->dir_len) < 0) return 0; - if (state->cb(&oid, state->data) < 0) + if (state->cb(&oid, state->data)) { + state->cb_error = GIT_EUSER; return -1; + } return 0; } @@ -728,10 +731,7 @@ static int foreach_cb(void *_state, git_buf *path) { struct foreach_state *state = (struct foreach_state *) _state; - if (git_path_direach(path, foreach_object_dir_cb, state) < 0) - return -1; - - return 0; + return git_path_direach(path, foreach_object_dir_cb, state); } static int loose_backend__foreach(git_odb_backend *_backend, int (*cb)(git_oid *oid, void *data), void *data) @@ -749,14 +749,16 @@ static int loose_backend__foreach(git_odb_backend *_backend, int (*cb)(git_oid * git_buf_sets(&buf, objects_dir); git_path_to_dir(&buf); + memset(&state, 0, sizeof(state)); state.cb = cb; state.data = data; state.dir_len = git_buf_len(&buf); error = git_path_direach(&buf, foreach_cb, &state); + git_buf_free(&buf); - return error; + return state.cb_error ? state.cb_error : error; } static int loose_backend__stream_fwrite(git_oid *oid, git_odb_stream *_stream) diff --git a/src/odb_pack.c b/src/odb_pack.c index 4b860e864..176be5f01 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -422,6 +422,7 @@ static int pack_backend__exists(git_odb_backend *backend, const git_oid *oid) static int pack_backend__foreach(git_odb_backend *_backend, int (*cb)(git_oid *oid, void *data), void *data) { + int error; struct git_pack_file *p; struct pack_backend *backend; unsigned int i; @@ -430,12 +431,14 @@ static int pack_backend__foreach(git_odb_backend *_backend, int (*cb)(git_oid *o backend = (struct pack_backend *)_backend; /* Make sure we know about the packfiles */ - if (packfile_refresh_all(backend) < 0) - return -1; + if ((error = packfile_refresh_all(backend)) < 0) + return error; git_vector_foreach(&backend->packs, i, p) { - git_pack_foreach_entry(p, cb, &data); + if ((error = git_pack_foreach_entry(p, cb, &data)) < 0) + return error; } + return 0; } diff --git a/src/pack.c b/src/pack.c index 1d88eaa7d..acdb40d35 100644 --- a/src/pack.c +++ b/src/pack.c @@ -687,10 +687,9 @@ static git_off_t nth_packed_object_offset(const struct git_pack_file *p, uint32_ } int git_pack_foreach_entry( - struct git_pack_file *p, - int (*cb)(git_oid *oid, void *data), - void *data) - + struct git_pack_file *p, + int (*cb)(git_oid *oid, void *data), + void *data) { const unsigned char *index = p->index_map.data, *current; unsigned stride; @@ -722,7 +721,9 @@ int git_pack_foreach_entry( current = index; for (i = 0; i < p->num_objects; i++) { - cb((git_oid *)current, data); + if (cb((git_oid *)current, data)) + return GIT_EUSER; + current += stride; } diff --git a/src/path.h b/src/path.h index d68393b3d..d611428c1 100644 --- a/src/path.h +++ b/src/path.h @@ -217,6 +217,7 @@ extern int git_path_apply_relative(git_buf *target, const char *relpath); * the input state and the second arg is pathbuf. The function * may modify the pathbuf, but only by appending new text. * @param state to pass to fn as the first arg. + * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ extern int git_path_direach( git_buf *pathbuf, diff --git a/src/refs.c b/src/refs.c index b3c140bec..723695cd6 100644 --- a/src/refs.c +++ b/src/refs.c @@ -501,6 +501,7 @@ struct dirent_list_data { int (*callback)(const char *, void *); void *callback_payload; + int callback_error; }; static int _dirent_loose_listall(void *_data, git_buf *full_path) @@ -521,7 +522,10 @@ static int _dirent_loose_listall(void *_data, git_buf *full_path) return 0; /* we are filtering out this reference */ } - return data->callback(file_path, data->callback_payload); + if (data->callback(file_path, data->callback_payload)) + data->callback_error = GIT_EUSER; + + return data->callback_error; } static int _dirent_loose_load(void *data, git_buf *full_path) @@ -844,15 +848,17 @@ static int reference_path_available( const char *ref, const char* old_ref) { + int error; struct reference_available_t data; data.new_ref = ref; data.old_ref = old_ref; data.available = 1; - if (git_reference_foreach(repo, GIT_REF_LISTALL, - _reference_available_cb, (void *)&data) < 0) - return -1; + error = git_reference_foreach( + repo, GIT_REF_LISTALL, _reference_available_cb, (void *)&data); + if (error < 0) + return error; if (!data.available) { giterr_set(GITERR_REFERENCE, @@ -1487,8 +1493,8 @@ int git_reference_foreach( return -1; git_strmap_foreach(repo->references.packfile, ref_name, ref, { - if (callback(ref_name, payload) < 0) - return 0; + if (callback(ref_name, payload)) + return GIT_EUSER; }); } @@ -1500,14 +1506,16 @@ int git_reference_foreach( data.repo = repo; data.callback = callback; data.callback_payload = payload; + data.callback_error = 0; if (git_buf_joinpath(&refs_path, repo->path_repository, GIT_REFS_DIR) < 0) return -1; result = git_path_direach(&refs_path, _dirent_loose_listall, &data); + git_buf_free(&refs_path); - return result; + return data.callback_error ? GIT_EUSER : result; } static int cb__reflist_add(const char *ref, void *data) diff --git a/src/status.c b/src/status.c index d78237689..618f60fd0 100644 --- a/src/status.c +++ b/src/status.c @@ -114,7 +114,8 @@ int git_status_foreach_ext( if (show == GIT_STATUS_SHOW_INDEX_THEN_WORKDIR) { for (i = 0; !err && i < idx2head->deltas.length; i++) { i2h = GIT_VECTOR_GET(&idx2head->deltas, i); - err = cb(i2h->old_file.path, index_delta2status(i2h->status), cbdata); + if (cb(i2h->old_file.path, index_delta2status(i2h->status), cbdata)) + err = GIT_EUSER; } git_diff_list_free(idx2head); idx2head = NULL; @@ -130,14 +131,17 @@ int git_status_foreach_ext( cmp = !w2i ? -1 : !i2h ? 1 : strcmp(i2h->old_file.path, w2i->old_file.path); if (cmp < 0) { - err = cb(i2h->old_file.path, index_delta2status(i2h->status), cbdata); + if (cb(i2h->old_file.path, index_delta2status(i2h->status), cbdata)) + err = GIT_EUSER; i++; } else if (cmp > 0) { - err = cb(w2i->old_file.path, workdir_delta2status(w2i->status), cbdata); + if (cb(w2i->old_file.path, workdir_delta2status(w2i->status), cbdata)) + err = GIT_EUSER; j++; } else { - err = cb(i2h->old_file.path, index_delta2status(i2h->status) | - workdir_delta2status(w2i->status), cbdata); + if (cb(i2h->old_file.path, index_delta2status(i2h->status) | + workdir_delta2status(w2i->status), cbdata)) + err = GIT_EUSER; i++; j++; } } @@ -146,6 +150,7 @@ cleanup: git_tree_free(head); git_diff_list_free(idx2head); git_diff_list_free(wd2idx); + return err; } @@ -166,9 +171,10 @@ int git_status_foreach( } struct status_file_info { + char *expected; unsigned int count; unsigned int status; - char *expected; + int ambiguous; }; static int get_one_status(const char *path, unsigned int status, void *data) @@ -183,6 +189,7 @@ static int get_one_status(const char *path, unsigned int status, void *data) p_fnmatch(sfi->expected, path, 0) != 0)) { giterr_set(GITERR_INVALID, "Ambiguous path '%s' given to git_status_file", sfi->expected); + sfi->ambiguous = true; return GIT_EAMBIGUOUS; } @@ -215,6 +222,9 @@ int git_status_file( error = git_status_foreach_ext(repo, &opts, get_one_status, &sfi); + if (error < 0 && sfi.ambiguous) + error = GIT_EAMBIGUOUS; + if (!error && !sfi.count) { giterr_set(GITERR_INVALID, "Attempt to get status of nonexistent file '%s'", path); diff --git a/src/transports/git.c b/src/transports/git.c index 45f571f20..0d0ec7821 100644 --- a/src/transports/git.c +++ b/src/transports/git.c @@ -239,10 +239,8 @@ static int git_ls(git_transport *transport, git_headlist_cb list_cb, void *opaqu pkt = (git_pkt_ref *)p; - if (list_cb(&pkt->head, opaque) < 0) { - giterr_set(GITERR_NET, "User callback returned error"); - return -1; - } + if (list_cb(&pkt->head, opaque)) + return GIT_EUSER; } return 0; diff --git a/src/transports/http.c b/src/transports/http.c index f25d639f3..993070aac 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -324,10 +324,8 @@ static int http_ls(git_transport *transport, git_headlist_cb list_cb, void *opaq if (p->type != GIT_PKT_REF) continue; - if (list_cb(&p->head, opaque) < 0) { - giterr_set(GITERR_NET, "The user callback returned error"); - return -1; - } + if (list_cb(&p->head, opaque)) + return GIT_EUSER; } return 0; diff --git a/src/transports/local.c b/src/transports/local.c index 0e1ae3752..ccbfb0492 100644 --- a/src/transports/local.c +++ b/src/transports/local.c @@ -126,8 +126,8 @@ static int local_ls(git_transport *transport, git_headlist_cb list_cb, void *pay assert(transport && transport->connected); git_vector_foreach(refs, i, h) { - if (list_cb(h, payload) < 0) - return -1; + if (list_cb(h, payload)) + return GIT_EUSER; } return 0; diff --git a/tests-clar/attr/repo.c b/tests-clar/attr/repo.c index c37ff544a..4a317e4f3 100644 --- a/tests-clar/attr/repo.c +++ b/tests-clar/attr/repo.c @@ -113,6 +113,22 @@ static int count_attrs( return 0; } +static int cancel_iteration( + const char *name, + const char *value, + void *payload) +{ + GIT_UNUSED(name); + GIT_UNUSED(value); + + *((int *)payload) -= 1; + + if (*((int *)payload) < 0) + return -1; + + return 0; +} + void test_attr_repo__foreach(void) { int count; @@ -131,6 +147,12 @@ void test_attr_repo__foreach(void) cl_git_pass(git_attr_foreach(g_repo, 0, "sub/subdir_test2.txt", &count_attrs, &count)); cl_assert(count == 6); /* repoattr, rootattr, subattr, reposub, negattr, another */ + + count = 2; + cl_assert_equal_i( + GIT_EUSER, git_attr_foreach( + g_repo, 0, "sub/subdir_test1", &cancel_iteration, &count) + ); } void test_attr_repo__manpage_example(void) diff --git a/tests-clar/config/read.c b/tests-clar/config/read.c index a8504da02..574ff8196 100644 --- a/tests-clar/config/read.c +++ b/tests-clar/config/read.c @@ -226,7 +226,7 @@ void test_config_read__foreach(void) count = 3; cl_git_fail(ret = git_config_foreach(cfg, cfg_callback_countdown, &count)); - cl_assert_equal_i(-100, ret); + cl_assert_equal_i(GIT_EUSER, ret); git_config_free(cfg); } diff --git a/tests-clar/diff/index.c b/tests-clar/diff/index.c index 171815df5..89e65e3b7 100644 --- a/tests-clar/diff/index.c +++ b/tests-clar/diff/index.c @@ -90,3 +90,53 @@ void test_diff_index__0(void) git_tree_free(a); git_tree_free(b); } + +static int diff_stop_after_2_files( + void *cb_data, + git_diff_delta *delta, + float progress) +{ + diff_expects *e = cb_data; + + GIT_UNUSED(progress); + GIT_UNUSED(delta); + + e->files++; + + return (e->files == 2); +} + +void test_diff_index__1(void) +{ + /* grabbed a couple of commit oids from the history of the attr repo */ + const char *a_commit = "26a125ee1bf"; /* the current HEAD */ + const char *b_commit = "0017bd4ab1ec3"; /* the start */ + git_tree *a = resolve_commit_oid_to_tree(g_repo, a_commit); + git_tree *b = resolve_commit_oid_to_tree(g_repo, b_commit); + git_diff_options opts = {0}; + git_diff_list *diff = NULL; + diff_expects exp; + + cl_assert(a); + cl_assert(b); + + opts.context_lines = 1; + opts.interhunk_lines = 1; + + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_index_to_tree(g_repo, &opts, a, &diff)); + + cl_assert_equal_i( + GIT_EUSER, + git_diff_foreach(diff, &exp, diff_stop_after_2_files, NULL, NULL) + ); + + cl_assert(exp.files == 2); + + git_diff_list_free(diff); + diff = NULL; + + git_tree_free(a); + git_tree_free(b); +} diff --git a/tests-clar/notes/notes.c b/tests-clar/notes/notes.c index e1387782e..dfd7f5231 100644 --- a/tests-clar/notes/notes.c +++ b/tests-clar/notes/notes.c @@ -95,11 +95,39 @@ void test_notes_notes__can_retrieve_a_list_of_notes_for_a_given_namespace(void) create_note(¬e_oid3, "refs/notes/i-can-see-dead-notes", "9fd738e8f7967c078dceed8190330fc8648ee56a", "I decorate 9fd7 and 4a20\n"); create_note(¬e_oid4, "refs/notes/i-can-see-dead-notes", "4a202b346bb0fb0db7eff3cffeb3c70babbd2045", "I decorate 9fd7 and 4a20\n"); - cl_git_pass(git_note_foreach(_repo, "refs/notes/i-can-see-dead-notes", note_list_cb, &retrieved_notes)); + cl_git_pass(git_note_foreach +(_repo, "refs/notes/i-can-see-dead-notes", note_list_cb, &retrieved_notes)); cl_assert_equal_i(4, retrieved_notes); } +static int note_cancel_cb(git_note_data *note_data, void *payload) +{ + unsigned int *count = (unsigned int *)payload; + + GIT_UNUSED(note_data); + + (*count)++; + + return (*count > 2); +} + +void test_notes_notes__can_cancel_foreach(void) +{ + git_oid note_oid1, note_oid2, note_oid3, note_oid4; + unsigned int retrieved_notes = 0; + + create_note(¬e_oid1, "refs/notes/i-can-see-dead-notes", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", "I decorate a65f\n"); + create_note(¬e_oid2, "refs/notes/i-can-see-dead-notes", "c47800c7266a2be04c571c04d5a6614691ea99bd", "I decorate c478\n"); + create_note(¬e_oid3, "refs/notes/i-can-see-dead-notes", "9fd738e8f7967c078dceed8190330fc8648ee56a", "I decorate 9fd7 and 4a20\n"); + create_note(¬e_oid4, "refs/notes/i-can-see-dead-notes", "4a202b346bb0fb0db7eff3cffeb3c70babbd2045", "I decorate 9fd7 and 4a20\n"); + + cl_assert_equal_i( + GIT_EUSER, + git_note_foreach(_repo, "refs/notes/i-can-see-dead-notes", + note_cancel_cb, &retrieved_notes)); +} + void test_notes_notes__retrieving_a_list_of_notes_for_an_unknown_namespace_returns_ENOTFOUND(void) { int error; diff --git a/tests-clar/odb/foreach.c b/tests-clar/odb/foreach.c index 525c70c09..e025fa210 100644 --- a/tests-clar/odb/foreach.c +++ b/tests-clar/odb/foreach.c @@ -31,6 +31,24 @@ static int foreach_cb(git_oid *oid, void *data) void test_odb_foreach__foreach(void) { + nobj = 0; cl_git_pass(git_odb_foreach(_odb, foreach_cb, NULL)); cl_assert(nobj == 1683); } + +static int foreach_stop_cb(git_oid *oid, void *data) +{ + GIT_UNUSED(data); + GIT_UNUSED(oid); + + nobj++; + + return (nobj == 1000); +} + +void test_odb_foreach__interrupt_foreach(void) +{ + nobj = 0; + cl_assert_equal_i(GIT_EUSER, git_odb_foreach(_odb, foreach_stop_cb, NULL)); + cl_assert(nobj == 1000); +} diff --git a/tests-clar/refs/branches/foreach.c b/tests-clar/refs/branches/foreach.c index b6e973799..8b39b7dc8 100644 --- a/tests-clar/refs/branches/foreach.c +++ b/tests-clar/refs/branches/foreach.c @@ -126,3 +126,28 @@ void test_refs_branches_foreach__retrieve_remote_symbolic_HEAD_when_present(void assert_branch_has_been_found(exp, "nulltoken/HEAD"); assert_branch_has_been_found(exp, "nulltoken/HEAD"); } + +static int branch_list_interrupt_cb( + const char *branch_name, git_branch_t branch_type, void *payload) +{ + int *count; + + GIT_UNUSED(branch_type); + GIT_UNUSED(branch_name); + + count = (int *)payload; + (*count)++; + + return (*count == 5); +} + +void test_refs_branches_foreach__can_cancel(void) +{ + int count = 0; + + cl_assert_equal_i(GIT_EUSER, + git_branch_foreach(repo, GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, + branch_list_interrupt_cb, &count)); + + cl_assert_equal_i(5, count); +} diff --git a/tests-clar/refs/foreachglob.c b/tests-clar/refs/foreachglob.c index d1412a94b..7d514d461 100644 --- a/tests-clar/refs/foreachglob.c +++ b/tests-clar/refs/foreachglob.c @@ -68,3 +68,25 @@ void test_refs_foreachglob__retrieve_partially_named_references(void) assert_retrieval("*test*", GIT_REF_LISTALL, 4); } + + +static int interrupt_cb(const char *reference_name, void *payload) +{ + int *count = (int *)payload; + + GIT_UNUSED(reference_name); + + (*count)++; + + return (*count == 11); +} + +void test_refs_foreachglob__can_cancel(void) +{ + int count = 0; + + cl_assert_equal_i(GIT_EUSER, git_reference_foreach_glob( + repo, "*", GIT_REF_LISTALL, interrupt_cb, &count) ); + + cl_assert_equal_i(11, count); +} diff --git a/tests-clar/status/worktree.c b/tests-clar/status/worktree.c index d84cb77ed..bfd257a3b 100644 --- a/tests-clar/status/worktree.c +++ b/tests-clar/status/worktree.c @@ -530,7 +530,7 @@ void test_status_worktree__bracket_in_filename(void) cl_git_pass(git_repository_init(&repo, "with_bracket", 0)); cl_git_mkfile("with_bracket/" FILE_WITH_BRACKET, "I have a bracket in my name\n"); - + /* file is new to working directory */ memset(&result, 0, sizeof(result)); @@ -578,7 +578,7 @@ void test_status_worktree__bracket_in_filename(void) cl_git_pass(git_status_file(&status_flags, repo, FILE_WITH_BRACKET)); cl_assert(status_flags == GIT_STATUS_INDEX_NEW); - + /* Create file without bracket */ cl_git_mkfile("with_bracket/" FILE_WITHOUT_BRACKET, "I have no bracket in my name!\n"); @@ -591,7 +591,7 @@ void test_status_worktree__bracket_in_filename(void) error = git_status_file(&status_flags, repo, FILE_WITH_BRACKET); cl_git_fail(error); - cl_assert(error == GIT_EAMBIGUOUS); + cl_assert_equal_i(GIT_EAMBIGUOUS, error); git_index_free(index); git_repository_free(repo); @@ -769,6 +769,31 @@ void test_status_worktree__disable_pathspec_match(void) cl_git_pass( git_status_foreach_ext(repo, &opts, cb_status__expected_path, NULL) ); - + git_repository_free(repo); } + + +static int cb_status__interrupt(const char *p, unsigned int s, void *payload) +{ + volatile int *count = (int *)payload; + + GIT_UNUSED(p); + GIT_UNUSED(s); + + (*count)++; + + return (*count == 8); +} + +void test_status_worktree__interruptable_foreach(void) +{ + int count = 0; + git_repository *repo = cl_git_sandbox_init("status"); + + cl_assert_equal_i( + GIT_EUSER, git_status_foreach(repo, cb_status__interrupt, &count) + ); + + cl_assert_equal_i(8, count); +} From b0d376695e7d3f71fed97d9d08b60661faad7a5a Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 3 Aug 2012 17:24:59 -0700 Subject: [PATCH 3/3] Add new iteration behavior to git_tree_walk Missed this one, ironically enough. --- src/tree.c | 10 ++-- tests-clar/object/tree/walk.c | 103 ++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 tests-clar/object/tree/walk.c diff --git a/src/tree.c b/src/tree.c index 422e62b28..2e6153ba0 100644 --- a/src/tree.c +++ b/src/tree.c @@ -787,9 +787,10 @@ static int tree_walk( for (i = 0; i < tree->entries.length; ++i) { git_tree_entry *entry = tree->entries.contents[i]; - if (preorder && - (error = callback(path->ptr, entry, payload)) != 0) + if (preorder && callback(path->ptr, entry, payload)) { + error = GIT_EUSER; break; + } if (git_tree_entry__is_tree(entry)) { git_tree *subtree; @@ -814,9 +815,10 @@ static int tree_walk( git_tree_free(subtree); } - if (!preorder && - (error = callback(path->ptr, entry, payload)) != 0) + if (!preorder && callback(path->ptr, entry, payload)) { + error = GIT_EUSER; break; + } } return error; diff --git a/tests-clar/object/tree/walk.c b/tests-clar/object/tree/walk.c new file mode 100644 index 000000000..a0ea64cf3 --- /dev/null +++ b/tests-clar/object/tree/walk.c @@ -0,0 +1,103 @@ +#include "clar_libgit2.h" +#include "tree.h" + +static const char *tree_oid = "1810dff58d8a660512d4832e740f692884338ccd"; +static git_repository *g_repo; + +void test_object_tree_walk__initialize(void) +{ + g_repo = cl_git_sandbox_init("testrepo"); +} + +void test_object_tree_walk__cleanup(void) +{ + cl_git_sandbox_cleanup(); +} + +static int treewalk_count_cb( + const char *root, const git_tree_entry *entry, void *payload) +{ + int *count = payload; + + GIT_UNUSED(root); + GIT_UNUSED(entry); + + (*count) += 1; + + return 0; +} + +void test_object_tree_walk__0(void) +{ + git_oid id; + git_tree *tree; + int ct; + + git_oid_fromstr(&id, tree_oid); + + cl_git_pass(git_tree_lookup(&tree, g_repo, &id)); + + ct = 0; + cl_git_pass(git_tree_walk(tree, treewalk_count_cb, GIT_TREEWALK_PRE, &ct)); + cl_assert_equal_i(3, ct); + + ct = 0; + cl_git_pass(git_tree_walk(tree, treewalk_count_cb, GIT_TREEWALK_POST, &ct)); + cl_assert_equal_i(3, ct); + + git_tree_free(tree); +} + + +static int treewalk_stop_cb( + const char *root, const git_tree_entry *entry, void *payload) +{ + int *count = payload; + + GIT_UNUSED(root); + GIT_UNUSED(entry); + + (*count) += 1; + + return (*count == 2); +} + +static int treewalk_stop_immediately_cb( + const char *root, const git_tree_entry *entry, void *payload) +{ + GIT_UNUSED(root); + GIT_UNUSED(entry); + GIT_UNUSED(payload); + return -100; +} + +void test_object_tree_walk__1(void) +{ + git_oid id; + git_tree *tree; + int ct; + + git_oid_fromstr(&id, tree_oid); + + cl_git_pass(git_tree_lookup(&tree, g_repo, &id)); + + ct = 0; + cl_assert_equal_i( + GIT_EUSER, git_tree_walk(tree, treewalk_stop_cb, GIT_TREEWALK_PRE, &ct)); + cl_assert_equal_i(2, ct); + + ct = 0; + cl_assert_equal_i( + GIT_EUSER, git_tree_walk(tree, treewalk_stop_cb, GIT_TREEWALK_POST, &ct)); + cl_assert_equal_i(2, ct); + + cl_assert_equal_i( + GIT_EUSER, git_tree_walk( + tree, treewalk_stop_immediately_cb, GIT_TREEWALK_PRE, NULL)); + + cl_assert_equal_i( + GIT_EUSER, git_tree_walk( + tree, treewalk_stop_immediately_cb, GIT_TREEWALK_POST, NULL)); + + git_tree_free(tree); +}