From 1795f87952a68155a618523799f70473483c7265 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 5 Nov 2010 03:20:17 +0200 Subject: [PATCH] Improve error handling All initialization functions now return error codes instead of pointers. Error codes are now properly propagated on most functions. Several new and more specific error codes have been added in common.h Signed-off-by: Vicent Marti --- src/commit.c | 43 ++++++++----------- src/git/commit.h | 10 +++-- src/git/common.h | 22 ++++++++++ src/git/index.h | 5 ++- src/git/repository.h | 14 +++--- src/git/revwalk.h | 7 +-- src/git/tag.h | 10 +++-- src/git/tree.h | 14 +++--- src/index.c | 30 ++++++------- src/odb.c | 31 +++++++------- src/repository.c | 96 ++++++++++++++++++++++++++++-------------- src/revwalk.c | 30 +++++++------ src/tag.c | 29 ++++--------- src/tree.c | 18 ++------ tests/t0401-parse.c | 3 +- tests/t0402-details.c | 6 +-- tests/t0403-write.c | 21 +++------ tests/t0501-walk.c | 10 ++--- tests/t0601-read.c | 15 +++---- tests/t0602-write.c | 3 +- tests/t0603-sort.c | 7 ++- tests/t0801-readtag.c | 6 +-- tests/t0802-write.c | 6 +-- tests/t0901-readtree.c | 15 +++---- tests/t0902-modify.c | 13 ++---- 25 files changed, 233 insertions(+), 231 deletions(-) diff --git a/src/commit.c b/src/commit.c index d0eb59f51..1f365c927 100644 --- a/src/commit.c +++ b/src/commit.c @@ -67,28 +67,17 @@ void git_commit__free(git_commit *commit) free(commit); } -git_commit *git_commit_new(git_repository *repo) -{ - return (git_commit *)git_object_new(repo, GIT_OBJ_COMMIT); -} - const git_oid *git_commit_id(git_commit *c) { return git_object_id((git_object *)c); } - -git_commit *git_commit_lookup(git_repository *repo, const git_oid *id) -{ - return (git_commit *)git_repository_lookup(repo, id, GIT_OBJ_COMMIT); -} - int git_commit__writeback(git_commit *commit, git_odb_source *src) { git_commit_parents *parent; if (commit->tree == NULL) - return GIT_ERROR; + return GIT_EMISSINGOBJDATA; git__write_oid(src, "tree", git_tree_id(commit->tree)); @@ -100,12 +89,12 @@ int git_commit__writeback(git_commit *commit, git_odb_source *src) } if (commit->author == NULL) - return GIT_ERROR; + return GIT_EMISSINGOBJDATA; git_person__write(src, "author", commit->author); if (commit->committer == NULL) - return GIT_ERROR; + return GIT_EMISSINGOBJDATA; git_person__write(src, "committer", commit->committer); @@ -124,11 +113,13 @@ int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int const char *buffer_end = (char *)data + len; git_oid oid; + int error; - if (git__parse_oid(&oid, &buffer, buffer_end, "tree ") < 0) - return GIT_EOBJCORRUPTED; + if ((error = git__parse_oid(&oid, &buffer, buffer_end, "tree ")) < 0) + return error; - commit->tree = git_tree_lookup(commit->object.repo, &oid); + if ((error = git_repository_lookup((git_object **)&commit->tree, commit->object.repo, &oid, GIT_OBJ_TREE)) < 0) + return error; /* * TODO: commit grafts! @@ -140,8 +131,8 @@ int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int git_commit *parent; git_commit_parents *node; - if ((parent = git_commit_lookup(commit->object.repo, &oid)) == NULL) - return GIT_ENOTFOUND; + if ((error = git_repository_lookup((git_object **)&parent, commit->object.repo, &oid, GIT_OBJ_COMMIT)) < 0) + return error; if ((node = git__malloc(sizeof(git_commit_parents))) == NULL) return GIT_ENOMEM; @@ -157,8 +148,8 @@ int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int git_person__free(commit->author); commit->author = git__malloc(sizeof(git_person)); - if (git_person__parse(commit->author, &buffer, buffer_end, "author ") < 0) - return GIT_EOBJCORRUPTED; + if ((error = git_person__parse(commit->author, &buffer, buffer_end, "author ")) < 0) + return error; } else { if ((buffer = memchr(buffer, '\n', buffer_end - buffer)) == NULL) @@ -172,8 +163,8 @@ int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int git_person__free(commit->committer); commit->committer = git__malloc(sizeof(git_person)); - if (git_person__parse(commit->committer, &buffer, buffer_end, "committer ") < 0) - return GIT_EOBJCORRUPTED; + if ((error = git_person__parse(commit->committer, &buffer, buffer_end, "committer ")) < 0) + return error; commit->commit_time = commit->committer->time; @@ -200,7 +191,7 @@ int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int commit->message_short[message_len] = 0; } - return 0; + return GIT_SUCCESS; } int git_commit__parse(git_commit *commit) @@ -217,8 +208,8 @@ int git_commit__parse_full(git_commit *commit) if (commit->full_parse) return 0; - if (git_object__source_open((git_object *)commit) < 0) - return GIT_ERROR; + if ((error = git_object__source_open((git_object *)commit)) < 0) + return error; error = commit_parse_buffer(commit, commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_FULL_PARSE); diff --git a/src/git/commit.h b/src/git/commit.h index 5d3fa0adb..8cb3401b6 100644 --- a/src/git/commit.h +++ b/src/git/commit.h @@ -23,12 +23,13 @@ typedef struct git_commit git_commit; * The generated commit object is owned by the revision * repo and shall not be freed by the user. * + * @param commit pointer to the looked up commit * @param repo the repo to use when locating the commit. * @param id identity of the commit to locate. If the object is * an annotated tag it will be peeled back to the commit. - * @return the commit; NULL if the commit could not be created + * @return 0 on success; error code otherwise */ -GIT_EXTERN(git_commit *) git_commit_lookup(git_repository *repo, const git_oid *id); +GIT_EXTERN(int) git_commit_lookup(git_commit **commit, git_repository *repo, const git_oid *id); /** * Create a new in-memory git_commit. @@ -37,10 +38,11 @@ GIT_EXTERN(git_commit *) git_commit_lookup(git_repository *repo, const git_oid * * setter methods before it can be written to its * repository. * + * @param commit pointer to the new commit * @param repo The repository where the object will reside - * @return the object if creation was possible; NULL otherwise + * @return 0 on success; error code otherwise */ -GIT_EXTERN(git_commit *) git_commit_new(git_repository *repo); +GIT_EXTERN(int) git_commit_new(git_commit ** commit, git_repository *repo); /** * Get the id of a commit. diff --git a/src/git/common.h b/src/git/common.h index 35408413d..82f08ac5d 100644 --- a/src/git/common.h +++ b/src/git/common.h @@ -84,6 +84,28 @@ /** The specified object has its data corrupted */ #define GIT_EOBJCORRUPTED (GIT_ERROR - 6) +/** The specified repository is invalid */ +#define GIT_ENOTAREPO (GIT_ERROR - 7) + +/** The object type is invalid or doesn't match */ +#define GIT_EINVALIDTYPE (GIT_ERROR - 8) + +/** The object cannot be written that because it's missing internal data */ +#define GIT_EMISSINGOBJDATA (GIT_ERROR - 9) + +/** The packfile for the ODB is corrupted */ +#define GIT_EPACKCORRUPTED (GIT_ERROR - 10) + +/** Failed to adquire or release a file lock */ +#define GIT_EFLOCKFAIL (GIT_ERROR - 11) + +/** The Z library failed to inflate/deflate an object's data */ +#define GIT_EZLIB (GIT_ERROR - 12) + +/** The queried object is currently busy */ +#define GIT_EBUSY (GIT_ERROR - 13) + + GIT_BEGIN_DECL /** diff --git a/src/git/index.h b/src/git/index.h index 3b262355e..ee410865d 100644 --- a/src/git/index.h +++ b/src/git/index.h @@ -56,11 +56,12 @@ typedef struct git_index_entry { * If 'working _dir' is NULL (e.g for bare repositories), the * methods working on on-disk files will fail. * + * @param index the pointer for the new index * @param index_path the path to the index file in disk * @param working_dir working dir for the git repository - * @return the index object; NULL if the index could not be created + * @return 0 on success; error code otherwise */ -GIT_EXTERN(git_index *) git_index_alloc(const char *index_path, const char *working_dir); +GIT_EXTERN(int) git_index_open(git_index **index, const char *index_path, const char *working_dir); /** * Clear the contents (all the entries) of an index object. diff --git a/src/git/repository.h b/src/git/repository.h index 058849b7f..81eb1ea59 100644 --- a/src/git/repository.h +++ b/src/git/repository.h @@ -34,10 +34,11 @@ GIT_BEGIN_DECL * The method will automatically detect if 'path' is a normal * or bare repository or fail is 'path' is neither. * + * @param repository pointer to the repo which will be opened * @param path the path to the repository * @return the new repository handle; NULL on error */ -GIT_EXTERN(git_repository *) git_repository_open(const char *path); +GIT_EXTERN(int) git_repository_open(git_repository **repository, const char *path); /** @@ -45,23 +46,19 @@ GIT_EXTERN(git_repository *) git_repository_open(const char *path); * * The generated reference is owned by the repository and * should not be freed by the user. - * The generated reference should be cast back to the - * expected type; e.g. - * - * git_commit *c = (git_commit *) - * git_repository_lookup(repo, id, GIT_OBJ_COMMIT); * * The 'type' parameter must match the type of the object * in the odb; the method will fail otherwise. * The special value 'GIT_OBJ_ANY' may be passed to let * the method guess the object's type. * + * @param object pointer to the looked-up object * @param repo the repository to look up the object * @param id the unique identifier for the object * @param type the type of the object * @return a reference to the object */ -GIT_EXTERN(git_object *) git_repository_lookup(git_repository *repo, const git_oid *id, git_otype type); +GIT_EXTERN(int) git_repository_lookup(git_object **object, git_repository *repo, const git_oid *id, git_otype type); /** * Get the object database behind a Git repository @@ -96,11 +93,12 @@ GIT_EXTERN(git_index *) git_repository_index(git_repository *rpeo); * will be automatically generated when writing to the * repository. * + * @param object pointer to the new object * @parem repo Repository where the object belongs * @param type Type of the object to be created * @return the new object */ -GIT_EXTERN(git_object *) git_object_new(git_repository *repo, git_otype type); +GIT_EXTERN(int) git_repository_newobject(git_object **object, git_repository *repo, git_otype type); /** * Write back an object to disk. diff --git a/src/git/revwalk.h b/src/git/revwalk.h index 42ffbc3e7..49fd32174 100644 --- a/src/git/revwalk.h +++ b/src/git/revwalk.h @@ -48,10 +48,11 @@ typedef struct git_revwalk git_revwalk; /** * Allocate a new revision walker to iterate through a repo. * + * @param walker pointer to the new revision walker * @param repo the repo to walk through - * @return the new walker handle; NULL if memory is exhausted. + * @return 0 on success; error code otherwise */ -GIT_EXTERN(git_revwalk *) git_revwalk_alloc(git_repository *repo); +GIT_EXTERN(int) git_revwalk_new(git_revwalk **walker, git_repository *repo); /** * Reset the walking machinery for reuse. @@ -89,7 +90,7 @@ GIT_EXTERN(git_commit *) git_revwalk_next(git_revwalk *walk); * @param walk the walker being used for the traversal. * @param sort_mode combination of GIT_RPSORT_XXX flags */ -GIT_EXTERN(void) git_revwalk_sorting(git_revwalk *walk, unsigned int sort_mode); +GIT_EXTERN(int) git_revwalk_sorting(git_revwalk *walk, unsigned int sort_mode); /** * Free a revwalk previously allocated. diff --git a/src/git/tag.h b/src/git/tag.h index e1cd1ab85..cc15651ab 100644 --- a/src/git/tag.h +++ b/src/git/tag.h @@ -23,11 +23,12 @@ typedef struct git_tag git_tag; * The generated tag object is owned by the revision * repo and shall not be freed by the user. * + * @param tag pointer to the looked up tag * @param repo the repo to use when locating the tag. * @param id identity of the tag to locate. - * @return the tag; NULL if the tag could not be created + * @return 0 on success; error code otherwise */ -GIT_EXTERN(git_tag *) git_tag_lookup(git_repository *repo, const git_oid *id); +GIT_EXTERN(int) git_tag_lookup(git_tag **tag, git_repository *repo, const git_oid *id); /** * Create a new in-memory git_tag. @@ -36,10 +37,11 @@ GIT_EXTERN(git_tag *) git_tag_lookup(git_repository *repo, const git_oid *id); * setter methods before it can be written to its * repository. * + * @param tag pointer to the new tag * @param repo The repository where the object will reside - * @return the object if creation was possible; NULL otherwise + * @return 0 on success; error code otherwise */ -GIT_EXTERN(git_tag *) git_tag_new(git_repository *repo); +GIT_EXTERN(int) git_tag_new(git_tag **tag, git_repository *repo); /** * Get the id of a tag. diff --git a/src/git/tree.h b/src/git/tree.h index 190fe95d6..f471113a8 100644 --- a/src/git/tree.h +++ b/src/git/tree.h @@ -26,11 +26,12 @@ typedef struct git_tree git_tree; * The generated tree object is owned by the revision * repo and shall not be freed by the user. * + * @param tree pointer to the looked up tree * @param repo the repo to use when locating the tree. * @param id identity of the tree to locate. - * @return the tree; NULL if the tree could not be created + * @return 0 on success; error code otherwise */ -GIT_EXTERN(git_tree *) git_tree_lookup(git_repository *repo, const git_oid *id); +GIT_EXTERN(int) git_tree_lookup(git_tree **tree, git_repository *repo, const git_oid *id); /** * Create a new in-memory git_tree. @@ -39,10 +40,11 @@ GIT_EXTERN(git_tree *) git_tree_lookup(git_repository *repo, const git_oid *id); * setter methods before it can be written to its * repository. * + * @param tree pointer to the new tree * @param repo The repository where the object will reside - * @return the object if creation was possible; NULL otherwise + * @return 0 on success; error code otherwise */ -GIT_EXTERN(git_tree *) git_tree_new(git_repository *repo); +GIT_EXTERN(int) git_tree_new(git_tree **tree, git_repository *repo); /** * Get the id of a tree. @@ -98,10 +100,12 @@ GIT_EXTERN(const git_oid *) git_tree_entry_id(git_tree_entry *entry); /** * Convert a tree entry to the git_object it points too. + * + * @param object pointer to the converted object * @param entry a tree entry * @return a reference to the pointed object in the repository */ -GIT_EXTERN(git_object *) git_tree_entry_2object(git_tree_entry *entry); +GIT_EXTERN(int) git_tree_entry_2object(git_object **object, git_tree_entry *entry); /** * Add a new entry to a tree. diff --git a/src/index.c b/src/index.c index 9996c50fe..71481cc2d 100644 --- a/src/index.c +++ b/src/index.c @@ -97,23 +97,22 @@ static int read_tree(git_index *index, const char *buffer, size_t buffer_size); static git_index_tree *read_tree_internal(const char **, const char *, git_index_tree *); -git_index *git_index_alloc(const char *index_path, const char *work_dir) +int git_index_open(git_index **index_out, const char *index_path, const char *work_dir) { git_index *index; - if (index_path == NULL) - return NULL; + assert(index_out && index_path); index = git__malloc(sizeof(git_index)); if (index == NULL) - return NULL; + return GIT_ENOMEM; memset(index, 0x0, sizeof(git_index)); index->index_file_path = git__strdup(index_path); if (index->index_file_path == NULL) { free(index); - return NULL; + return GIT_ENOMEM; } if (work_dir != NULL) @@ -123,7 +122,8 @@ git_index *git_index_alloc(const char *index_path, const char *work_dir) if (gitfo_exists(index->index_file_path) == 0) index->on_disk = 1; - return index; + *index_out = index; + return GIT_SUCCESS; } void git_index_clear(git_index *index) @@ -161,8 +161,7 @@ int git_index_read(git_index *index) struct stat indexst; int error = 0; - if (index->index_file_path == NULL) - return GIT_ERROR; + assert(index->index_file_path); if (!index->on_disk || gitfo_exists(index->index_file_path) < 0) { git_index_clear(index); @@ -178,7 +177,7 @@ int git_index_read(git_index *index) gitfo_buf buffer; if (gitfo_read_file(&buffer, index->index_file_path) < 0) - return GIT_EOSERR; + return GIT_ENOTFOUND; git_index_clear(index); error = git_index__parse(index, buffer.data, buffer.len); @@ -201,10 +200,10 @@ int git_index_write(git_index *index) git_index__sort(index); if (git_filelock_init(&file, index->index_file_path) < 0) - return GIT_EOSERR; + return GIT_EFLOCKFAIL; if (git_filelock_lock(&file, 0) < 0) - return GIT_EOSERR; + return GIT_EFLOCKFAIL; if (git_index__write(index, &file) < 0) { git_filelock_unlock(&file); @@ -212,7 +211,7 @@ int git_index_write(git_index *index) } if (git_filelock_commit(&file) < 0) - return GIT_EOSERR; + return GIT_EFLOCKFAIL; if (gitfo_stat(index->index_file_path, &indexst) == 0) { index->last_modified = indexst.st_mtime; @@ -301,7 +300,7 @@ int git_index__append(git_index *index, const git_index_entry *source_entry) memcpy(offset, source_entry, sizeof(git_index_entry)); index->sorted = 0; - return 0; + return GIT_SUCCESS; } int git_index__remove_pos(git_index *index, unsigned int position) @@ -628,11 +627,10 @@ int git_index__write(git_index *index, git_filelock *file) git_hash_ctx *digest; git_oid hash_final; - if (index == NULL || file == NULL || !file->is_locked) - return GIT_ERROR; + assert(index && file && file->is_locked); if ((digest = git_hash_new_ctx()) == NULL) - return GIT_ERROR; + return GIT_ENOMEM; #define WRITE_WORD(_word) {\ uint32_t network_word = htonl((_word));\ diff --git a/src/odb.c b/src/odb.c index ff258b5db..bf5ad7ca4 100644 --- a/src/odb.c +++ b/src/odb.c @@ -1590,7 +1590,7 @@ static int read_packed(git_rawobj *out, const obj_location *loc) assert(out && loc); if (pack_openidx(loc->pack.ptr)) - return GIT_ERROR; + return GIT_EPACKCORRUPTED; res = loc->pack.ptr->idx_get(&e, loc->pack.ptr, loc->pack.n); @@ -1614,7 +1614,7 @@ static int read_header_packed(git_rawobj *out, const obj_location *loc) pack = loc->pack.ptr; if (pack_openidx(pack)) - return GIT_ERROR; + return GIT_EPACKCORRUPTED; if (pack->idx_get(&e, pack, loc->pack.n) < 0 || open_pack(pack) < 0) { @@ -1691,7 +1691,7 @@ static int read_header_loose(git_rawobj *out, git_odb *db, const obj_location *l init_stream(&zs, inflated_buffer, sizeof(inflated_buffer)); if (inflateInit(&zs) < Z_OK) { - error = GIT_ERROR; + error = GIT_EZLIB; goto cleanup; } @@ -1725,15 +1725,15 @@ static int write_obj(gitfo_buf *buf, git_oid *id, git_odb *db) git_file fd; if (object_file_name(file, sizeof(file), db->objects_dir, id)) - return GIT_ERROR; + return GIT_EOSERR; if (make_temp_file(&fd, temp, sizeof(temp), file) < 0) - return GIT_ERROR; + return GIT_EOSERR; if (gitfo_write(fd, buf->data, buf->len) < 0) { gitfo_close(fd); gitfo_unlink(temp); - return GIT_ERROR; + return GIT_EOSERR; } if (db->fsync_object_files) @@ -1743,7 +1743,7 @@ static int write_obj(gitfo_buf *buf, git_oid *id, git_odb *db) if (gitfo_move_file(temp, file) < 0) { gitfo_unlink(temp); - return GIT_ERROR; + return GIT_EOSERR; } return GIT_SUCCESS; @@ -1766,12 +1766,12 @@ int git_odb_open(git_odb **out, const char *objects_dir) { git_odb *db = git__calloc(1, sizeof(*db)); if (!db) - return GIT_ERROR; + return GIT_ENOMEM; db->objects_dir = git__strdup(objects_dir); if (!db->objects_dir) { free(db); - return GIT_ERROR; + return GIT_ENOMEM; } gitlck_init(&db->lock); @@ -1898,21 +1898,22 @@ int git_odb_write(git_oid *id, git_odb *db, git_rawobj *obj) char hdr[64]; int hdrlen; gitfo_buf buf = GITFO_BUF_INIT; + int error; assert(id && db && obj); - if (hash_obj(id, hdr, sizeof(hdr), &hdrlen, obj) < 0) - return GIT_ERROR; + if ((error = hash_obj(id, hdr, sizeof(hdr), &hdrlen, obj)) < 0) + return error; if (git_odb_exists(db, id)) return GIT_SUCCESS; - if (deflate_obj(&buf, hdr, hdrlen, obj, db->object_zlib_level) < 0) - return GIT_ERROR; + if ((error = deflate_obj(&buf, hdr, hdrlen, obj, db->object_zlib_level)) < 0) + return error; - if (write_obj(&buf, id, db) < 0) { + if ((error = write_obj(&buf, id, db)) < 0) { gitfo_free_buf(&buf); - return GIT_ERROR; + return error; } gitfo_free_buf(&buf); diff --git a/src/repository.c b/src/repository.c index f64f806aa..8cef18218 100644 --- a/src/repository.c +++ b/src/repository.c @@ -71,7 +71,7 @@ static int parse_repository_folders(git_repository *repo, const char *repository int path_len, i; if (gitfo_isdir(repository_path) < 0) - return GIT_ERROR; + return GIT_ENOTAREPO; path_len = strlen(repository_path); strcpy(path_aux, repository_path); @@ -88,13 +88,13 @@ static int parse_repository_folders(git_repository *repo, const char *repository /* objects database */ strcpy(path_aux + path_len, "objects/"); if (gitfo_isdir(path_aux) < 0) - return GIT_ERROR; + return GIT_ENOTAREPO; repo->path_odb = git__strdup(path_aux); /* HEAD file */ strcpy(path_aux + path_len, "HEAD"); if (gitfo_exists(path_aux) < 0) - return GIT_ERROR; + return GIT_ENOTAREPO; i = path_len - 2; while (path_aux[i] != '/') @@ -109,7 +109,7 @@ static int parse_repository_folders(git_repository *repo, const char *repository /* index file */ strcpy(path_aux + path_len, "index"); if (gitfo_exists(path_aux) < 0) - return GIT_ERROR; + return GIT_ENOTAREPO; repo->path_index = git__strdup(path_aux); } else { @@ -141,21 +141,31 @@ git_repository *git_repository__alloc() return repo; } -git_repository *git_repository_open(const char *path) +int git_repository_open(git_repository **repo_out, const char *path) { git_repository *repo; + int error = GIT_SUCCESS; + + assert(repo_out && path); repo = git_repository__alloc(); if (repo == NULL) - return NULL; + return GIT_ENOMEM; - if (parse_repository_folders(repo, path) < 0 || - git_odb_open(&repo->db, repo->path_odb) < 0) { - git_repository_free(repo); - return NULL; - } + error = parse_repository_folders(repo, path); + if (error < 0) + goto cleanup; - return repo; + error = git_odb_open(&repo->db, repo->path_odb); + if (error < 0) + goto cleanup; + + *repo_out = repo; + return GIT_SUCCESS; + +cleanup: + git_repository_free(repo); + return error; } void git_repository_free(git_repository *repo) @@ -163,6 +173,8 @@ void git_repository_free(git_repository *repo) git_hashtable_iterator it; git_object *object; + assert(repo); + free(repo->path_workdir); free(repo->path_index); free(repo->path_repository); @@ -183,7 +195,9 @@ void git_repository_free(git_repository *repo) git_index *git_repository_index(git_repository *repo) { if (repo->index == NULL) { - repo->index = git_index_alloc(repo->path_index, repo->path_workdir); + if (git_index_open(&repo->index, repo->path_index, repo->path_workdir) < 0) + return NULL; + assert(repo->index && repo->index->on_disk); } @@ -216,8 +230,7 @@ int git__source_printf(git_odb_source *source, const char *format, ...) va_list arglist; int len, did_resize = 0; - if (!source->open || source->write_ptr == NULL) - return GIT_ERROR; + assert(source->open && source->write_ptr); va_start(arglist, format); @@ -243,8 +256,7 @@ int git__source_write(git_odb_source *source, const void *bytes, size_t len) { assert(source); - if (!source->open || source->write_ptr == NULL) - return GIT_ERROR; + assert(source->open && source->write_ptr); while (source->written_bytes + len >= source->raw.len) { if (source_resize(source) < 0) @@ -426,10 +438,14 @@ git_repository *git_object_owner(git_object *obj) return obj->repo; } -git_object *git_object_new(git_repository *repo, git_otype type) +int git_repository_newobject(git_object **object_out, git_repository *repo, git_otype type) { git_object *object = NULL; + assert(object_out && repo); + + *object_out = NULL; + switch (type) { case GIT_OBJ_COMMIT: case GIT_OBJ_TAG: @@ -438,13 +454,13 @@ git_object *git_object_new(git_repository *repo, git_otype type) break; default: - return NULL; + return GIT_EINVALIDTYPE; } object = git__malloc(object_sizes[type]); if (object == NULL) - return NULL; + return GIT_ENOMEM; memset(object, 0x0, object_sizes[type]); object->repo = repo; @@ -453,33 +469,37 @@ git_object *git_object_new(git_repository *repo, git_otype type) object->source.raw.type = type; - return object; + *object_out = object; + return GIT_SUCCESS; } -git_object *git_repository_lookup(git_repository *repo, const git_oid *id, git_otype type) +int git_repository_lookup(git_object **object_out, git_repository *repo, const git_oid *id, git_otype type) { git_object *object = NULL; git_rawobj obj_file; int error = 0; - assert(repo); + assert(repo && object_out && id); object = git_hashtable_lookup(repo->objects, id); - if (object != NULL) - return object; + if (object != NULL) { + *object_out = object; + return GIT_SUCCESS; + } - if (git_odb_read(&obj_file, repo->db, id) < 0) - return NULL; + error = git_odb_read(&obj_file, repo->db, id); + if (error < 0) + return error; if (type != GIT_OBJ_ANY && type != obj_file.type) - return NULL; + return GIT_EINVALIDTYPE; type = obj_file.type; object = git__malloc(object_sizes[type]); if (object == NULL) - return NULL; + return GIT_ENOMEM; memset(object, 0x0, object_sizes[type]); @@ -511,10 +531,24 @@ git_object *git_repository_lookup(git_repository *repo, const git_oid *id, git_o if (error < 0) { git_object_free(object); - return NULL; + return error; } git_object__source_close(object); git_hashtable_insert(repo->objects, &object->id, object); - return object; + + *object_out = object; + return GIT_SUCCESS; } + +#define GIT_NEWOBJECT_TEMPLATE(obj, tp) \ + int git_##obj##_new(git_##obj **o, git_repository *repo) {\ + return git_repository_newobject((git_object **)o, repo, GIT_OBJ_##tp); } \ + int git_##obj##_lookup(git_##obj **o, git_repository *repo, const git_oid *id) { \ + return git_repository_lookup((git_object **)o, repo, id, GIT_OBJ_##tp); } + +GIT_NEWOBJECT_TEMPLATE(commit, COMMIT) +GIT_NEWOBJECT_TEMPLATE(tag, TAG) +GIT_NEWOBJECT_TEMPLATE(tree, TREE) + + diff --git a/src/revwalk.c b/src/revwalk.c index 8cd444fb2..eda45f04b 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -28,7 +28,7 @@ #include "revwalk.h" #include "hashtable.h" -uint32_t git_revwalk_commit_hash(const void *key) +uint32_t git_revwalk__commit_hash(const void *key) { uint32_t r; git_commit *commit; @@ -38,7 +38,7 @@ uint32_t git_revwalk_commit_hash(const void *key) return r; } -int git_revwalk_commit_haskey(void *object, const void *key) +int git_revwalk__commit_haskey(void *object, const void *key) { git_revwalk_commit *walk_commit; git_commit *commit_object; @@ -50,27 +50,29 @@ int git_revwalk_commit_haskey(void *object, const void *key) } -git_revwalk *git_revwalk_alloc(git_repository *repo) +int git_revwalk_new(git_revwalk **revwalk_out, git_repository *repo) { git_revwalk *walk; walk = git__malloc(sizeof(git_revwalk)); if (walk == NULL) - return NULL; + return GIT_ENOMEM; memset(walk, 0x0, sizeof(git_revwalk)); walk->commits = git_hashtable_alloc(64, - git_revwalk_commit_hash, - git_revwalk_commit_haskey); + git_revwalk__commit_hash, + git_revwalk__commit_haskey); if (walk->commits == NULL) { free(walk); - return NULL; + return GIT_ENOMEM; } walk->repo = repo; - return walk; + + *revwalk_out = walk; + return GIT_SUCCESS; } void git_revwalk_free(git_revwalk *walk) @@ -86,13 +88,14 @@ git_repository *git_revwalk_repository(git_revwalk *walk) return walk->repo; } -void git_revwalk_sorting(git_revwalk *walk, unsigned int sort_mode) +int git_revwalk_sorting(git_revwalk *walk, unsigned int sort_mode) { if (walk->walking) - return; + return GIT_EBUSY; walk->sorting = sort_mode; git_revwalk_reset(walk); + return GIT_SUCCESS; } static git_revwalk_commit *commit_to_walkcommit(git_revwalk *walk, git_commit *commit_object) @@ -159,15 +162,14 @@ static git_revwalk_commit *insert_commit(git_revwalk *walk, git_commit *commit_o int git_revwalk_push(git_revwalk *walk, git_commit *commit) { - return insert_commit(walk, commit) ? GIT_SUCCESS : GIT_ERROR; + return insert_commit(walk, commit) ? GIT_SUCCESS : GIT_ENOMEM; } static void mark_uninteresting(git_revwalk_commit *commit) { git_revwalk_listnode *parent; - if (commit == NULL) - return; + assert(commit); commit->uninteresting = 1; parent = commit->parents.head; @@ -184,7 +186,7 @@ int git_revwalk_hide(git_revwalk *walk, git_commit *commit) hide = insert_commit(walk, commit); if (hide == NULL) - return GIT_ERROR; + return GIT_ENOMEM; mark_uninteresting(hide); return GIT_SUCCESS; diff --git a/src/tag.c b/src/tag.c index d778bcc46..fe71f7b94 100644 --- a/src/tag.c +++ b/src/tag.c @@ -39,11 +39,6 @@ void git_tag__free(git_tag *tag) free(tag); } -git_tag *git_tag_new(git_repository *repo) -{ - return (git_tag *)git_object_new(repo, GIT_OBJ_TAG); -} - const git_oid *git_tag_id(git_tag *c) { return git_object_id((git_object *)c); @@ -138,9 +133,10 @@ static int parse_tag_buffer(git_tag *tag, char *buffer, const char *buffer_end) git_oid target_oid; unsigned int i, text_len; char *search; + int error; - if (git__parse_oid(&target_oid, &buffer, buffer_end, "object ") < 0) - return GIT_EOBJCORRUPTED; + if ((error = git__parse_oid(&target_oid, &buffer, buffer_end, "object ")) < 0) + return error; if (buffer + 5 >= buffer_end) return GIT_EOBJCORRUPTED; @@ -167,12 +163,9 @@ static int parse_tag_buffer(git_tag *tag, char *buffer, const char *buffer_end) if (tag->type == GIT_OBJ_BAD) return GIT_EOBJCORRUPTED; - /* Lookup the tagged object once we know its type */ - tag->target = - git_repository_lookup(tag->object.repo, &target_oid, tag->type); - - if (tag->target == NULL) - return GIT_EOBJCORRUPTED; + error = git_repository_lookup(&tag->target, tag->object.repo, &target_oid, tag->type); + if (error < 0) + return error; if (buffer + 4 >= buffer_end) return GIT_EOBJCORRUPTED; @@ -201,8 +194,8 @@ static int parse_tag_buffer(git_tag *tag, char *buffer, const char *buffer_end) tag->tagger = git__malloc(sizeof(git_person)); - if (git_person__parse(tag->tagger, &buffer, buffer_end, "tagger ") != 0) - return GIT_EOBJCORRUPTED; + if ((error = git_person__parse(tag->tagger, &buffer, buffer_end, "tagger ")) != 0) + return error; text_len = buffer_end - ++buffer; @@ -219,7 +212,7 @@ static int parse_tag_buffer(git_tag *tag, char *buffer, const char *buffer_end) int git_tag__writeback(git_tag *tag, git_odb_source *src) { if (tag->target == NULL || tag->tag_name == NULL || tag->tagger == NULL) - return GIT_ERROR; + return GIT_EMISSINGOBJDATA; git__write_oid(src, "object", git_object_id(tag->target)); git__source_printf(src, "type %s\n", git_obj_type_to_string(tag->type)); @@ -239,7 +232,3 @@ int git_tag__parse(git_tag *tag) return parse_tag_buffer(tag, tag->object.source.raw.data, tag->object.source.raw.data + tag->object.source.raw.len); } -git_tag *git_tag_lookup(git_repository *repo, const git_oid *id) -{ - return (git_tag *)git_repository_lookup(repo, id, GIT_OBJ_TAG); -} diff --git a/src/tree.c b/src/tree.c index 526f9dc31..9dd407d7e 100644 --- a/src/tree.c +++ b/src/tree.c @@ -93,21 +93,11 @@ void git_tree__free(git_tree *tree) free(tree); } -git_tree *git_tree_new(git_repository *repo) -{ - return (git_tree *)git_object_new(repo, GIT_OBJ_TREE); -} - const git_oid *git_tree_id(git_tree *c) { return git_object_id((git_object *)c); } -git_tree *git_tree_lookup(git_repository *repo, const git_oid *id) -{ - return (git_tree *)git_repository_lookup(repo, id, GIT_OBJ_TREE); -} - void git_tree_entry_set_attributes(git_tree_entry *entry, int attr) { assert(entry && entry->owner); @@ -151,10 +141,10 @@ const git_oid *git_tree_entry_id(git_tree_entry *entry) return &entry->oid; } -git_object *git_tree_entry_2object(git_tree_entry *entry) +int git_tree_entry_2object(git_object **object_out, git_tree_entry *entry) { - assert(entry); - return git_repository_lookup(entry->owner->object.repo, &entry->oid, GIT_OBJ_ANY); + assert(entry && object_out); + return git_repository_lookup(object_out, entry->owner->object.repo, &entry->oid, GIT_OBJ_ANY); } git_tree_entry *git_tree_entry_byname(git_tree *tree, const char *filename) @@ -253,7 +243,7 @@ int git_tree__writeback(git_tree *tree, git_odb_source *src) assert(tree && src); if (tree->entries == NULL) - return GIT_ERROR; + return GIT_EMISSINGOBJDATA; entry_resort(tree); diff --git a/tests/t0401-parse.c b/tests/t0401-parse.c index 1a2806bf1..0ce59fc30 100644 --- a/tests/t0401-parse.c +++ b/tests/t0401-parse.c @@ -224,8 +224,7 @@ BEGIN_TEST(parse_buffer_test) int i; git_repository *repo; - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); for (i = 0; i < broken_commit_count; ++i) { git_commit *commit; diff --git a/tests/t0402-details.c b/tests/t0402-details.c index d60b6481b..22d1f8eba 100644 --- a/tests/t0402-details.c +++ b/tests/t0402-details.c @@ -22,8 +22,7 @@ BEGIN_TEST(query_details_test) unsigned int i; git_repository *repo; - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); for (i = 0; i < commit_count; ++i) { git_oid id; @@ -35,8 +34,7 @@ BEGIN_TEST(query_details_test) git_oid_mkstr(&id, commit_ids[i]); - commit = git_commit_lookup(repo, &id); - must_be_true(commit != NULL); + must_pass(git_commit_lookup(&commit, repo, &id)); message = git_commit_message(commit); message_short = git_commit_message_short(commit); diff --git a/tests/t0403-write.c b/tests/t0403-write.c index 60133c409..4bd659730 100644 --- a/tests/t0403-write.c +++ b/tests/t0403-write.c @@ -26,17 +26,14 @@ BEGIN_TEST(writenew_test) git_oid id; /* char hex_oid[41]; */ - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); /* Create commit in memory */ - commit = git_commit_new(repo); - must_be_true(commit != NULL); + must_pass(git_commit_new(&commit, repo)); /* Add new parent */ git_oid_mkstr(&id, commit_ids[4]); - parent = git_commit_lookup(repo, &id); - must_be_true(parent != NULL); + must_pass(git_commit_lookup(&parent, repo, &id)); git_commit_add_parent(commit, parent); @@ -49,8 +46,7 @@ This is a commit created in memory and it will be written back to disk\n"); /* add new tree */ git_oid_mkstr(&id, tree_oid); - tree = git_tree_lookup(repo, &id); - must_be_true(tree != NULL); + must_pass(git_tree_lookup(&tree, repo, &id)); git_commit_set_tree(commit, tree); @@ -82,13 +78,11 @@ BEGIN_TEST(writeback_test) const char *message; /* char hex_oid[41]; */ - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); git_oid_mkstr(&id, commit_ids[0]); - commit = git_commit_lookup(repo, &id); - must_be_true(commit != NULL); + must_pass(git_commit_lookup(&commit, repo, &id)); message = git_commit_message(commit); @@ -101,8 +95,7 @@ BEGIN_TEST(writeback_test) git_commit_set_message(commit, "This is a new test message. Cool!\n"); git_oid_mkstr(&id, commit_ids[4]); - parent = git_commit_lookup(repo, &id); - must_be_true(parent != NULL); + must_pass(git_commit_lookup(&parent, repo, &id)); git_commit_add_parent(commit, parent); diff --git a/tests/t0501-walk.c b/tests/t0501-walk.c index 55ae936d2..8dd7990d4 100644 --- a/tests/t0501-walk.c +++ b/tests/t0501-walk.c @@ -95,17 +95,13 @@ BEGIN_TEST(simple_walk_test) git_revwalk *walk; git_commit *head = NULL; - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); - walk = git_revwalk_alloc(repo); - must_be_true(walk != NULL); + must_pass(git_revwalk_new(&walk, repo)); git_oid_mkstr(&id, commit_head); - head = git_commit_lookup(repo, &id); - must_be_true(head != NULL); - + must_pass(git_commit_lookup(&head, repo, &id)); must_pass(test_walk(walk, head, GIT_SORT_TIME, diff --git a/tests/t0601-read.c b/tests/t0601-read.c index 345e5b534..133a26679 100644 --- a/tests/t0601-read.c +++ b/tests/t0601-read.c @@ -29,8 +29,7 @@ struct test_entry TEST_ENTRIES[] = { BEGIN_TEST(index_loadempty_test) git_index *index; - index = git_index_alloc("in-memory-index", NULL); - must_be_true(index != NULL); + must_pass(git_index_open(&index, "in-memory-index", NULL)); must_be_true(index->on_disk == 0); must_pass(git_index_read(index)); @@ -46,8 +45,7 @@ BEGIN_TEST(index_load_test) git_index *index; unsigned int i; - index = git_index_alloc(TEST_INDEX_PATH, NULL); - must_be_true(index != NULL); + must_pass(git_index_open(&index, TEST_INDEX_PATH, NULL)); must_be_true(index->on_disk); must_pass(git_index_read(index)); @@ -70,8 +68,7 @@ END_TEST BEGIN_TEST(index2_load_test) git_index *index; - index = git_index_alloc(TEST_INDEX2_PATH, NULL); - must_be_true(index != NULL); + must_pass(git_index_open(&index, TEST_INDEX2_PATH, NULL)); must_be_true(index->on_disk); must_pass(git_index_read(index)); @@ -88,8 +85,7 @@ BEGIN_TEST(index_find_test) git_index *index; unsigned int i; - index = git_index_alloc(TEST_INDEX_PATH, NULL); - must_be_true(index != NULL); + must_pass(git_index_open(&index, TEST_INDEX_PATH, NULL)); must_pass(git_index_read(index)); for (i = 0; i < ARRAY_SIZE(TEST_ENTRIES); ++i) { @@ -104,8 +100,7 @@ BEGIN_TEST(index_findempty_test) git_index *index; unsigned int i; - index = git_index_alloc("fake-index", NULL); - must_be_true(index != NULL); + must_pass(git_index_open(&index, "fake-index", NULL)); for (i = 0; i < ARRAY_SIZE(TEST_ENTRIES); ++i) { int idx = git_index_find(index, TEST_ENTRIES[i].path); diff --git a/tests/t0602-write.c b/tests/t0602-write.c index d232193b7..415183f94 100644 --- a/tests/t0602-write.c +++ b/tests/t0602-write.c @@ -35,8 +35,7 @@ BEGIN_TEST(index_load_test) git_index *index; git_filelock out_file; - index = git_index_alloc(TEST_INDEX_PATH, NULL); - must_be_true(index != NULL); + must_pass(git_index_open(&index, TEST_INDEX_PATH, NULL)); must_pass(git_index_read(index)); must_be_true(index->on_disk); diff --git a/tests/t0603-sort.c b/tests/t0603-sort.c index dfbb6e175..0ee14d713 100644 --- a/tests/t0603-sort.c +++ b/tests/t0603-sort.c @@ -36,8 +36,7 @@ BEGIN_TEST(index_sort_test) git_index *index; unsigned int i; - index = git_index_alloc(TEST_INDEX_PATH, NULL); - must_be_true(index != NULL); + must_pass(git_index_open(&index, TEST_INDEX_PATH, NULL)); must_pass(git_index_read(index)); randomize_entries(index); @@ -54,8 +53,8 @@ END_TEST BEGIN_TEST(index_sort_empty_test) git_index *index; - index = git_index_alloc("fake-index", NULL); - must_be_true(index != NULL); + + must_pass(git_index_open(&index, "fake-index", NULL)); git_index__sort(index); must_be_true(index->sorted); diff --git a/tests/t0801-readtag.c b/tests/t0801-readtag.c index 798fa8f49..8314304bd 100644 --- a/tests/t0801-readtag.c +++ b/tests/t0801-readtag.c @@ -16,15 +16,13 @@ BEGIN_TEST(readtag) git_commit *commit; git_oid id1, id2, id_commit; - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); git_oid_mkstr(&id1, tag1_id); git_oid_mkstr(&id2, tag2_id); git_oid_mkstr(&id_commit, tagged_commit); - tag1 = git_tag_lookup(repo, &id1); - must_be_true(tag1 != NULL); + must_pass(git_tag_lookup(&tag1, repo, &id1)); must_be_true(strcmp(git_tag_name(tag1), "test") == 0); must_be_true(git_tag_type(tag1) == GIT_OBJ_TAG); diff --git a/tests/t0802-write.c b/tests/t0802-write.c index 20c1eab0a..8dd8de5ea 100644 --- a/tests/t0802-write.c +++ b/tests/t0802-write.c @@ -14,13 +14,11 @@ BEGIN_TEST(tag_writeback_test) git_tag *tag; /* char hex_oid[41]; */ - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); git_oid_mkstr(&id, tag_id); - tag = git_tag_lookup(repo, &id); - must_be_true(tag != NULL); + must_pass(git_tag_lookup(&tag, repo, &id)); git_tag_set_name(tag, "This is a different tag LOL"); diff --git a/tests/t0901-readtree.c b/tests/t0901-readtree.c index e3a85c52d..c14f5fd1f 100644 --- a/tests/t0901-readtree.c +++ b/tests/t0901-readtree.c @@ -13,13 +13,11 @@ BEGIN_TEST(tree_entry_access_test) git_repository *repo; git_tree *tree; - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); git_oid_mkstr(&id, tree_oid); - tree = git_tree_lookup(repo, &id); - must_be_true(tree != NULL); + must_pass(git_tree_lookup(&tree, repo, &id)); must_be_true(git_tree_entry_byname(tree, "README") != NULL); must_be_true(git_tree_entry_byname(tree, "NOTEXISTS") == NULL); @@ -37,14 +35,13 @@ BEGIN_TEST(tree_read_test) git_repository *repo; git_tree *tree; git_tree_entry *entry; + git_object *obj; - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); git_oid_mkstr(&id, tree_oid); - tree = git_tree_lookup(repo, &id); - must_be_true(tree != NULL); + must_pass(git_tree_lookup(&tree, repo, &id)); must_be_true(git_tree_entrycount(tree) == 3); @@ -53,7 +50,7 @@ BEGIN_TEST(tree_read_test) must_be_true(strcmp(git_tree_entry_name(entry), "README") == 0); - must_be_true(git_tree_entry_2object(entry) != NULL); + must_pass(git_tree_entry_2object(&obj, entry)); git_repository_free(repo); END_TEST diff --git a/tests/t0902-modify.c b/tests/t0902-modify.c index 23fc03cb2..a83349df9 100644 --- a/tests/t0902-modify.c +++ b/tests/t0902-modify.c @@ -16,11 +16,8 @@ BEGIN_TEST(tree_in_memory_add_test) unsigned int i; git_oid entry_id; - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); - - tree = git_tree_new(repo); - must_be_true(tree != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); + must_pass(git_tree_new(&tree, repo)); git_oid_mkstr(&entry_id, tree_oid); for (i = 0; i < entry_count; ++i) { @@ -46,13 +43,11 @@ BEGIN_TEST(tree_add_entry_test) unsigned int i; /* char hex_oid[41]; */ - repo = git_repository_open(REPOSITORY_FOLDER); - must_be_true(repo != NULL); + must_pass(git_repository_open(&repo, REPOSITORY_FOLDER)); git_oid_mkstr(&id, tree_oid); - tree = git_tree_lookup(repo, &id); - must_be_true(tree != NULL); + must_pass(git_tree_lookup(&tree, repo, &id)); must_be_true(git_tree_entrycount(tree) == 3);