From ae9e29fde7e7d1c0c3e95bdabbb5c96fc71b1c71 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 6 Mar 2012 16:14:31 -0800 Subject: [PATCH] Migrating diff to new error handling Ended up migrating a bunch of upstream functions as well including vector, attr_file, and odb in order to get this to work right. --- src/attr_file.c | 99 +++++++++-------------- src/diff.c | 186 ++++++++++++++++++++----------------------- src/diff_output.c | 179 ++++++++++++++++++++--------------------- src/errors.c | 9 +++ src/filebuf.c | 4 +- src/fileops.c | 17 ++-- src/fileops.h | 14 ++++ src/index.c | 2 +- src/odb.c | 197 ++++++++++++++++++++-------------------------- src/path.c | 2 +- src/refs.c | 3 +- src/util.c | 15 ++-- src/util.h | 5 ++ src/vector.c | 63 +++++++-------- 14 files changed, 371 insertions(+), 424 deletions(-) diff --git a/src/attr_file.c b/src/attr_file.c index 48424123a..029934317 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -11,24 +11,19 @@ static void git_attr_rule__clear(git_attr_rule *rule); int git_attr_file__new(git_attr_file **attrs_ptr) { - int error; git_attr_file *attrs = NULL; attrs = git__calloc(1, sizeof(git_attr_file)); - if (attrs == NULL) - error = GIT_ENOMEM; - else - error = git_vector_init(&attrs->rules, 4, NULL); + GITERR_CHECK_ALLOC(attrs); - if (error != GIT_SUCCESS) { - git__rethrow(error, "Could not allocate attribute storage"); + if (git_vector_init(&attrs->rules, 4, NULL) < 0) { git__free(attrs); attrs = NULL; } *attrs_ptr = attrs; - return error; + return attrs ? 0 : -1; } int git_attr_file__set_path( @@ -50,13 +45,15 @@ int git_attr_file__set_path( file->path = git__strdup(path); } - return (file->path == NULL) ? GIT_ENOMEM : GIT_SUCCESS; + GITERR_CHECK_ALLOC(file->path); + + return 0; } int git_attr_file__from_buffer( git_repository *repo, const char *buffer, git_attr_file *attrs) { - int error = GIT_SUCCESS; + int error = 0; const char *scan = NULL; char *context = NULL; git_attr_rule *rule = NULL; @@ -68,13 +65,13 @@ int git_attr_file__from_buffer( if (attrs->path && git__suffixcmp(attrs->path, GIT_ATTR_FILE) == 0) { context = git__strndup(attrs->path, strlen(attrs->path) - strlen(GIT_ATTR_FILE)); - if (!context) error = GIT_ENOMEM; + GITERR_CHECK_ALLOC(context); } - while (error == GIT_SUCCESS && *scan) { + while (!error && *scan) { /* allocate rule if needed */ if (!rule && !(rule = git__calloc(1, sizeof(git_attr_rule)))) { - error = GIT_ENOMEM; + error = -1; break; } @@ -92,10 +89,10 @@ int git_attr_file__from_buffer( } /* if the rule wasn't a pattern, on to the next */ - if (error != GIT_SUCCESS) { + if (error < 0) { git_attr_rule__clear(rule); /* reset rule contents */ if (error == GIT_ENOTFOUND) - error = GIT_SUCCESS; + error = 0; } else { rule = NULL; /* vector now "owns" the rule */ } @@ -110,21 +107,20 @@ int git_attr_file__from_buffer( int git_attr_file__from_file( git_repository *repo, const char *path, git_attr_file *file) { - int error = GIT_SUCCESS; + int error; git_buf fbuf = GIT_BUF_INIT; assert(path && file); - if (file->path == NULL) - error = git_attr_file__set_path(repo, path, file); + if (file->path == NULL && git_attr_file__set_path(repo, path, file) < 0) + return -1; - if (error == GIT_SUCCESS && - (error = git_futils_readbuffer(&fbuf, path)) == GIT_SUCCESS) - error = git_attr_file__from_buffer(repo, fbuf.ptr, file); + if (git_futils_readbuffer(&fbuf, path) < 0) + return -1; + + error = git_attr_file__from_buffer(repo, fbuf.ptr, file); git_buf_free(&fbuf); - if (error != GIT_SUCCESS) - git__rethrow(error, "Could not open attribute file '%s'", path); return error; } @@ -176,7 +172,6 @@ int git_attr_file__lookup_one( git_attr_file__foreach_matching_rule(file, path, i, rule) { int pos = git_vector_bsearch(&rule->assigns, &name); - git_clearerror(); /* okay if search failed */ if (pos >= 0) { *value = ((git_attr_assignment *) @@ -230,7 +225,6 @@ git_attr_assignment *git_attr_rule__lookup_assignment( key.name_hash = git_attr_file__name_hash(name); pos = git_vector_bsearch(&rule->assigns, &key); - git_clearerror(); /* okay if search failed */ return (pos >= 0) ? git_vector_get(&rule->assigns, pos) : NULL; } @@ -248,16 +242,15 @@ int git_attr_path__init( if (base != NULL && git_path_root(path) < 0) { git_buf full_path = GIT_BUF_INIT; - int error = git_buf_joinpath(&full_path, base, path); - if (error == GIT_SUCCESS) - info->is_dir = (int)git_path_isdir(full_path.ptr); - + if (git_buf_joinpath(&full_path, base, path) < 0) + return -1; + info->is_dir = (int)git_path_isdir(full_path.ptr); git_buf_free(&full_path); - return error; + return 0; } info->is_dir = (int)git_path_isdir(path); - return GIT_SUCCESS; + return 0; } @@ -374,7 +367,7 @@ int git_attr_fnmatch__parse( if (!spec->pattern) { *base = git__next_line(pattern); - return GIT_ENOMEM; + return -1; } else { /* strip '\' that might have be used for internal whitespace */ char *to = spec->pattern; @@ -390,7 +383,7 @@ int git_attr_fnmatch__parse( } } - return GIT_SUCCESS; + return 0; } static int sort_by_hash_and_name(const void *a_raw, const void *b_raw) @@ -434,7 +427,7 @@ int git_attr_assignment__parse( git_vector *assigns, const char **base) { - int error = GIT_SUCCESS; + int error; const char *scan = *base; git_attr_assignment *assign = NULL; @@ -442,7 +435,7 @@ int git_attr_assignment__parse( assigns->_cmp = sort_by_hash_and_name; - while (*scan && *scan != '\n' && error == GIT_SUCCESS) { + while (*scan && *scan != '\n') { const char *name_start, *value_start; /* skip leading blanks */ @@ -451,10 +444,7 @@ int git_attr_assignment__parse( /* allocate assign if needed */ if (!assign) { assign = git__calloc(1, sizeof(git_attr_assignment)); - if (!assign) { - error = GIT_ENOMEM; - break; - } + GITERR_CHECK_ALLOC(assign); GIT_REFCOUNT_INC(assign); } @@ -489,10 +479,7 @@ int git_attr_assignment__parse( /* allocate permanent storage for name */ assign->name = git__strndup(name_start, scan - name_start); - if (!assign->name) { - error = GIT_ENOMEM; - break; - } + GITERR_CHECK_ALLOC(assign->name); /* if there is an equals sign, find the value */ if (*scan == '=') { @@ -501,12 +488,8 @@ int git_attr_assignment__parse( /* if we found a value, allocate permanent storage for it */ if (scan > value_start) { assign->value = git__strndup(value_start, scan - value_start); - if (!assign->value) { - error = GIT_ENOMEM; - break; - } else { - assign->is_allocated = 1; - } + GITERR_CHECK_ALLOC(assign->value); + assign->is_allocated = 1; } } @@ -524,35 +507,27 @@ int git_attr_assignment__parse( error = git_vector_insert_sorted( assigns, massign, &merge_assignments); - - if (error == GIT_EEXISTS) - error = GIT_SUCCESS; - else if (error != GIT_SUCCESS) - break; + if (error < 0 && error != GIT_EEXISTS) + return error; } } } /* insert allocated assign into vector */ error = git_vector_insert_sorted(assigns, assign, &merge_assignments); - if (error == GIT_EEXISTS) - error = GIT_SUCCESS; - else if (error < GIT_SUCCESS) - break; + if (error < 0 && error != GIT_EEXISTS) + return error; /* clear assign since it is now "owned" by the vector */ assign = NULL; } - if (!assigns->length) - error = git__throw(GIT_ENOTFOUND, "No attribute assignments found for rule"); - if (assign != NULL) git_attr_assignment__free(assign); *base = git__next_line(scan); - return error; + return (assigns->length == 0) ? GIT_ENOTFOUND : 0; } static void git_attr_rule__clear(git_attr_rule *rule) diff --git a/src/diff.c b/src/diff.c index a0fd5fdf1..06c61122a 100644 --- a/src/diff.c +++ b/src/diff.c @@ -132,10 +132,8 @@ static int diff_delta__from_one( git_delta_t status, const git_index_entry *entry) { - int error; git_diff_delta *delta = diff_delta__alloc(diff, status, entry->path); - if (!delta) - return git__rethrow(GIT_ENOMEM, "Could not allocate diff record"); + GITERR_CHECK_ALLOC(delta); /* This fn is just for single-sided diffs */ assert(status != GIT_DELTA_MODIFIED); @@ -153,10 +151,12 @@ static int diff_delta__from_one( delta->old.flags |= GIT_DIFF_FILE_VALID_OID; delta->new.flags |= GIT_DIFF_FILE_VALID_OID; - if ((error = git_vector_insert(&diff->deltas, delta)) < GIT_SUCCESS) + if (git_vector_insert(&diff->deltas, delta) < 0) { diff_delta__free(delta); + return -1; + } - return error; + return 0; } static int diff_delta__from_two( @@ -166,7 +166,6 @@ static int diff_delta__from_two( const git_index_entry *new, git_oid *new_oid) { - int error; git_diff_delta *delta; if ((diff->opts.flags & GIT_DIFF_REVERSE) != 0) { @@ -176,8 +175,7 @@ static int diff_delta__from_two( } delta = diff_delta__alloc(diff, status, old->path); - if (!delta) - return git__rethrow(GIT_ENOMEM, "Could not allocate diff record"); + GITERR_CHECK_ALLOC(delta); delta->old.mode = old->mode; git_oid_cpy(&delta->old.oid, &old->oid); @@ -188,10 +186,12 @@ static int diff_delta__from_two( if (new_oid || !git_oid_iszero(&new->oid)) delta->new.flags |= GIT_DIFF_FILE_VALID_OID; - if ((error = git_vector_insert(&diff->deltas, delta)) < GIT_SUCCESS) + if (git_vector_insert(&diff->deltas, delta) < 0) { diff_delta__free(delta); + return -1; + } - return error; + return 0; } #define DIFF_SRC_PREFIX_DEFAULT "a/" @@ -284,27 +284,24 @@ static int oid_for_workdir_item( const git_index_entry *item, git_oid *oid) { - int error = GIT_SUCCESS; + int result; git_buf full_path = GIT_BUF_INIT; - error = git_buf_joinpath( - &full_path, git_repository_workdir(repo), item->path); - if (error != GIT_SUCCESS) - return error; + if (git_buf_joinpath(&full_path, git_repository_workdir(repo), item->path) < 0) + return -1; - /* otherwise calculate OID for file */ + /* calculate OID for file if possible*/ if (S_ISLNK(item->mode)) - error = git_odb__hashlink(oid, full_path.ptr); - else if (!git__is_sizet(item->file_size)) - error = git__throw(GIT_ERROR, "File size overflow for 32-bit systems"); - else { - int fd; - - if ((fd = p_open(full_path.ptr, O_RDONLY)) < 0) - error = git__throw( - GIT_EOSERR, "Could not open '%s'", item->path); + result = git_odb__hashlink(oid, full_path.ptr); + else if (!git__is_sizet(item->file_size)) { + giterr_set(GITERR_OS, "File size overflow for 32-bit systems"); + result = -1; + } else { + int fd = git_futils_open_ro(full_path.ptr); + if (fd < 0) + result = fd; else { - error = git_odb__hashfd( + result = git_odb__hashfd( oid, fd, (size_t)item->file_size, GIT_OBJ_BLOB); p_close(fd); } @@ -312,7 +309,7 @@ static int oid_for_workdir_item( git_buf_free(&full_path); - return error; + return result; } static int maybe_modified( @@ -322,7 +319,6 @@ static int maybe_modified( const git_index_entry *nitem, git_diff_list *diff) { - int error = GIT_SUCCESS; git_oid noid, *use_noid = NULL; GIT_UNUSED(old); @@ -330,18 +326,18 @@ static int maybe_modified( /* support "assume unchanged" & "skip worktree" bits */ if ((oitem->flags_extended & GIT_IDXENTRY_INTENT_TO_ADD) != 0 || (oitem->flags_extended & GIT_IDXENTRY_SKIP_WORKTREE) != 0) - return GIT_SUCCESS; + return 0; if (GIT_MODE_TYPE(oitem->mode) != GIT_MODE_TYPE(nitem->mode)) { - error = diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem); - if (error == GIT_SUCCESS) - error = diff_delta__from_one(diff, GIT_DELTA_ADDED, nitem); - return error; + if (diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem) < 0 || + diff_delta__from_one(diff, GIT_DELTA_ADDED, nitem) < 0) + return -1; + return 0; } if (git_oid_cmp(&oitem->oid, &nitem->oid) == 0 && oitem->mode == nitem->mode) - return GIT_SUCCESS; + return 0; if (git_oid_iszero(&nitem->oid) && new->type == GIT_ITERATOR_WORKDIR) { /* if they files look exactly alike, then we'll assume the same */ @@ -352,18 +348,18 @@ static int maybe_modified( oitem->ino == nitem->ino && oitem->uid == nitem->uid && oitem->gid == nitem->gid) - return GIT_SUCCESS; + return 0; /* TODO: check git attributes so we will not have to read the file * in if it is marked binary. */ - error = oid_for_workdir_item(diff->repo, nitem, &noid); - if (error != GIT_SUCCESS) - return error; + + if (oid_for_workdir_item(diff->repo, nitem, &noid) < 0) + return -1; if (git_oid_cmp(&oitem->oid, &noid) == 0 && oitem->mode == nitem->mode) - return GIT_SUCCESS; + return 0; /* store calculated oid so we don't have to recalc later */ use_noid = &noid; @@ -380,37 +376,33 @@ static int diff_from_iterators( git_iterator *new, git_diff_list **diff_ptr) { - int error; const git_index_entry *oitem, *nitem; char *ignore_prefix = NULL; git_diff_list *diff = git_diff_list_alloc(repo, opts); - if (!diff) { - error = GIT_ENOMEM; - goto cleanup; - } + if (!diff) + goto fail; diff->old_src = old->type; diff->new_src = new->type; - if ((error = git_iterator_current(old, &oitem)) < GIT_SUCCESS || - (error = git_iterator_current(new, &nitem)) < GIT_SUCCESS) - goto cleanup; + if (git_iterator_current(old, &oitem) < 0 || + git_iterator_current(new, &nitem) < 0) + goto fail; /* run iterators building diffs */ - while (!error && (oitem || nitem)) { + while (oitem || nitem) { /* create DELETED records for old items not matched in new */ if (oitem && (!nitem || strcmp(oitem->path, nitem->path) < 0)) { - error = diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem); - if (error == GIT_SUCCESS) - error = git_iterator_advance(old, &oitem); - continue; + if (diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem) < 0 || + git_iterator_advance(old, &oitem) < 0) + goto fail; } /* create ADDED, TRACKED, or IGNORED records for new items not * matched in old (and/or descend into directories as needed) */ - if (nitem && (!oitem || strcmp(oitem->path, nitem->path) > 0)) { + else if (nitem && (!oitem || strcmp(oitem->path, nitem->path) > 0)) { int is_ignored; git_delta_t delta_type = GIT_DELTA_ADDED; @@ -418,7 +410,8 @@ static int diff_from_iterators( if (ignore_prefix != NULL && git__prefixcmp(nitem->path, ignore_prefix) == 0) { - error = git_iterator_advance(new, &nitem); + if (git_iterator_advance(new, &nitem) < 0) + goto fail; continue; } @@ -428,7 +421,8 @@ static int diff_from_iterators( if (git__prefixcmp(oitem->path, nitem->path) == 0) { if (is_ignored) ignore_prefix = nitem->path; - error = git_iterator_advance_into_directory(new, &nitem); + if (git_iterator_advance_into_directory(new, &nitem) < 0) + goto fail; continue; } delta_type = GIT_DELTA_UNTRACKED; @@ -438,36 +432,35 @@ static int diff_from_iterators( else if (new->type == GIT_ITERATOR_WORKDIR) delta_type = GIT_DELTA_UNTRACKED; - error = diff_delta__from_one(diff, delta_type, nitem); - if (error == GIT_SUCCESS) - error = git_iterator_advance(new, &nitem); - continue; + if (diff_delta__from_one(diff, delta_type, nitem) < 0 || + git_iterator_advance(new, &nitem) < 0) + goto fail; } /* otherwise item paths match, so create MODIFIED record * (or ADDED and DELETED pair if type changed) */ - assert(oitem && nitem && strcmp(oitem->path, nitem->path) == 0); + else { + assert(oitem && nitem && strcmp(oitem->path, nitem->path) == 0); - error = maybe_modified(old, oitem, new, nitem, diff); - if (error == GIT_SUCCESS) - error = git_iterator_advance(old, &oitem); - if (error == GIT_SUCCESS) - error = git_iterator_advance(new, &nitem); + if (maybe_modified(old, oitem, new, nitem, diff) < 0 || + git_iterator_advance(old, &oitem) < 0 || + git_iterator_advance(new, &nitem) < 0) + goto fail; + } } -cleanup: git_iterator_free(old); git_iterator_free(new); - - if (error != GIT_SUCCESS) { - git_diff_list_free(diff); - diff = NULL; - } - *diff_ptr = diff; + return 0; - return error; +fail: + git_iterator_free(old); + git_iterator_free(new); + git_diff_list_free(diff); + *diff_ptr = NULL; + return -1; } @@ -478,14 +471,13 @@ int git_diff_tree_to_tree( git_tree *new, git_diff_list **diff) { - int error; git_iterator *a = NULL, *b = NULL; assert(repo && old && new && diff); - if ((error = git_iterator_for_tree(repo, old, &a)) < GIT_SUCCESS || - (error = git_iterator_for_tree(repo, new, &b)) < GIT_SUCCESS) - return error; + if (git_iterator_for_tree(repo, old, &a) < 0 || + git_iterator_for_tree(repo, new, &b) < 0) + return -1; return diff_from_iterators(repo, opts, a, b, diff); } @@ -496,14 +488,13 @@ int git_diff_index_to_tree( git_tree *old, git_diff_list **diff) { - int error; git_iterator *a = NULL, *b = NULL; assert(repo && old && diff); - if ((error = git_iterator_for_tree(repo, old, &a)) < GIT_SUCCESS || - (error = git_iterator_for_index(repo, &b)) < GIT_SUCCESS) - return error; + if (git_iterator_for_tree(repo, old, &a) < 0 || + git_iterator_for_index(repo, &b) < 0) + return -1; return diff_from_iterators(repo, opts, a, b, diff); } @@ -513,14 +504,13 @@ int git_diff_workdir_to_index( const git_diff_options *opts, git_diff_list **diff) { - int error; git_iterator *a = NULL, *b = NULL; assert(repo && diff); - if ((error = git_iterator_for_index(repo, &a)) < GIT_SUCCESS || - (error = git_iterator_for_workdir(repo, &b)) < GIT_SUCCESS) - return error; + if (git_iterator_for_index(repo, &a) < 0 || + git_iterator_for_workdir(repo, &b) < 0) + return -1; return diff_from_iterators(repo, opts, a, b, diff); } @@ -532,14 +522,13 @@ int git_diff_workdir_to_tree( git_tree *old, git_diff_list **diff) { - int error; git_iterator *a = NULL, *b = NULL; assert(repo && old && diff); - if ((error = git_iterator_for_tree(repo, old, &a)) < GIT_SUCCESS || - (error = git_iterator_for_workdir(repo, &b)) < GIT_SUCCESS) - return error; + if (git_iterator_for_tree(repo, old, &a) < 0 || + git_iterator_for_workdir(repo, &b) < 0) + return -1; return diff_from_iterators(repo, opts, a, b, diff); } @@ -548,16 +537,15 @@ int git_diff_merge( git_diff_list *onto, const git_diff_list *from) { - int error; + int error = 0; unsigned int i = 0, j = 0; git_vector onto_new; git_diff_delta *delta; - error = git_vector_init(&onto_new, onto->deltas.length, diff_delta__cmp); - if (error < GIT_SUCCESS) - return error; + if (git_vector_init(&onto_new, onto->deltas.length, diff_delta__cmp) < 0) + return -1; - while (i < onto->deltas.length || j < from->deltas.length) { + while (!error && (i < onto->deltas.length || j < from->deltas.length)) { git_diff_delta *o = git_vector_get(&onto->deltas, i); const git_diff_delta *f = git_vector_get_const(&from->deltas, j); const char *opath = !o ? NULL : o->old.path ? o->old.path : o->new.path; @@ -575,16 +563,10 @@ int git_diff_merge( j++; } - if (!delta) - error = GIT_ENOMEM; - else - error = git_vector_insert(&onto_new, delta); - - if (error != GIT_SUCCESS) - break; + error = !delta ? -1 : git_vector_insert(&onto_new, delta); } - if (error == GIT_SUCCESS) { + if (error == 0) { git_vector_swap(&onto->deltas, &onto_new); onto->new_src = from->new_src; } diff --git a/src/diff_output.c b/src/diff_output.c index 9c34dcf71..638cabca5 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -34,31 +34,39 @@ static int read_next_int(const char **str, int *value) v = (v * 10) + (*scan - '0'); *str = scan; *value = v; - return (digits > 0) ? GIT_SUCCESS : GIT_ENOTFOUND; + return (digits > 0) ? 0 : -1; } static int diff_output_cb(void *priv, mmbuffer_t *bufs, int len) { - int err = GIT_SUCCESS; diff_output_info *info = priv; if (len == 1 && info->hunk_cb) { git_diff_range range = { -1, 0, -1, 0 }; + const char *scan = bufs[0].ptr; /* expect something of the form "@@ -%d[,%d] +%d[,%d] @@" */ - if (bufs[0].ptr[0] == '@') { - const char *scan = bufs[0].ptr; - if (!(err = read_next_int(&scan, &range.old_start)) && *scan == ',') - err = read_next_int(&scan, &range.old_lines); - if (!err && - !(err = read_next_int(&scan, &range.new_start)) && *scan == ',') - err = read_next_int(&scan, &range.new_lines); - if (!err && range.old_start >= 0 && range.new_start >= 0) - err = info->hunk_cb( - info->cb_data, info->delta, &range, bufs[0].ptr, bufs[0].size); - } + if (*scan != '@') + return -1; + + 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; + + return info->hunk_cb( + info->cb_data, info->delta, &range, bufs[0].ptr, bufs[0].size); } - else if ((len == 2 || len == 3) && info->line_cb) { + + if ((len == 2 || len == 3) && info->line_cb) { int origin; /* expect " "/"-"/"+", then data, then maybe newline */ @@ -67,41 +75,43 @@ static int diff_output_cb(void *priv, mmbuffer_t *bufs, int len) (*bufs[0].ptr == '-') ? GIT_DIFF_LINE_DELETION : GIT_DIFF_LINE_CONTEXT; - err = info->line_cb( - info->cb_data, info->delta, origin, bufs[1].ptr, bufs[1].size); + if (info->line_cb( + info->cb_data, info->delta, origin, bufs[1].ptr, bufs[1].size) < 0) + return -1; /* deal with adding and removing newline at EOF */ - if (err == GIT_SUCCESS && len == 3) { + if (len == 3) { if (origin == GIT_DIFF_LINE_ADDITION) origin = GIT_DIFF_LINE_ADD_EOFNL; else origin = GIT_DIFF_LINE_DEL_EOFNL; - err = info->line_cb( + return info->line_cb( info->cb_data, info->delta, origin, bufs[2].ptr, bufs[2].size); } } - return err; + return 0; } #define BINARY_DIFF_FLAGS (GIT_DIFF_FILE_BINARY|GIT_DIFF_FILE_NOT_BINARY) -static int set_file_is_binary_by_attr(git_repository *repo, git_diff_file *file) +static int update_file_is_binary_by_attr(git_repository *repo, git_diff_file *file) { const char *value; - int error = git_attr_get(repo, file->path, "diff", &value); - if (error != GIT_SUCCESS) - return error; + if (git_attr_get(repo, file->path, "diff", &value) < 0) + return -1; + if (GIT_ATTR_FALSE(value)) file->flags |= GIT_DIFF_FILE_BINARY; else if (GIT_ATTR_TRUE(value)) file->flags |= GIT_DIFF_FILE_NOT_BINARY; /* otherwise leave file->flags alone */ - return error; + + return 0; } -static void set_delta_is_binary(git_diff_delta *delta) +static void update_delta_is_binary(git_diff_delta *delta) { if ((delta->old.flags & GIT_DIFF_FILE_BINARY) != 0 || (delta->new.flags & GIT_DIFF_FILE_BINARY) != 0) @@ -116,7 +126,7 @@ static int file_is_binary_by_attr( git_diff_list *diff, git_diff_delta *delta) { - int error, mirror_new; + int error = 0, mirror_new; delta->binary = -1; @@ -127,7 +137,7 @@ static int file_is_binary_by_attr( delta->old.flags |= GIT_DIFF_FILE_BINARY; delta->new.flags |= GIT_DIFF_FILE_BINARY; delta->binary = 1; - return GIT_SUCCESS; + return 0; } /* check if user is forcing us to text diff these files */ @@ -135,22 +145,21 @@ static int file_is_binary_by_attr( delta->old.flags |= GIT_DIFF_FILE_NOT_BINARY; delta->new.flags |= GIT_DIFF_FILE_NOT_BINARY; delta->binary = 0; - return GIT_SUCCESS; + return 0; } /* check diff attribute +, -, or 0 */ - error = set_file_is_binary_by_attr(diff->repo, &delta->old); - if (error != GIT_SUCCESS) - return error; + if (update_file_is_binary_by_attr(diff->repo, &delta->old) < 0) + return -1; mirror_new = (delta->new.path == delta->old.path || strcmp(delta->new.path, delta->old.path) == 0); if (mirror_new) delta->new.flags &= (delta->old.flags & BINARY_DIFF_FLAGS); else - error = set_file_is_binary_by_attr(diff->repo, &delta->new); + error = update_file_is_binary_by_attr(diff->repo, &delta->new); - set_delta_is_binary(delta); + update_delta_is_binary(delta); return error; } @@ -179,11 +188,11 @@ static int file_is_binary_by_content( delta->new.flags |= GIT_DIFF_FILE_NOT_BINARY; } - set_delta_is_binary(delta); + update_delta_is_binary(delta); /* TODO: if value != NULL, implement diff drivers */ - return GIT_SUCCESS; + return 0; } static void setup_xdiff_options( @@ -214,17 +223,15 @@ static int get_blob_content( git_map *map, git_blob **blob) { - int error; - if (git_oid_iszero(oid)) - return GIT_SUCCESS; + return 0; - if ((error = git_blob_lookup(blob, repo, oid)) == GIT_SUCCESS) { - map->data = (void *)git_blob_rawcontent(*blob); - map->len = git_blob_rawsize(*blob); - } + if (git_blob_lookup(blob, repo, oid) < 0) + return -1; - return error; + map->data = (void *)git_blob_rawcontent(*blob); + map->len = git_blob_rawsize(*blob); + return 0; } static int get_workdir_content( @@ -232,35 +239,33 @@ static int get_workdir_content( git_diff_file *file, git_map *map) { - git_buf full_path = GIT_BUF_INIT; - int error = git_buf_joinpath( - &full_path, git_repository_workdir(repo), file->path); - if (error != GIT_SUCCESS) - return error; + int error = 0; + git_buf path = GIT_BUF_INIT; + + if (git_buf_joinpath(&path, git_repository_workdir(repo), file->path) < 0) + return -1; if (S_ISLNK(file->mode)) { + ssize_t read_len; + file->flags |= GIT_DIFF_FILE_FREE_DATA; file->flags |= GIT_DIFF_FILE_BINARY; map->data = git__malloc((size_t)file->size + 1); - if (map->data == NULL) - error = GIT_ENOMEM; - else { - ssize_t read_len = - p_readlink(full_path.ptr, map->data, (size_t)file->size + 1); - if (read_len != (ssize_t)file->size) - error = git__throw( - GIT_EOSERR, "Failed to read symlink %s", file->path); - else - map->len = read_len; + GITERR_CHECK_ALLOC(map->data); - } + read_len = p_readlink(path.ptr, map->data, (size_t)file->size + 1); + if (read_len != (ssize_t)file->size) { + giterr_set(GITERR_OS, "Failed to read symlink '%s'", file->path); + error = -1; + } else + map->len = read_len; } else { - error = git_futils_mmap_ro_file(map, full_path.ptr); + error = git_futils_mmap_ro_file(map, path.ptr); file->flags |= GIT_DIFF_FILE_UNMAP_DATA; } - git_buf_free(&full_path); + git_buf_free(&path); return error; } @@ -288,7 +293,7 @@ int git_diff_foreach( git_diff_hunk_fn hunk_cb, git_diff_line_fn line_cb) { - int error = GIT_SUCCESS; + int error = 0; diff_output_info info; git_diff_delta *delta; xpparam_t xdiff_params; @@ -320,8 +325,7 @@ int git_diff_foreach( (diff->opts.flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0) continue; - error = file_is_binary_by_attr(diff, delta); - if (error < GIT_SUCCESS) + if ((error = file_is_binary_by_attr(diff, delta)) < 0) goto cleanup; old_data.data = ""; @@ -345,7 +349,7 @@ int git_diff_foreach( else error = get_blob_content( diff->repo, &delta->old.oid, &old_data, &old_blob); - if (error != GIT_SUCCESS) + if (error < 0) goto cleanup; } @@ -359,13 +363,13 @@ int git_diff_foreach( else error = get_blob_content( diff->repo, &delta->new.oid, &new_data, &new_blob); - if (error != GIT_SUCCESS) + if (error < 0) goto cleanup; if ((delta->new.flags | GIT_DIFF_FILE_VALID_OID) == 0) { error = git_odb_hash( &delta->new.oid, new_data.data, new_data.len, GIT_OBJ_BLOB); - if (error != GIT_SUCCESS) + if (error < 0) goto cleanup; /* since we did not have the definitive oid, we may have @@ -384,7 +388,7 @@ int git_diff_foreach( if (delta->binary == -1) { error = file_is_binary_by_content( diff, delta, &old_data, &new_data); - if (error < GIT_SUCCESS) + if (error < 0) goto cleanup; } @@ -394,7 +398,7 @@ int git_diff_foreach( if (file_cb != NULL) { error = file_cb(data, delta, (float)info.index / diff->deltas.length); - if (error != GIT_SUCCESS) + if (error < 0) goto cleanup; } @@ -417,7 +421,7 @@ cleanup: release_content(&delta->old, &old_data, old_blob); release_content(&delta->new, &new_data, new_blob); - if (error != GIT_SUCCESS) + if (error < 0) break; } @@ -462,7 +466,7 @@ static int print_compact(void *data, git_diff_delta *delta, float progress) } if (!code) - return GIT_SUCCESS; + return 0; old_suffix = pick_suffix(delta->old.mode); new_suffix = pick_suffix(delta->new.mode); @@ -482,8 +486,8 @@ static int print_compact(void *data, git_diff_delta *delta, float progress) else git_buf_printf(pi->buf, "%c\t%s\n", code, delta->old.path); - if (git_buf_oom(pi->buf) == true) - return GIT_ENOMEM; + if (git_buf_oom(pi->buf)) + return -1; return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr); } @@ -535,14 +539,13 @@ static int print_oid_range(diff_print_info *pi, git_diff_delta *delta) } if (git_buf_oom(pi->buf)) - return GIT_ENOMEM; + return -1; return 0; } static int print_patch_file(void *data, git_diff_delta *delta, float progress) { - int error; diff_print_info *pi = data; const char *oldpfx = pi->diff->opts.src_prefix; const char *oldpath = delta->old.path; @@ -553,8 +556,9 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress) git_buf_clear(pi->buf); git_buf_printf(pi->buf, "diff --git %s%s %s%s\n", oldpfx, delta->old.path, newpfx, delta->new.path); - if ((error = print_oid_range(pi, delta)) < GIT_SUCCESS) - return error; + + if (print_oid_range(pi, delta) < 0) + return -1; if (git_oid_iszero(&delta->old.oid)) { oldpfx = ""; @@ -571,18 +575,18 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress) } if (git_buf_oom(pi->buf)) - return GIT_ENOMEM; + return -1; - error = pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr); - if (error != GIT_SUCCESS || delta->binary != 1) - return error; + if (pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr) < 0 || + delta->binary != 1) + return -1; git_buf_clear(pi->buf); git_buf_printf( pi->buf, "Binary files %s%s and %s%s differ\n", oldpfx, oldpath, newpfx, newpath); if (git_buf_oom(pi->buf)) - return GIT_ENOMEM; + return -1; return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_BINARY, pi->buf->ptr); } @@ -600,11 +604,10 @@ static int print_patch_hunk( GIT_UNUSED(r); git_buf_clear(pi->buf); + if (git_buf_printf(pi->buf, "%.*s", (int)header_len, header) < 0) + return -1; - if (git_buf_printf(pi->buf, "%.*s", (int)header_len, header) == GIT_SUCCESS) - return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_HUNK_HDR, pi->buf->ptr); - else - return GIT_ENOMEM; + return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_HUNK_HDR, pi->buf->ptr); } static int print_patch_line( @@ -628,7 +631,7 @@ static int print_patch_line( git_buf_printf(pi->buf, "%.*s", (int)content_len, content); if (git_buf_oom(pi->buf)) - return GIT_ENOMEM; + return -1; return pi->print_cb(pi->cb_data, line_origin, pi->buf->ptr); } @@ -721,5 +724,5 @@ int git_diff_blobs( xdl_diff(&old, &new, &xdiff_params, &xdiff_config, &xdiff_callback); - return GIT_SUCCESS; + return 0; } diff --git a/src/errors.c b/src/errors.c index 548e44a32..0454856cf 100644 --- a/src/errors.c +++ b/src/errors.c @@ -123,6 +123,8 @@ void giterr_set(int error_class, const char *string, ...) char error_str[1024]; va_list arglist; git_error *error; + const char *oserr = + (error_class == GITERR_OS && errno > 0) ? strerror(errno) : NULL; error = &GIT_GLOBAL->error_t; free(error->message); @@ -131,6 +133,13 @@ void giterr_set(int error_class, const char *string, ...) p_vsnprintf(error_str, sizeof(error_str), string, arglist); va_end(arglist); + /* automatically suffix strerror(errno) for GITERR_OS errors */ + if (oserr != NULL) { + strncat(error_str, ": ", sizeof(error_str)); + strncat(error_str, oserr, sizeof(error_str)); + errno = 0; + } + error->message = git__strdup(error_str); error->klass = error_class; diff --git a/src/filebuf.c b/src/filebuf.c index e6e68014a..8297b4fcf 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -45,8 +45,8 @@ static int lock_file(git_filebuf *file, int flags) source = p_open(file->path_original, O_RDONLY); if (source < 0) { giterr_set(GITERR_OS, - "Failed to open file '%s' for reading: %s", - file->path_original, strerror(errno)); + "Failed to open file '%s' for reading", + file->path_original); return -1; } diff --git a/src/fileops.c b/src/fileops.c index 4414c86c6..7ee39f1d5 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -37,7 +37,7 @@ int git_futils_mktmp(git_buf *path_out, const char *filename) if ((fd = p_mkstemp(path_out->ptr)) < 0) { giterr_set(GITERR_OS, - "Failed to create temporary file '%s': %s", path_out->ptr, strerror(errno)); + "Failed to create temporary file '%s'", path_out->ptr); return -1; } @@ -53,8 +53,7 @@ int git_futils_creat_withpath(const char *path, const mode_t dirmode, const mode fd = p_creat(path, mode); if (fd < 0) { - giterr_set(GITERR_OS, - "Failed to create file '%s': %s", path, strerror(errno)); + giterr_set(GITERR_OS, "Failed to create file '%s'", path); return -1; } @@ -65,8 +64,7 @@ int git_futils_creat_locked(const char *path, const mode_t mode) { int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_EXCL, mode); if (fd < 0) { - giterr_set(GITERR_OS, - "Failed to create locked file '%s': %s", path, strerror(errno)); + giterr_set(GITERR_OS, "Failed to create locked file '%s'", path); return -1; } @@ -115,10 +113,8 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime, if (updated != NULL) *updated = 0; - if ((fd = p_open(path, O_RDONLY)) < 0) { - giterr_set(GITERR_OS, "Failed to read file '%s': %s", path, strerror(errno)); - return (errno == ENOENT) ? GIT_ENOTFOUND : -1; - } + if ((fd = git_futils_open_ro(path)) < 0) + return fd; if (p_fstat(fd, &st) < 0 || S_ISDIR(st.st_mode) || !git__is_sizet(st.st_size+1)) { close(fd); @@ -154,8 +150,7 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime, if (read_size < 0) { close(fd); - giterr_set(GITERR_OS, "Failed to read descriptor for %s: %s", - path, strerror(errno)); + giterr_set(GITERR_OS, "Failed to read descriptor for '%s'", path); return -1; } diff --git a/src/fileops.h b/src/fileops.h index ab57b6f38..c2ba8ffc8 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -76,6 +76,20 @@ extern int git_futils_mktmp(git_buf *path_out, const char *filename); */ extern int git_futils_mv_withpath(const char *from, const char *to, const mode_t dirmode); +/** + * Open a file readonly and set error if needed + */ +GIT_INLINE(int) git_futils_open_ro(const char *path) +{ + int fd = p_open(path, O_RDONLY); + if (fd < 0) { + if (errno == ENOENT) + fd = GIT_ENOTFOUND; + giterr_set(GITERR_OS, "Failed to open '%s'", path); + } + return fd; +} + /** * Get the filesize in bytes of a file */ diff --git a/src/index.c b/src/index.c index b646dfcbb..d5410a3a7 100644 --- a/src/index.c +++ b/src/index.c @@ -544,7 +544,7 @@ const git_index_entry_unmerged *git_index_get_unmerged_bypath(git_index *index, if (!index->unmerged.length) return NULL; - if ((pos = git_vector_bsearch2(&index->unmerged, unmerged_srch, path)) < GIT_SUCCESS) + if ((pos = git_vector_bsearch2(&index->unmerged, unmerged_srch, path)) < 0) return NULL; return git_vector_get(&index->unmerged, pos); diff --git a/src/odb.c b/src/odb.c index 53e07519d..7ca8c21fe 100644 --- a/src/odb.c +++ b/src/odb.c @@ -32,10 +32,7 @@ static int format_object_header(char *hdr, size_t n, size_t obj_len, git_otype o { const char *type_str = git_object_type2string(obj_type); int len = p_snprintf(hdr, n, "%s %"PRIuZ, type_str, obj_len); - - if (len < 0 || len >= (int)n) - return git__throw(GIT_ERROR, "Cannot format object header. Length is out of bounds"); - + assert(len > 0 && len <= (int)n); return len+1; } @@ -48,13 +45,11 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj) assert(id && obj); if (!git_object_typeisloose(obj->type)) - return git__throw(GIT_ERROR, "Failed to hash object. Wrong object type"); - + return -1; if (!obj->data && obj->len != 0) - return git__throw(GIT_ERROR, "Failed to hash object. No data given"); + return -1; - if ((hdrlen = format_object_header(header, sizeof(header), obj->len, obj->type)) < 0) - return git__rethrow(hdrlen, "Failed to hash object"); + hdrlen = format_object_header(header, sizeof(header), obj->len, obj->type); vec[0].data = header; vec[0].len = hdrlen; @@ -63,7 +58,7 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj) git_hash_vec(id, vec, 2); - return GIT_SUCCESS; + return 0; } @@ -120,8 +115,6 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type) git_hash_ctx *ctx; hdr_len = format_object_header(hdr, sizeof(hdr), size, type); - if (hdr_len < 0) - return git__throw(GIT_ERROR, "Failed to format blob header. Length is out of bounds"); ctx = git_hash_new_ctx(); @@ -132,7 +125,8 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type) if (read_len < 0) { git_hash_free_ctx(ctx); - return git__throw(GIT_EOSERR, "Error when reading file: %s", strerror(errno)); + giterr_set(GITERR_OS, "Error reading file"); + return -1; } git_hash_update(ctx, buffer, read_len); @@ -142,69 +136,69 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type) git_hash_final(out, ctx); git_hash_free_ctx(ctx); - return GIT_SUCCESS; + return 0; } int git_odb__hashlink(git_oid *out, const char *path) { struct stat st; - int error; git_off_t size; + int result; - error = p_lstat(path, &st); - if (error < 0) - return git__throw(GIT_EOSERR, "Failed to stat blob. %s", strerror(errno)); + if (p_lstat(path, &st) < 0) { + giterr_set(GITERR_OS, "Failed to stat object '%s'", path); + return -1; + } size = st.st_size; - if (!git__is_sizet(size)) - return git__throw(GIT_EOSERR, "File size overflow for 32-bit systems"); + if (!git__is_sizet(size)) { + giterr_set(GITERR_OS, "File size overflow for 32-bit systems"); + return -1; + } if (S_ISLNK(st.st_mode)) { char *link_data; ssize_t read_len; link_data = git__malloc((size_t)size); - if (link_data == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(link_data); read_len = p_readlink(path, link_data, (size_t)(size + 1)); - if (read_len != (ssize_t)size) - return git__throw(GIT_EOSERR, "Failed to read symlink data"); + if (read_len != (ssize_t)size) { + giterr_set(GITERR_OS, "Failed to read symlink data for '%s'", path); + return -1; + } - error = git_odb_hash(out, link_data, (size_t)size, GIT_OBJ_BLOB); + result = git_odb_hash(out, link_data, (size_t)size, GIT_OBJ_BLOB); free(link_data); } else { - int fd; - - if ((fd = p_open(path, O_RDONLY)) < 0) - return git__throw(GIT_ENOTFOUND, "Could not open '%s'", path); - - error = git_odb__hashfd(out, fd, (size_t)size, GIT_OBJ_BLOB); + int fd = git_futils_open_ro(path); + if (fd < 0) + return -1; + result = git_odb__hashfd(out, fd, (size_t)size, GIT_OBJ_BLOB); p_close(fd); } - return error; + return result; } int git_odb_hashfile(git_oid *out, const char *path, git_otype type) { - int fd, error; git_off_t size; - - if ((fd = p_open(path, O_RDONLY)) < 0) - return git__throw(GIT_ENOTFOUND, "Could not open '%s'", path); + int result, fd = git_futils_open_ro(path); + if (fd < 0) + return -1; if ((size = git_futils_filesize(fd)) < 0 || !git__is_sizet(size)) { + giterr_set(GITERR_OS, "File size overflow for 32-bit systems"); p_close(fd); - return git__throw(GIT_EOSERR, - "File size overflow. The object is too big to fit in 32-bit mode"); + return -1; } - error = git_odb__hashfd(out, fd, (size_t)size, type); - + result = git_odb__hashfd(out, fd, (size_t)size, type); p_close(fd); - return error; + return result; } int git_odb_hash(git_oid *id, const void *data, size_t len, git_otype type) @@ -242,11 +236,11 @@ static int fake_wstream__write(git_odb_stream *_stream, const char *data, size_t fake_wstream *stream = (fake_wstream *)_stream; if (stream->written + len > stream->size) - return GIT_ENOMEM; + return -1; memcpy(stream->buffer + stream->written, data, len); stream->written += len; - return GIT_SUCCESS; + return 0; } static void fake_wstream__free(git_odb_stream *_stream) @@ -262,15 +256,14 @@ static int init_fake_wstream(git_odb_stream **stream_p, git_odb_backend *backend fake_wstream *stream; stream = git__calloc(1, sizeof(fake_wstream)); - if (stream == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(stream); stream->size = size; stream->type = type; stream->buffer = git__malloc(size); if (stream->buffer == NULL) { git__free(stream); - return GIT_ENOMEM; + return -1; } stream->stream.backend = backend; @@ -281,7 +274,7 @@ static int init_fake_wstream(git_odb_stream **stream_p, git_odb_backend *backend stream->stream.mode = GIT_STREAM_WRONLY; *stream_p = (git_odb_stream *)stream; - return GIT_SUCCESS; + return 0; } /*********************************************************** @@ -305,26 +298,19 @@ static int backend_sort_cmp(const void *a, const void *b) int git_odb_new(git_odb **out) { - int error; - git_odb *db = git__calloc(1, sizeof(*db)); - if (!db) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(db); - error = git_cache_init(&db->cache, GIT_DEFAULT_CACHE_SIZE, &free_odb_object); - if (error < GIT_SUCCESS) { + if (git_cache_init(&db->cache, GIT_DEFAULT_CACHE_SIZE, &free_odb_object) < 0 || + git_vector_init(&db->backends, 4, backend_sort_cmp) < 0) + { git__free(db); - return git__rethrow(error, "Failed to create object database"); - } - - if ((error = git_vector_init(&db->backends, 4, backend_sort_cmp)) < GIT_SUCCESS) { - git__free(db); - return git__rethrow(error, "Failed to create object database"); + return -1; } *out = db; GIT_REFCOUNT_INC(db); - return GIT_SUCCESS; + return 0; } static int add_backend_internal(git_odb *odb, git_odb_backend *backend, int priority, int is_alternate) @@ -333,12 +319,15 @@ static int add_backend_internal(git_odb *odb, git_odb_backend *backend, int prio assert(odb && backend); - if (backend->odb != NULL && backend->odb != odb) + if (backend->odb != NULL && backend->odb != odb) { + /* + * TODO: Not sure how to convert this! + */ return git__throw(GIT_EBUSY, "The backend is already owned by another ODB"); + } internal = git__malloc(sizeof(backend_internal)); - if (internal == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(internal); internal->backend = backend; internal->priority = priority; @@ -346,12 +335,12 @@ static int add_backend_internal(git_odb *odb, git_odb_backend *backend, int prio if (git_vector_insert(&odb->backends, internal) < 0) { git__free(internal); - return GIT_ENOMEM; + return -1; } git_vector_sort(&odb->backends); internal->backend->odb = odb; - return GIT_SUCCESS; + return 0; } int git_odb_add_backend(git_odb *odb, git_odb_backend *backend, int priority) @@ -367,27 +356,18 @@ int git_odb_add_alternate(git_odb *odb, git_odb_backend *backend, int priority) static int add_default_backends(git_odb *db, const char *objects_dir, int as_alternates) { git_odb_backend *loose, *packed; - int error; /* add the loose object backend */ - error = git_odb_backend_loose(&loose, objects_dir, -1, 0); - if (error < GIT_SUCCESS) - return error; - - error = add_backend_internal(db, loose, GIT_LOOSE_PRIORITY, as_alternates); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to add backend"); + if (git_odb_backend_loose(&loose, objects_dir, -1, 0) < 0 || + add_backend_internal(db, loose, GIT_LOOSE_PRIORITY, as_alternates) < 0) + return -1; /* add the packed file backend */ - error = git_odb_backend_pack(&packed, objects_dir); - if (error < GIT_SUCCESS) - return error; + if (git_odb_backend_pack(&packed, objects_dir) < 0 || + add_backend_internal(db, packed, GIT_PACKED_PRIORITY, as_alternates) < 0) + return -1; - error = add_backend_internal(db, packed, GIT_PACKED_PRIORITY, as_alternates); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to add backend"); - - return GIT_SUCCESS; + return 0; } static int load_alternates(git_odb *odb, const char *objects_dir) @@ -396,24 +376,22 @@ static int load_alternates(git_odb *odb, const char *objects_dir) git_buf alternates_buf = GIT_BUF_INIT; char *buffer; const char *alternate; - int error; + int result = 0; - error = git_buf_joinpath(&alternates_path, objects_dir, GIT_ALTERNATES_FILE); - if (error < GIT_SUCCESS) - return error; + if (git_buf_joinpath(&alternates_path, objects_dir, GIT_ALTERNATES_FILE) < 0) + return -1; if (git_path_exists(alternates_path.ptr) == false) { git_buf_free(&alternates_path); - return GIT_SUCCESS; + return 0; } - if (git_futils_readbuffer(&alternates_buf, alternates_path.ptr) < GIT_SUCCESS) { + if (git_futils_readbuffer(&alternates_buf, alternates_path.ptr) < 0) { git_buf_free(&alternates_path); - return git__throw(GIT_EOSERR, "Failed to add backend. Can't read alternates"); + return -1; } buffer = (char *)alternates_buf.ptr; - error = GIT_SUCCESS; /* add each alternate as a new backend; one alternate per line */ while ((alternate = git__strtok(&buffer, "\r\n")) != NULL) { @@ -422,48 +400,41 @@ static int load_alternates(git_odb *odb, const char *objects_dir) /* relative path: build based on the current `objects` folder */ if (*alternate == '.') { - error = git_buf_joinpath(&alternates_path, objects_dir, alternate); - if (error < GIT_SUCCESS) + if ((result = git_buf_joinpath(&alternates_path, objects_dir, alternate)) < 0) break; alternate = git_buf_cstr(&alternates_path); } - if ((error = add_default_backends(odb, alternate, 1)) < GIT_SUCCESS) + if ((result = add_default_backends(odb, alternate, 1)) < 0) break; } git_buf_free(&alternates_path); git_buf_free(&alternates_buf); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to load alternates"); - return error; + return result; } int git_odb_open(git_odb **out, const char *objects_dir) { git_odb *db; - int error; assert(out && objects_dir); *out = NULL; - if ((error = git_odb_new(&db)) < 0) - return git__rethrow(error, "Failed to open ODB"); + if (git_odb_new(&db) < 0) + return -1; - if ((error = add_default_backends(db, objects_dir, 0)) < GIT_SUCCESS) - goto cleanup; - - if ((error = load_alternates(db, objects_dir)) < GIT_SUCCESS) - goto cleanup; + if (add_default_backends(db, objects_dir, 0) < 0 || + load_alternates(db, objects_dir) < 0) + { + git_odb_free(db); + return -1; + } *out = db; - return GIT_SUCCESS; - -cleanup: - git_odb_free(db); - return error; /* error already set - pass through */ + return 0; } static void odb_free(git_odb *db) @@ -497,13 +468,13 @@ int git_odb_exists(git_odb *db, const git_oid *id) { git_odb_object *object; unsigned int i; - int found = 0; + bool found = false; assert(db && id); if ((object = git_cache_get(&db->cache, id)) != NULL) { git_odb_object_free(object); - return 1; + return (int)true; } for (i = 0; i < db->backends.length && !found; ++i) { @@ -514,7 +485,7 @@ int git_odb_exists(git_odb *db, const git_oid *id) found = b->exists(b, id); } - return found; + return (int)found; } int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git_oid *id) @@ -529,7 +500,7 @@ int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git *len_p = object->raw.len; *type_p = object->raw.type; git_odb_object_free(object); - return GIT_SUCCESS; + return 0; } for (i = 0; i < db->backends.length && error < 0; ++i) { @@ -541,7 +512,7 @@ int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git } if (error == GIT_EPASSTHROUGH) - return GIT_SUCCESS; + return 0; /* * no backend could read only the header. diff --git a/src/path.c b/src/path.c index 8d0cf288f..1ff257a98 100644 --- a/src/path.c +++ b/src/path.c @@ -499,7 +499,7 @@ int git_path_direach( wd_len = path->size; dir = opendir(path->ptr); if (!dir) { - giterr_set(GITERR_OS, "Failed to `opendir` %s: %s", path->ptr, strerror(errno)); + giterr_set(GITERR_OS, "Failed to 'opendir' %s", path->ptr); return -1; } diff --git a/src/refs.c b/src/refs.c index 461b50719..e90cf5de1 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1021,8 +1021,7 @@ static int reference_delete(git_reference *ref) git_buf_free(&full_path); /* done with path at this point */ if (result < 0) { - giterr_set(GITERR_OS, - "Failed to unlink '%s': %s", full_path.ptr, strerror(errno)); + giterr_set(GITERR_OS, "Failed to unlink '%s'", full_path.ptr); return -1; } diff --git a/src/util.c b/src/util.c index 1fb01170b..d2309124b 100644 --- a/src/util.c +++ b/src/util.c @@ -355,23 +355,26 @@ int git__bsearch( int (*compare)(const void *, const void *), size_t *position) { - int lim, cmp; + int lim, cmp = -1; void **part, **base = array; for (lim = array_len; lim != 0; lim >>= 1) { part = base + (lim >> 1); cmp = (*compare)(key, *part); if (cmp == 0) { - *position = (part - array); - return GIT_SUCCESS; - } else if (cmp > 0) { /* key > p; take right partition */ + base = part; + break; + } + if (cmp > 0) { /* key > p; take right partition */ base = part + 1; lim--; } /* else take left partition */ } - *position = (base - array); - return GIT_ENOTFOUND; + if (position) + *position = (base - array); + + return (cmp == 0) ? 0 : -1; } /** diff --git a/src/util.h b/src/util.h index 803c63d73..01755b59e 100644 --- a/src/util.h +++ b/src/util.h @@ -115,6 +115,11 @@ extern int git__fnmatch(const char *pattern, const char *name, int flags); extern void git__tsort(void **dst, size_t size, int (*cmp)(const void *, const void *)); +/** + * @param position If non-NULL, this will be set to the position where the + * element is or would be inserted if not found. + * @return pos (>=0) if found or -1 if not found + */ extern int git__bsearch( void **array, size_t array_len, diff --git a/src/vector.c b/src/vector.c index 7513ea3f0..d73c2b418 100644 --- a/src/vector.c +++ b/src/vector.c @@ -19,10 +19,9 @@ static int resize_vector(git_vector *v) v->_alloc_size = minimum_size; v->contents = git__realloc(v->contents, v->_alloc_size * sizeof(void *)); - if (v->contents == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(v->contents); - return GIT_SUCCESS; + return 0; } void git_vector_free(git_vector *v) @@ -52,30 +51,29 @@ int git_vector_init(git_vector *v, unsigned int initial_size, git_vector_cmp cmp v->sorted = 1; v->contents = git__malloc(v->_alloc_size * sizeof(void *)); - if (v->contents == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(v->contents); - return GIT_SUCCESS; + return 0; } int git_vector_insert(git_vector *v, void *element) { assert(v); - if (v->length >= v->_alloc_size) { - if (resize_vector(v) < 0) - return GIT_ENOMEM; - } + if (v->length >= v->_alloc_size && + resize_vector(v) < 0) + return -1; v->contents[v->length++] = element; v->sorted = 0; - return GIT_SUCCESS; + return 0; } -int git_vector_insert_sorted(git_vector *v, void *element, int (*on_dup)(void **old, void *new)) +int git_vector_insert_sorted( + git_vector *v, void *element, int (*on_dup)(void **old, void *new)) { - int error = GIT_SUCCESS; + int result; size_t pos; assert(v && v->_cmp); @@ -83,22 +81,18 @@ int git_vector_insert_sorted(git_vector *v, void *element, int (*on_dup)(void ** if (!v->sorted) git_vector_sort(v); - if (v->length >= v->_alloc_size) { - if (resize_vector(v) < 0) - return GIT_ENOMEM; - } + if (v->length >= v->_alloc_size && + resize_vector(v) < 0) + return -1; - error = git__bsearch(v->contents, v->length, element, v->_cmp, &pos); - - /* If we found the element and have a duplicate handler callback, - * invoke it. If it returns an error, then cancel insert, otherwise + /* If we find the element and have a duplicate handler callback, + * invoke it. If it returns non-zero, then cancel insert, otherwise * proceed with normal insert. */ - if (error == GIT_SUCCESS && on_dup != NULL) { - error = on_dup(&v->contents[pos], element); - if (error != GIT_SUCCESS) - return error; - } + if (git__bsearch(v->contents, v->length, element, v->_cmp, &pos) >= 0 && + on_dup != NULL && + (result = on_dup(&v->contents[pos], element)) < 0) + return result; /* shift elements to the right */ if (pos < v->length) { @@ -108,8 +102,7 @@ int git_vector_insert_sorted(git_vector *v, void *element, int (*on_dup)(void ** v->contents[pos] = element; v->length++; - - return GIT_SUCCESS; + return 0; } void git_vector_sort(git_vector *v) @@ -130,16 +123,14 @@ int git_vector_bsearch2(git_vector *v, git_vector_cmp key_lookup, const void *ke assert(v && key && key_lookup); /* need comparison function to sort the vector */ - if (v->_cmp == NULL) - return git__throw(GIT_ENOTFOUND, "Can't sort vector. No comparison function set"); + assert(v->_cmp != NULL); git_vector_sort(v); - if (git__bsearch(v->contents, v->length, key, key_lookup, - &pos) == GIT_SUCCESS) + if (git__bsearch(v->contents, v->length, key, key_lookup, &pos) >= 0) return (int)pos; - return git__throw(GIT_ENOTFOUND, "Can't find element"); + return GIT_ENOTFOUND; } int git_vector_search2(git_vector *v, git_vector_cmp key_lookup, const void *key) @@ -153,7 +144,7 @@ int git_vector_search2(git_vector *v, git_vector_cmp key_lookup, const void *key return i; } - return git__throw(GIT_ENOTFOUND, "Can't find element"); + return GIT_ENOTFOUND; } static int strict_comparison(const void *a, const void *b) @@ -178,13 +169,13 @@ int git_vector_remove(git_vector *v, unsigned int idx) assert(v); if (idx >= v->length || v->length == 0) - return git__throw(GIT_ENOTFOUND, "Can't remove element. Index out of bounds"); + return GIT_ENOTFOUND; for (i = idx; i < v->length - 1; ++i) v->contents[i] = v->contents[i + 1]; v->length--; - return GIT_SUCCESS; + return 0; } void git_vector_pop(git_vector *v)