diff options
author | Vicent Martà <tanoku@gmail.com> | 2012-02-17 00:13:34 +0100 |
---|---|---|
committer | Vicent Martà <tanoku@gmail.com> | 2012-03-06 00:43:10 +0100 |
commit | 1a48112342932e9fcd45a1ff5935f1c9c53b83d1 (patch) | |
tree | fbb18cfe64e65025c6e1790972d1a106eea4cc54 | |
parent | 45d387ac78bcf3167d69b736d0b322717bc492d4 (diff) | |
download | libgit2-1a48112342932e9fcd45a1ff5935f1c9c53b83d1.tar.gz |
error-handling: References
Yes, this is error handling solely for `refs.c`, but some of the
abstractions leak all ofer the code base.
-rw-r--r-- | include/git2/errors.h | 11 | ||||
-rw-r--r-- | src/attr_file.c | 5 | ||||
-rw-r--r-- | src/cc-compat.h | 8 | ||||
-rw-r--r-- | src/errors.c | 54 | ||||
-rw-r--r-- | src/filebuf.c | 75 | ||||
-rw-r--r-- | src/fileops.c | 73 | ||||
-rw-r--r-- | src/global.h | 3 | ||||
-rw-r--r-- | src/index.c | 4 | ||||
-rw-r--r-- | src/odb.c | 2 | ||||
-rw-r--r-- | src/odb_loose.c | 9 | ||||
-rw-r--r-- | src/odb_pack.c | 2 | ||||
-rw-r--r-- | src/pack.c | 2 | ||||
-rw-r--r-- | src/path.c | 74 | ||||
-rw-r--r-- | src/path.h | 24 | ||||
-rw-r--r-- | src/reflog.c | 6 | ||||
-rw-r--r-- | src/refs.c | 808 | ||||
-rw-r--r-- | src/repository.c | 14 | ||||
-rw-r--r-- | src/status.c | 8 | ||||
-rw-r--r-- | tests-clar/config/stress.c | 2 | ||||
-rw-r--r-- | tests-clar/core/filebuf.c | 16 | ||||
-rw-r--r-- | tests/t00-core.c | 2 | ||||
-rw-r--r-- | tests/t03-objwrite.c | 4 | ||||
-rw-r--r-- | tests/t08-tag.c | 2 | ||||
-rw-r--r-- | tests/t10-refs.c | 34 | ||||
-rw-r--r-- | tests/t12-repo.c | 2 | ||||
-rw-r--r-- | tests/test_helpers.c | 10 |
26 files changed, 590 insertions, 664 deletions
diff --git a/include/git2/errors.h b/include/git2/errors.h index e3f5f4cb0..9b28093dc 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -122,16 +122,17 @@ typedef struct { typedef enum { GITERR_NOMEMORY, + GITERR_OS, GITERR_REFERENCE, + GITERR_ZLIB, } git_error_class; -#define GITERR_CHECK_ALLOC(ptr, error) if (ptr == NULL) { giterr_set_oom(error); return -1 } +#define GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { return -1; } -GIT_EXTERN(void) giterr_set(git_error **error_out, int error_class, const char *string, ...); -GIT_EXTERN(void) giterr_set_oom(git_error **error); -GIT_EXTERN(void) giterr_free(git_error *error); -GIT_EXTERN(void) giterr_clear(git_error **error); +GIT_EXTERN(void) giterr_set_oom(void); +GIT_EXTERN(void) giterr_set(int error_class, const char *string, ...); +GIT_EXTERN(void) giterr_clear(void); /** * Return a detailed error string with the latest error diff --git a/src/attr_file.c b/src/attr_file.c index 3783b5ef3..48424123a 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -250,11 +250,12 @@ int git_attr_path__init( git_buf full_path = GIT_BUF_INIT; int error = git_buf_joinpath(&full_path, base, path); if (error == GIT_SUCCESS) - info->is_dir = (git_path_isdir(full_path.ptr) == GIT_SUCCESS); + info->is_dir = (int)git_path_isdir(full_path.ptr); + git_buf_free(&full_path); return error; } - info->is_dir = (git_path_isdir(path) == GIT_SUCCESS); + info->is_dir = (int)git_path_isdir(path); return GIT_SUCCESS; } diff --git a/src/cc-compat.h b/src/cc-compat.h index 3df36b61f..507985daa 100644 --- a/src/cc-compat.h +++ b/src/cc-compat.h @@ -50,4 +50,12 @@ # pragma warning ( disable : 4127 ) #endif +#if defined (_MSC_VER) + typedef unsigned char bool; +# define true 1 +# define false 0 +#else +# include <stdbool.h> +#endif + #endif /* INCLUDE_compat_h__ */ diff --git a/src/errors.c b/src/errors.c index 0105c2538..548e44a32 100644 --- a/src/errors.c +++ b/src/errors.c @@ -108,20 +108,24 @@ void git_clearerror(void) * New error handling ********************************************/ -void giterr_set(git_error **error_out, int error_class, const char *string, ...) +static git_error g_git_oom_error = { + "Out of memory", + GITERR_NOMEMORY +}; + +void giterr_set_oom(void) +{ + GIT_GLOBAL->last_error = &g_git_oom_error; +} + +void giterr_set(int error_class, const char *string, ...) { char error_str[1024]; va_list arglist; git_error *error; - if (error_out == NULL) - return; - - error = git__malloc(sizeof(git_error)); - if (!error) { - giterr_set_oom(error_out); - return; - } + error = &GIT_GLOBAL->error_t; + free(error->message); va_start(arglist, string); p_vsnprintf(error_str, sizeof(error_str), string, arglist); @@ -131,38 +135,14 @@ void giterr_set(git_error **error_out, int error_class, const char *string, ...) error->klass = error_class; if (error->message == NULL) { - free(error); - giterr_set_oom(error_out); + giterr_set_oom(); return; } - *error_out = error; -} - -static git_error g_git_oom_error = { - "Out of memory", - GITERR_NOMEMORY -}; - -void giterr_set_oom(git_error **error) -{ - if (error != NULL) - *error = &g_git_oom_error; -} - -void giterr_free(git_error *error) -{ - if (error == NULL || error == &g_git_oom_error) - return; - - free(error->message); - free(error); + GIT_GLOBAL->last_error = error; } -void giterr_clear(git_error **error) +void giterr_clear(void) { - if (error != NULL) { - giterr_free(*error); - *error = NULL; - } + GIT_GLOBAL->last_error = NULL; } diff --git a/src/filebuf.c b/src/filebuf.c index 01df8e2d0..e6e68014a 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -16,11 +16,14 @@ static const size_t WRITE_BUFFER_SIZE = (4096 * 2); static int lock_file(git_filebuf *file, int flags) { - if (git_path_exists(file->path_lock) == 0) { + if (git_path_exists(file->path_lock) == true) { if (flags & GIT_FILEBUF_FORCE) p_unlink(file->path_lock); - else - return git__throw(GIT_EOSERR, "Failed to lock file"); + else { + giterr_set(GITERR_OS, + "Failed to lock file '%s' for writing", file->path_lock); + return -1; + } } /* create path to the file buffer is required */ @@ -32,16 +35,20 @@ static int lock_file(git_filebuf *file, int flags) } if (file->fd < 0) - return git__throw(GIT_EOSERR, "Failed to create lock"); + return -1; - if ((flags & GIT_FILEBUF_APPEND) && git_path_exists(file->path_original) == 0) { + if ((flags & GIT_FILEBUF_APPEND) && git_path_exists(file->path_original) == true) { git_file source; char buffer[2048]; size_t read_bytes; source = p_open(file->path_original, O_RDONLY); - if (source < 0) - return git__throw(GIT_EOSERR, "Failed to lock file. Could not open %s", file->path_original); + if (source < 0) { + giterr_set(GITERR_OS, + "Failed to open file '%s' for reading: %s", + file->path_original, strerror(errno)); + return -1; + } while ((read_bytes = p_read(source, buffer, 2048)) > 0) { p_write(file->fd, buffer, read_bytes); @@ -60,7 +67,7 @@ void git_filebuf_cleanup(git_filebuf *file) if (file->fd >= 0) p_close(file->fd); - if (file->fd >= 0 && file->path_lock && git_path_exists(file->path_lock) == GIT_SUCCESS) + if (file->fd >= 0 && file->path_lock && git_path_exists(file->path_lock) == true) p_unlink(file->path_lock); if (file->digest) @@ -141,13 +148,13 @@ static int write_deflate(git_filebuf *file, void *source, size_t len) int git_filebuf_open(git_filebuf *file, const char *path, int flags) { - int error, compression; + int compression; size_t path_len; - assert(file && path); - - if (file->buffer) - return git__throw(GIT_EINVALIDARGS, "Tried to reopen an open filebuf"); + /* opening an already open buffer is a programming error; + * assert that this never happens instead of returning + * an error code */ + assert(file && path && file->buffer == NULL); memset(file, 0x0, sizeof(git_filebuf)); @@ -157,17 +164,12 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags) /* Allocate the main cache buffer */ file->buffer = git__malloc(file->buf_size); - if (file->buffer == NULL){ - error = GIT_ENOMEM; - goto cleanup; - } + GITERR_CHECK_ALLOC(file->buffer); /* If we are hashing on-write, allocate a new hash context */ if (flags & GIT_FILEBUF_HASH_CONTENTS) { - if ((file->digest = git_hash_new_ctx()) == NULL) { - error = GIT_ENOMEM; - goto cleanup; - } + file->digest = git_hash_new_ctx(); + GITERR_CHECK_ALLOC(file->digest); } compression = flags >> GIT_FILEBUF_DEFLATE_SHIFT; @@ -176,16 +178,13 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags) if (compression != 0) { /* Initialize the ZLib stream */ if (deflateInit(&file->zs, compression) != Z_OK) { - error = git__throw(GIT_EZLIB, "Failed to initialize zlib"); + giterr_set(GITERR_ZLIB, "Failed to initialize zlib"); goto cleanup; } /* Allocate the Zlib cache buffer */ file->z_buf = git__malloc(file->buf_size); - if (file->z_buf == NULL){ - error = GIT_ENOMEM; - goto cleanup; - } + GITERR_CHECK_ALLOC(file->z_buf); /* Never flush */ file->flush_mode = Z_NO_FLUSH; @@ -200,50 +199,40 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags) /* Open the file as temporary for locking */ file->fd = git_futils_mktmp(&tmp_path, path); + if (file->fd < 0) { git_buf_free(&tmp_path); - error = GIT_EOSERR; goto cleanup; } /* No original path */ file->path_original = NULL; file->path_lock = git_buf_detach(&tmp_path); - - if (file->path_lock == NULL) { - error = GIT_ENOMEM; - goto cleanup; - } + GITERR_CHECK_ALLOC(file->path_lock); } else { path_len = strlen(path); /* Save the original path of the file */ file->path_original = git__strdup(path); - if (file->path_original == NULL) { - error = GIT_ENOMEM; - goto cleanup; - } + GITERR_CHECK_ALLOC(file->path_original); /* create the locking path by appending ".lock" to the original */ file->path_lock = git__malloc(path_len + GIT_FILELOCK_EXTLENGTH); - if (file->path_lock == NULL) { - error = GIT_ENOMEM; - goto cleanup; - } + GITERR_CHECK_ALLOC(file->path_lock); memcpy(file->path_lock, file->path_original, path_len); memcpy(file->path_lock + path_len, GIT_FILELOCK_EXTENSION, GIT_FILELOCK_EXTLENGTH); /* open the file for locking */ - if ((error = lock_file(file, flags)) < GIT_SUCCESS) + if (lock_file(file, flags) < 0) goto cleanup; } - return GIT_SUCCESS; + return 0; cleanup: git_filebuf_cleanup(file); - return git__rethrow(error, "Failed to open file buffer for '%s'", path); + return -1; } int git_filebuf_hash(git_oid *oid, git_filebuf *file) diff --git a/src/fileops.c b/src/fileops.c index 856823afb..ffaf8319d 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -10,25 +10,19 @@ int git_futils_mkpath2file(const char *file_path, const mode_t mode) { - int error; + int result = 0; git_buf target_folder = GIT_BUF_INIT; - error = git_path_dirname_r(&target_folder, file_path); - if (error < GIT_SUCCESS) { - git_buf_free(&target_folder); - return git__throw(GIT_EINVALIDPATH, "Failed to recursively build `%s` tree structure. Unable to parse parent folder name", file_path); - } else { - /* reset error */ - error = GIT_SUCCESS; - } + if (git_path_dirname_r(&target_folder, file_path) < 0) + return -1; /* Does the containing folder exist? */ - if (git_path_isdir(target_folder.ptr) != GIT_SUCCESS) + if (git_path_isdir(target_folder.ptr) == false) /* Let's create the tree structure */ - error = git_futils_mkdir_r(target_folder.ptr, NULL, mode); + result = git_futils_mkdir_r(target_folder.ptr, NULL, mode); git_buf_free(&target_folder); - return error; + return result; } int git_futils_mktmp(git_buf *path_out, const char *filename) @@ -39,33 +33,50 @@ int git_futils_mktmp(git_buf *path_out, const char *filename) git_buf_puts(path_out, "_git2_XXXXXX"); if (git_buf_oom(path_out)) - return git__rethrow(git_buf_lasterror(path_out), - "Failed to create temporary file for %s", filename); + return -1; - if ((fd = p_mkstemp(path_out->ptr)) < 0) - return git__throw(GIT_EOSERR, "Failed to create temporary file %s", path_out->ptr); + if ((fd = p_mkstemp(path_out->ptr)) < 0) { + giterr_set(GITERR_OS, + "Failed to create temporary file '%s': %s", path_out->ptr, strerror(errno)); + return -1; + } return fd; } int git_futils_creat_withpath(const char *path, const mode_t dirmode, const mode_t mode) { - if (git_futils_mkpath2file(path, dirmode) < GIT_SUCCESS) - return git__throw(GIT_EOSERR, "Failed to create file %s", path); + int fd; - return p_creat(path, mode); + if (git_futils_mkpath2file(path, dirmode) < 0) + return -1; + + fd = p_creat(path, mode); + if (fd < 0) { + giterr_set(GITERR_OS, + "Failed to create file '%s': %s", path, strerror(errno)); + return -1; + } + + return fd; } 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); - return fd >= 0 ? fd : git__throw(GIT_EOSERR, "Failed to create locked file. Could not open %s", path); + if (fd < 0) { + giterr_set(GITERR_OS, + "Failed to create locked file '%s': %s", path, strerror(errno)); + return -1; + } + + return fd; } int git_futils_creat_locked_withpath(const char *path, const mode_t dirmode, const mode_t mode) { - if (git_futils_mkpath2file(path, dirmode) < GIT_SUCCESS) - return git__throw(GIT_EOSERR, "Failed to create locked file %s", path); + if (git_futils_mkpath2file(path, dirmode) < 0) + return -1; return git_futils_creat_locked(path, mode); } @@ -105,12 +116,14 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime, *updated = 0; if ((fd = p_open(path, O_RDONLY)) < 0) { - return git__throw(GIT_ENOTFOUND, "Failed to read file '%s': %s", path, strerror(errno)); + giterr_set(GITERR_OS, "Failed to read file '%s': %s", path, strerror(errno)); + return (errno == ENOENT) ? GIT_ENOTFOUND : -1; } if (p_fstat(fd, &st) < 0 || S_ISDIR(st.st_mode) || !git__is_sizet(st.st_size+1)) { close(fd); - return git__throw(GIT_EOSERR, "Failed to stat file '%s'", path); + giterr_set(GITERR_OS, "Invalid regular file stat for '%s'", path); + return -1; } /* @@ -141,7 +154,9 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime, if (read_size < 0) { close(fd); - return git__throw(GIT_EOSERR, "Failed to read from FD"); + giterr_set(GITERR_OS, "Failed to read descriptor for %s: %s", + path, strerror(errno)); + return -1; } len -= read_size; @@ -218,7 +233,7 @@ int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode) pp += root_path_offset; /* On Windows, will skip the drive name (eg. C: or D:) */ while (error == GIT_SUCCESS && (sp = strchr(pp, '/')) != NULL) { - if (sp != pp && git_path_isdir(make_path.ptr) < GIT_SUCCESS) { + if (sp != pp && git_path_isdir(make_path.ptr) == false) { *sp = 0; error = p_mkdir(make_path.ptr, mode); @@ -251,7 +266,7 @@ static int _rmdir_recurs_foreach(void *opaque, git_buf *path) int error = GIT_SUCCESS; int force = *(int *)opaque; - if (git_path_isdir(path->ptr) == GIT_SUCCESS) { + if (git_path_isdir(path->ptr) == true) { error = git_path_direach(path, _rmdir_recurs_foreach, opaque); if (error < GIT_SUCCESS) return git__rethrow(error, "Failed to remove directory `%s`", path->ptr); @@ -293,7 +308,7 @@ int git_futils_find_global_file(git_buf *path, const char *filename) if ((error = git_buf_joinpath(path, home, filename)) < GIT_SUCCESS) return error; - if (git_path_exists(path->ptr) < GIT_SUCCESS) { + if (git_path_exists(path->ptr) == false) { git_buf_clear(path); return GIT_ENOTFOUND; } @@ -395,7 +410,7 @@ int git_futils_find_system_file(git_buf *path, const char *filename) if (git_buf_joinpath(path, "/etc", filename) < GIT_SUCCESS) return git_buf_lasterror(path); - if (git_path_exists(path->ptr) == GIT_SUCCESS) + if (git_path_exists(path->ptr) == true) return GIT_SUCCESS; git_buf_clear(path); diff --git a/src/global.h b/src/global.h index 0c1e3289c..2b525ce07 100644 --- a/src/global.h +++ b/src/global.h @@ -14,6 +14,9 @@ typedef struct { char last[1024]; } error; + git_error *last_error; + git_error error_t; + git_mwindow_ctl mem_ctl; } git_global_st; diff --git a/src/index.c b/src/index.c index 5ac99de3e..b646dfcbb 100644 --- a/src/index.c +++ b/src/index.c @@ -150,7 +150,7 @@ int git_index_open(git_index **index_out, const char *index_path) git_vector_init(&index->entries, 32, index_cmp); /* Check if index file is stored on disk already */ - if (git_path_exists(index->index_file_path) == 0) + if (git_path_exists(index->index_file_path) == true) index->on_disk = 1; *index_out = index; @@ -221,7 +221,7 @@ int git_index_read(git_index *index) assert(index->index_file_path); - if (!index->on_disk || git_path_exists(index->index_file_path) < 0) { + if (!index->on_disk || git_path_exists(index->index_file_path) == false) { git_index_clear(index); index->on_disk = 0; return GIT_SUCCESS; @@ -402,7 +402,7 @@ static int load_alternates(git_odb *odb, const char *objects_dir) if (error < GIT_SUCCESS) return error; - if (git_path_exists(alternates_path.ptr) < GIT_SUCCESS) { + if (git_path_exists(alternates_path.ptr) == false) { git_buf_free(&alternates_path); return GIT_SUCCESS; } diff --git a/src/odb_loose.c b/src/odb_loose.c index f5f6e35ac..6546fa839 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -466,7 +466,7 @@ static int locate_object( int error = object_file_name(object_location, backend->objects_dir, oid); if (error == GIT_SUCCESS) - error = git_path_exists(git_buf_cstr(object_location)); + error = git_path_exists(git_buf_cstr(object_location)) ? GIT_SUCCESS : GIT_ENOTFOUND; return error; } @@ -480,7 +480,7 @@ static int fn_locate_object_short_oid(void *state, git_buf *pathbuf) { return GIT_SUCCESS; } - if (!git_path_exists(pathbuf->ptr) && git_path_isdir(pathbuf->ptr)) { + if (git_path_isdir(pathbuf->ptr) == true) { /* We are already in the directory matching the 2 first hex characters, * compare the first ncmp characters of the oids */ if (!memcmp(sstate->short_oid + 2, @@ -533,8 +533,7 @@ static int locate_object_short_oid( return git__rethrow(error, "Failed to locate object from short oid"); /* Check that directory exists */ - if (git_path_exists(object_location->ptr) || - git_path_isdir(object_location->ptr)) + if (git_path_isdir(object_location->ptr) == false) return git__throw(GIT_ENOTFOUND, "Failed to locate object from short oid. Object not found"); state.dir_len = object_location->size; @@ -716,7 +715,7 @@ static int loose_backend__stream_fwrite(git_oid *oid, git_odb_stream *_stream) * is what git does and allows us to sidestep the fact that * we're not allowed to overwrite a read-only file on Windows. */ - if (git_path_exists(final_path.ptr) == GIT_SUCCESS) { + if (git_path_exists(final_path.ptr) == true) { git_filebuf_cleanup(&stream->fbuf); goto cleanup; } diff --git a/src/odb_pack.c b/src/odb_pack.c index 249144a3a..159c88685 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -469,7 +469,7 @@ int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir) if (error < GIT_SUCCESS) goto cleanup; - if (git_path_isdir(git_buf_cstr(&path)) == GIT_SUCCESS) { + if (git_path_isdir(git_buf_cstr(&path)) == true) { backend->pack_folder = git_buf_detach(&path); backend->pack_folder_mtime = 0; } diff --git a/src/pack.c b/src/pack.c index 0d618324b..acab8734b 100644 --- a/src/pack.c +++ b/src/pack.c @@ -600,7 +600,7 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path) memcpy(p->pack_name, path, path_len); strcpy(p->pack_name + path_len, ".keep"); - if (git_path_exists(p->pack_name) == GIT_SUCCESS) + if (git_path_exists(p->pack_name) == true) p->pack_keep = 1; strcpy(p->pack_name + path_len, ".pack"); diff --git a/src/path.c b/src/path.c index c882fe387..5d35e0ef2 100644 --- a/src/path.c +++ b/src/path.c @@ -354,80 +354,75 @@ int git_path_walk_up( return error; } -int git_path_exists(const char *path) +bool git_path_exists(const char *path) { assert(path); - return p_access(path, F_OK); + return p_access(path, F_OK) == 0; } -int git_path_isdir(const char *path) +bool git_path_isdir(const char *path) { #ifdef GIT_WIN32 DWORD attr = GetFileAttributes(path); if (attr == INVALID_FILE_ATTRIBUTES) - return GIT_ERROR; + return false; - return (attr & FILE_ATTRIBUTE_DIRECTORY) ? GIT_SUCCESS : GIT_ERROR; + return (attr & FILE_ATTRIBUTE_DIRECTORY) != 0; #else struct stat st; - if (p_stat(path, &st) < GIT_SUCCESS) - return GIT_ERROR; + if (p_stat(path, &st) < 0) + return false; - return S_ISDIR(st.st_mode) ? GIT_SUCCESS : GIT_ERROR; + return S_ISDIR(st.st_mode) != 0; #endif } -int git_path_isfile(const char *path) +bool git_path_isfile(const char *path) { struct stat st; - int stat_error; assert(path); - stat_error = p_stat(path, &st); + if (p_stat(path, &st) < 0) + return false; - if (stat_error < GIT_SUCCESS) - return -1; - - if (!S_ISREG(st.st_mode)) - return -1; - - return 0; + return S_ISREG(st.st_mode) != 0; } -static int _check_dir_contents( +static bool _check_dir_contents( git_buf *dir, const char *sub, - int (*predicate)(const char *)) + bool (*predicate)(const char *)) { - int error = GIT_SUCCESS; + bool result; size_t dir_size = dir->size; size_t sub_size = strlen(sub); - /* separate allocation and join, so we can always leave git_buf valid */ - if ((error = git_buf_try_grow(dir, dir_size + sub_size + 2)) < GIT_SUCCESS) - return error; + /* leave base valid even if we could not make space for subdir */ + if (git_buf_try_grow(dir, dir_size + sub_size + 2) < 0) + return false; + + /* save excursion */ git_buf_joinpath(dir, dir->ptr, sub); - error = (*predicate)(dir->ptr); + result = predicate(dir->ptr); /* restore path */ git_buf_truncate(dir, dir_size); - - return error; + return result; } -int git_path_contains(git_buf *dir, const char *item) +bool git_path_contains(git_buf *dir, const char *item) { return _check_dir_contents(dir, item, &git_path_exists); } -int git_path_contains_dir(git_buf *base, const char *subdir) +bool git_path_contains_dir(git_buf *base, const char *subdir) { return _check_dir_contents(base, subdir, &git_path_isdir); } -int git_path_contains_file(git_buf *base, const char *file) +bool git_path_contains_file(git_buf *base, const char *file) { return _check_dir_contents(base, file, &git_path_isfile); } @@ -448,7 +443,7 @@ int git_path_find_dir(git_buf *dir, const char *path, const char *base) } /* call dirname if this is not a directory */ - if (error == GIT_SUCCESS && git_path_isdir(dir->ptr) != GIT_SUCCESS) + if (error == GIT_SUCCESS && git_path_isdir(dir->ptr) == false) if (git_path_dirname_r(dir, dir->ptr) < GIT_SUCCESS) error = git_buf_lasterror(dir); @@ -486,21 +481,20 @@ GIT_INLINE(int) is_dot_or_dotdot(const char *name) int git_path_direach( git_buf *path, - int (*fn)(void *, git_buf *, git_error **), - void *arg, - git_error **error) + int (*fn)(void *, git_buf *), + void *arg) { ssize_t wd_len; DIR *dir; struct dirent de_buf, *de; - if (git_path_to_dir(path, error) < 0) + if (git_path_to_dir(path) < 0) return -1; wd_len = path->size; dir = opendir(path->ptr); if (!dir) { - giterr_set(error, GITERR_OS, "Failed to `opendir` %s: %s", path->ptr, strerror(errno)); + giterr_set(GITERR_OS, "Failed to `opendir` %s: %s", path->ptr, strerror(errno)); return -1; } @@ -510,12 +504,10 @@ int git_path_direach( if (is_dot_or_dotdot(de->d_name)) continue; - if (git_buf_puts(path, de->d_name) < 0) { - giterr_set_oom(error); + if (git_buf_puts(path, de->d_name) < 0) return -1; - } - result = fn(arg, path, error); + result = fn(arg, path); git_buf_truncate(path, wd_len); /* restore path */ @@ -526,7 +518,7 @@ int git_path_direach( } closedir(dir); - return GIT_SUCCESS; + return 0; } int git_path_dirload( diff --git a/src/path.h b/src/path.h index 981fdd6a4..e885d875e 100644 --- a/src/path.h +++ b/src/path.h @@ -113,21 +113,21 @@ extern int git_path_fromurl(git_buf *local_path_out, const char *file_url); /** * Check if a file exists and can be accessed. - * @return GIT_SUCCESS if file exists, < 0 otherwise. + * @return true or false */ -extern int git_path_exists(const char *path); +extern bool git_path_exists(const char *path); /** * Check if the given path points to a directory. - * @return GIT_SUCCESS if it is a directory, < 0 otherwise. + * @return true or false */ -extern int git_path_isdir(const char *path); +extern bool git_path_isdir(const char *path); /** * Check if the given path points to a regular file. - * @return GIT_SUCCESS if it is a regular file, < 0 otherwise. + * @return true or false */ -extern int git_path_isfile(const char *path); +extern bool git_path_isfile(const char *path); /** * Check if the parent directory contains the item. @@ -136,25 +136,27 @@ extern int git_path_isfile(const char *path); * @param item Item that might be in the directory. * @return GIT_SUCCESS if item exists in directory, <0 otherwise. */ -extern int git_path_contains(git_buf *dir, const char *item); +extern bool git_path_contains(git_buf *dir, const char *item); /** * Check if the given path contains the given subdirectory. * * @param parent Directory path that might contain subdir * @param subdir Subdirectory name to look for in parent - * @return GIT_SUCCESS if subdirectory exists, < 0 otherwise. + * @param append_if_exists If true, then subdir will be appended to the parent path if it does exist + * @return true if subdirectory exists, false otherwise. */ -extern int git_path_contains_dir(git_buf *parent, const char *subdir); +extern bool git_path_contains_dir(git_buf *parent, const char *subdir); /** * Check if the given path contains the given file. * * @param dir Directory path that might contain file * @param file File name to look for in parent - * @return GIT_SUCCESS if file exists, < 0 otherwise. + * @param append_if_exists If true, then file will be appended to the path if it does exist + * @return true if file exists, false otherwise. */ -extern int git_path_contains_file(git_buf *dir, const char *file); +extern bool git_path_contains_file(git_buf *dir, const char *file); /** * Clean up path, prepending base if it is not already rooted. diff --git a/src/reflog.c b/src/reflog.c index 6ca9418cf..8f68a3ac7 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -246,12 +246,12 @@ int git_reflog_write(git_reference *ref, const git_oid *oid_old, if (error < GIT_SUCCESS) goto cleanup; - if (git_path_exists(log_path.ptr)) { + if (git_path_exists(log_path.ptr) == false) { error = git_futils_mkpath2file(log_path.ptr, GIT_REFLOG_DIR_MODE); if (error < GIT_SUCCESS) git__rethrow(error, "Failed to write reflog. Cannot create reflog directory"); - } else if (git_path_isfile(log_path.ptr)) { + } else if (git_path_isfile(log_path.ptr) == false) { error = git__throw(GIT_ERROR, "Failed to write reflog. `%s` is directory", log_path.ptr); } else if (oid_old == NULL) { @@ -302,7 +302,7 @@ int git_reflog_delete(git_reference *ref) error = git_buf_join_n(&path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name); - if (error == GIT_SUCCESS && git_path_exists(path.ptr) == 0) + if (error == GIT_SUCCESS && git_path_exists(path.ptr) == true) error = p_unlink(path.ptr); git_buf_free(&path); diff --git a/src/refs.c b/src/refs.c index ebfabc635..461b50719 100644 --- a/src/refs.c +++ b/src/refs.c @@ -61,8 +61,8 @@ static int packed_lookup(git_reference *ref); static int packed_write(git_repository *repo); /* internal helpers */ -static int reference_available(git_repository *repo, - const char *ref, const char *old_ref, git_error **error); +static int reference_path_available(git_repository *repo, + const char *ref, const char *old_ref); static int reference_delete(git_reference *ref); static int reference_lookup(git_reference *ref); @@ -97,36 +97,37 @@ static int reference_alloc( assert(ref_out && repo && name); reference = git__malloc(sizeof(git_reference)); - if (reference == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(reference); memset(reference, 0x0, sizeof(git_reference)); reference->owner = repo; reference->name = git__strdup(name); - if (reference->name == NULL) { - git__free(reference); - return GIT_ENOMEM; - } + GITERR_CHECK_ALLOC(reference->name); *ref_out = reference; - return GIT_SUCCESS; + return 0; } -static int reference_read(git_buf *file_content, time_t *mtime, const char *repo_path, const char *ref_name, int *updated) +static int reference_read( + git_buf *file_content, + time_t *mtime, + const char *repo_path, + const char *ref_name, + int *updated) { git_buf path = GIT_BUF_INIT; - int error = GIT_SUCCESS; + int result; assert(file_content && repo_path && ref_name); /* Determine the full path of the file */ - if ((error = git_buf_joinpath(&path, repo_path, ref_name)) == GIT_SUCCESS) - error = git_futils_readbuffer_updated(file_content, path.ptr, mtime, updated); + if (git_buf_joinpath(&path, repo_path, ref_name) < 0) + return -1; + result = git_futils_readbuffer_updated(file_content, path.ptr, mtime, updated); git_buf_free(&path); - - return error; + return result; } static int loose_parse_symbolic(git_reference *ref, git_buf *file_content) @@ -138,57 +139,58 @@ static int loose_parse_symbolic(git_reference *ref, git_buf *file_content) refname_start = (const char *)file_content->ptr; if (file_content->size < (header_len + 1)) - return git__throw(GIT_EOBJCORRUPTED, - "Failed to parse loose reference. Object too short"); + goto corrupt; /* * Assume we have already checked for the header * before calling this function */ - refname_start += header_len; ref->target.symbolic = git__strdup(refname_start); - if (ref->target.symbolic == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(ref->target.symbolic); /* remove newline at the end of file */ eol = strchr(ref->target.symbolic, '\n'); if (eol == NULL) - return git__throw(GIT_EOBJCORRUPTED, - "Failed to parse loose reference. Missing EOL"); + goto corrupt; *eol = '\0'; if (eol[-1] == '\r') eol[-1] = '\0'; - return GIT_SUCCESS; + return 0; + +corrupt: + giterr_set(GITERR_REFERENCE, "Corrupted loose reference file"); + return -1; } static int loose_parse_oid(git_oid *oid, git_buf *file_content) { - int error; char *buffer; buffer = (char *)file_content->ptr; /* File format: 40 chars (OID) + newline */ if (file_content->size < GIT_OID_HEXSZ + 1) - return git__throw(GIT_EOBJCORRUPTED, - "Failed to parse loose reference. Reference too short"); + goto corrupt; - if ((error = git_oid_fromstr(oid, buffer)) < GIT_SUCCESS) - return git__rethrow(GIT_EOBJCORRUPTED, "Failed to parse loose reference."); + if (git_oid_fromstr(oid, buffer) < 0) + goto corrupt; buffer = buffer + GIT_OID_HEXSZ; if (*buffer == '\r') buffer++; if (*buffer != '\n') - return git__throw(GIT_EOBJCORRUPTED, - "Failed to parse loose reference. Missing EOL"); + goto corrupt; return GIT_SUCCESS; + +corrupt: + giterr_set(GITERR_REFERENCE, "Corrupted loose reference file"); + return -1; } static git_rtype loose_guess_rtype(const git_buf *full_path) @@ -211,15 +213,17 @@ static git_rtype loose_guess_rtype(const git_buf *full_path) static int loose_lookup(git_reference *ref) { - int error = GIT_SUCCESS, updated; + int result, updated; git_buf ref_file = GIT_BUF_INIT; - if (reference_read(&ref_file, &ref->mtime, - ref->owner->path_repository, ref->name, &updated) < GIT_SUCCESS) - return git__throw(GIT_ENOTFOUND, "Failed to lookup loose reference"); + result = reference_read(&ref_file, &ref->mtime, + ref->owner->path_repository, ref->name, &updated); + + if (result < 0) + return result; if (!updated) - return GIT_SUCCESS; + return 0; if (ref->flags & GIT_REF_SYMBOLIC) { git__free(ref->target.symbolic); @@ -230,18 +234,14 @@ static int loose_lookup(git_reference *ref) if (git__prefixcmp((const char *)(ref_file.ptr), GIT_SYMREF) == 0) { ref->flags |= GIT_REF_SYMBOLIC; - error = loose_parse_symbolic(ref, &ref_file); + result = loose_parse_symbolic(ref, &ref_file); } else { ref->flags |= GIT_REF_OID; - error = loose_parse_oid(&ref->target.oid, &ref_file); + result = loose_parse_oid(&ref->target.oid, &ref_file); } git_buf_free(&ref_file); - - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to lookup loose reference"); - - return GIT_SUCCESS; + return result; } static int loose_lookup_to_packfile( @@ -249,54 +249,50 @@ static int loose_lookup_to_packfile( git_repository *repo, const char *name) { - int error = GIT_SUCCESS; git_buf ref_file = GIT_BUF_INIT; struct packref *ref = NULL; size_t name_len; *ref_out = NULL; - error = reference_read(&ref_file, NULL, repo->path_repository, name, NULL); - if (error < GIT_SUCCESS) - goto cleanup; + if (reference_read(&ref_file, NULL, repo->path_repository, name, NULL) < 0) + return -1; name_len = strlen(name); ref = git__malloc(sizeof(struct packref) + name_len + 1); + GITERR_CHECK_ALLOC(ref); memcpy(ref->name, name, name_len); ref->name[name_len] = 0; - error = loose_parse_oid(&ref->oid, &ref_file); - if (error < GIT_SUCCESS) - goto cleanup; + if (loose_parse_oid(&ref->oid, &ref_file) < 0) { + git_buf_free(&ref_file); + free(ref); + return -1; + } ref->flags = GIT_PACKREF_WAS_LOOSE; *ref_out = ref; git_buf_free(&ref_file); - return GIT_SUCCESS; - -cleanup: - git_buf_free(&ref_file); - git__free(ref); - - return git__rethrow(error, "Failed to lookup loose reference"); + return 0; } static int loose_write(git_reference *ref) { git_filebuf file = GIT_FILEBUF_INIT; git_buf ref_path = GIT_BUF_INIT; - int error; struct stat st; - error = git_buf_joinpath(&ref_path, ref->owner->path_repository, ref->name); - if (error < GIT_SUCCESS) - goto unlock; + if (git_buf_joinpath(&ref_path, ref->owner->path_repository, ref->name) < 0) + return -1; + + if (git_filebuf_open(&file, ref_path.ptr, GIT_FILEBUF_FORCE) < 0) { + git_buf_free(&ref_path); + return -1; + } - error = git_filebuf_open(&file, ref_path.ptr, GIT_FILEBUF_FORCE); - if (error < GIT_SUCCESS) - goto unlock; + git_buf_free(&ref_path); if (ref->flags & GIT_REF_OID) { char oid[GIT_OID_HEXSZ + 1]; @@ -304,29 +300,18 @@ static int loose_write(git_reference *ref) git_oid_fmt(oid, &ref->target.oid); oid[GIT_OID_HEXSZ] = '\0'; - error = git_filebuf_printf(&file, "%s\n", oid); - if (error < GIT_SUCCESS) - goto unlock; + git_filebuf_printf(&file, "%s\n", oid); - } else if (ref->flags & GIT_REF_SYMBOLIC) { /* GIT_REF_SYMBOLIC */ - error = git_filebuf_printf(&file, GIT_SYMREF "%s\n", ref->target.symbolic); + } else if (ref->flags & GIT_REF_SYMBOLIC) { + git_filebuf_printf(&file, GIT_SYMREF "%s\n", ref->target.symbolic); } else { - error = git__throw(GIT_EOBJCORRUPTED, - "Failed to write reference. Invalid reference type"); - goto unlock; + assert(0); /* don't let this happen */ } - if (p_stat(ref_path.ptr, &st) == GIT_SUCCESS) + if (p_stat(ref_path.ptr, &st) == 0) ref->mtime = st.st_mtime; - git_buf_free(&ref_path); - return git_filebuf_commit(&file, GIT_REFS_FILE_MODE); - -unlock: - git_buf_free(&ref_path); - git_filebuf_cleanup(&file); - return git__rethrow(error, "Failed to write loose reference"); } static int packed_parse_peel( @@ -340,34 +325,32 @@ static int packed_parse_peel( /* Ensure it's not the first entry of the file */ if (tag_ref == NULL) - return git__throw(GIT_EPACKEDREFSCORRUPTED, - "Failed to parse packed reference. " - "Reference is the first entry of the file"); + goto corrupt; /* Ensure reference is a tag */ if (git__prefixcmp(tag_ref->name, GIT_REFS_TAGS_DIR) != 0) - return git__throw(GIT_EPACKEDREFSCORRUPTED, - "Failed to parse packed reference. Reference is not a tag"); + goto corrupt; if (buffer + GIT_OID_HEXSZ >= buffer_end) - return git__throw(GIT_EPACKEDREFSCORRUPTED, - "Failed to parse packed reference. Buffer too small"); + goto corrupt; /* Is this a valid object id? */ if (git_oid_fromstr(&tag_ref->peel, buffer) < GIT_SUCCESS) - return git__throw(GIT_EPACKEDREFSCORRUPTED, - "Failed to parse packed reference. Not a valid object ID"); + goto corrupt; buffer = buffer + GIT_OID_HEXSZ; if (*buffer == '\r') buffer++; if (*buffer != '\n') - return git__throw(GIT_EPACKEDREFSCORRUPTED, - "Failed to parse packed reference. Buffer not terminated correctly"); + goto corrupt; *buffer_out = buffer + 1; - return GIT_SUCCESS; + return 0; + +corrupt: + giterr_set(GITERR_REFERENCE, "The packed references file is corrupted"); + return -1; } static int packed_parse_oid( @@ -380,26 +363,20 @@ static int packed_parse_oid( const char *buffer = *buffer_out; const char *refname_begin, *refname_end; - int error = GIT_SUCCESS; size_t refname_len; git_oid id; refname_begin = (buffer + GIT_OID_HEXSZ + 1); - if (refname_begin >= buffer_end || - refname_begin[-1] != ' ') { - error = GIT_EPACKEDREFSCORRUPTED; - goto cleanup; - } + if (refname_begin >= buffer_end || refname_begin[-1] != ' ') + goto corrupt; /* Is this a valid object id? */ - if ((error = git_oid_fromstr(&id, buffer)) < GIT_SUCCESS) - goto cleanup; + if (git_oid_fromstr(&id, buffer) < 0) + goto corrupt; refname_end = memchr(refname_begin, '\n', buffer_end - refname_begin); - if (refname_end == NULL) { - error = GIT_EPACKEDREFSCORRUPTED; - goto cleanup; - } + if (refname_end == NULL) + goto corrupt; if (refname_end[-1] == '\r') refname_end--; @@ -407,6 +384,7 @@ static int packed_parse_oid( refname_len = refname_end - refname_begin; ref = git__malloc(sizeof(struct packref) + refname_len + 1); + GITERR_CHECK_ALLOC(ref); memcpy(ref->name, refname_begin, refname_len); ref->name[refname_len] = 0; @@ -418,16 +396,17 @@ static int packed_parse_oid( *ref_out = ref; *buffer_out = refname_end + 1; - return GIT_SUCCESS; + return 0; -cleanup: +corrupt: git__free(ref); - return git__rethrow(error, "Failed to parse OID of packed reference"); + giterr_set(GITERR_REFERENCE, "The packed references file is corrupted"); + return -1; } static int packed_load(git_repository *repo) { - int error = GIT_SUCCESS, updated; + int result, updated; git_buf packfile = GIT_BUF_INIT; const char *buffer_start, *buffer_end; git_refcache *ref_cache = &repo->references; @@ -437,13 +416,10 @@ static int packed_load(git_repository *repo) ref_cache->packfile = git_hashtable_alloc( default_table_size, git_hash__strhash_cb, git_hash__strcmp_cb); - if (ref_cache->packfile == NULL) { - error = GIT_ENOMEM; - goto cleanup; - } + GITERR_CHECK_ALLOC(ref_cache->packfile); } - error = reference_read(&packfile, &ref_cache->packfile_time, + result = reference_read(&packfile, &ref_cache->packfile_time, repo->path_repository, GIT_PACKEDREFS_FILE, &updated); /* @@ -453,20 +429,21 @@ static int packed_load(git_repository *repo) * for us here, so just return. Anything else means we need to * refresh the packed refs. */ - if (error == GIT_ENOTFOUND) { + if (result == GIT_ENOTFOUND) { git_hashtable_clear(ref_cache->packfile); - return GIT_SUCCESS; - } else if (error < GIT_SUCCESS) { - return git__rethrow(error, "Failed to read packed refs"); - } else if (!updated) { - return GIT_SUCCESS; + return 0; } + if (result < 0) + return -1; + + if (!updated) + return 0; + /* * At this point, we want to refresh the packed refs. We already * have the contents in our buffer. */ - git_hashtable_clear(ref_cache->packfile); buffer_start = (const char *)packfile.ptr; @@ -474,41 +451,35 @@ static int packed_load(git_repository *repo) while (buffer_start < buffer_end && buffer_start[0] == '#') { buffer_start = strchr(buffer_start, '\n'); - if (buffer_start == NULL) { - error = GIT_EPACKEDREFSCORRUPTED; - goto cleanup; - } + if (buffer_start == NULL) + goto parse_failed; + buffer_start++; } while (buffer_start < buffer_end) { struct packref *ref = NULL; - error = packed_parse_oid(&ref, &buffer_start, buffer_end); - if (error < GIT_SUCCESS) - goto cleanup; + if (packed_parse_oid(&ref, &buffer_start, buffer_end) < 0) + goto parse_failed; if (buffer_start[0] == '^') { - error = packed_parse_peel(ref, &buffer_start, buffer_end); - if (error < GIT_SUCCESS) - goto cleanup; + if (packed_parse_peel(ref, &buffer_start, buffer_end) < 0) + goto parse_failed; } - error = git_hashtable_insert(ref_cache->packfile, ref->name, ref); - if (error < GIT_SUCCESS) { - git__free(ref); - goto cleanup; - } + if (git_hashtable_insert(ref_cache->packfile, ref->name, ref) < 0) + return -1; } git_buf_free(&packfile); - return GIT_SUCCESS; + return 0; -cleanup: +parse_failed: git_hashtable_free(ref_cache->packfile); ref_cache->packfile = NULL; git_buf_free(&packfile); - return git__rethrow(error, "Failed to load packed references"); + return -1; } @@ -521,22 +492,22 @@ struct dirent_list_data { void *callback_payload; }; -static int _dirent_loose_listall(void *_data, git_buf *full_path, git_error **error) +static int _dirent_loose_listall(void *_data, git_buf *full_path) { struct dirent_list_data *data = (struct dirent_list_data *)_data; const char *file_path = full_path->ptr + data->repo_path_len; - if (git_path_isdir(full_path->ptr) == GIT_SUCCESS) + if (git_path_isdir(full_path->ptr) == true) return git_path_direach(full_path, _dirent_loose_listall, _data); /* do not add twice a reference that exists already in the packfile */ if ((data->list_flags & GIT_REF_PACKED) != 0 && git_hashtable_lookup(data->repo->references.packfile, file_path) != NULL) - return GIT_SUCCESS; + return 0; if (data->list_flags != GIT_REF_LISTALL) { if ((data->list_flags & loose_guess_rtype(full_path)) == 0) - return GIT_SUCCESS; /* we are filtering out this reference */ + return 0; /* we are filtering out this reference */ } return data->callback(file_path, data->callback_payload); @@ -548,30 +519,23 @@ static int _dirent_loose_load(void *data, git_buf *full_path) void *old_ref = NULL; struct packref *ref; const char *file_path; - int error; - if (git_path_isdir(full_path->ptr) == GIT_SUCCESS) + if (git_path_isdir(full_path->ptr) == true) return git_path_direach(full_path, _dirent_loose_load, repository); file_path = full_path->ptr + strlen(repository->path_repository); - error = loose_lookup_to_packfile(&ref, repository, file_path); - if (error == GIT_SUCCESS) { - - if (git_hashtable_insert2( - repository->references.packfile, - ref->name, ref, &old_ref) < GIT_SUCCESS) { - git__free(ref); - return GIT_ENOMEM; - } + if (loose_lookup_to_packfile(&ref, repository, file_path) < 0) + return -1; - if (old_ref != NULL) - git__free(old_ref); + if (git_hashtable_insert2(repository->references.packfile, + ref->name, ref, &old_ref) < 0) { + git__free(ref); + return -1; } - return error == GIT_SUCCESS ? - GIT_SUCCESS : - git__rethrow(error, "Failed to load loose references into packfile"); + git__free(old_ref); + return 0; } /* @@ -582,24 +546,24 @@ static int _dirent_loose_load(void *data, git_buf *full_path) */ static int packed_loadloose(git_repository *repository) { - int error = GIT_SUCCESS; git_buf refs_path = GIT_BUF_INIT; + int result; /* the packfile must have been previously loaded! */ assert(repository->references.packfile); - if ((error = git_buf_joinpath(&refs_path, - repository->path_repository, GIT_REFS_DIR)) < GIT_SUCCESS) - return error; + if (git_buf_joinpath(&refs_path, repository->path_repository, GIT_REFS_DIR) < 0) + return -1; /* * Load all the loose files from disk into the Packfile table. * This will overwrite any old packed entries with their * updated loose versions */ - error = git_path_direach(&refs_path, _dirent_loose_load, repository); + result = git_path_direach(&refs_path, _dirent_loose_load, repository); git_buf_free(&refs_path); - return error; + + return result; } /* @@ -607,7 +571,6 @@ static int packed_loadloose(git_repository *repository) */ static int packed_write_ref(struct packref *ref, git_filebuf *file) { - int error; char oid[GIT_OID_HEXSZ + 1]; git_oid_fmt(oid, &ref->oid); @@ -628,14 +591,14 @@ static int packed_write_ref(struct packref *ref, git_filebuf *file) git_oid_fmt(peel, &ref->peel); peel[GIT_OID_HEXSZ] = 0; - error = git_filebuf_printf(file, "%s %s\n^%s\n", oid, ref->name, peel); + if (git_filebuf_printf(file, "%s %s\n^%s\n", oid, ref->name, peel) < 0) + return -1; } else { - error = git_filebuf_printf(file, "%s %s\n", oid, ref->name); + if (git_filebuf_printf(file, "%s %s\n", oid, ref->name) < 0) + return -1; } - return error == GIT_SUCCESS ? - GIT_SUCCESS : - git__rethrow(error, "Failed to write packed reference"); + return 0; } /* @@ -649,24 +612,22 @@ static int packed_write_ref(struct packref *ref, git_filebuf *file) static int packed_find_peel(git_repository *repo, struct packref *ref) { git_object *object; - int error; if (ref->flags & GIT_PACKREF_HAS_PEEL) - return GIT_SUCCESS; + return 0; /* * Only applies to tags, i.e. references * in the /refs/tags folder */ if (git__prefixcmp(ref->name, GIT_REFS_TAGS_DIR) != 0) - return GIT_SUCCESS; + return 0; /* * Find the tagged object in the repository */ - error = git_object_lookup(&object, repo, &ref->oid, GIT_OBJ_ANY); - if (error < GIT_SUCCESS) - return git__throw(GIT_EOBJCORRUPTED, "Failed to find packed reference"); + if (git_object_lookup(&object, repo, &ref->oid, GIT_OBJ_ANY) < 0) + return -1; /* * If the tagged object is a Tag object, we need to resolve it; @@ -690,7 +651,7 @@ static int packed_find_peel(git_repository *repo, struct packref *ref) } git_object_free(object); - return GIT_SUCCESS; + return 0; } /* @@ -708,25 +669,27 @@ static int packed_remove_loose(git_repository *repo, git_vector *packing_list) { unsigned int i; git_buf full_path = GIT_BUF_INIT; - int error = GIT_SUCCESS; + int failed = 0; for (i = 0; i < packing_list->length; ++i) { struct packref *ref = git_vector_get(packing_list, i); - int an_error; if ((ref->flags & GIT_PACKREF_WAS_LOOSE) == 0) continue; - an_error = git_buf_joinpath(&full_path, repo->path_repository, ref->name); + if (git_buf_joinpath(&full_path, repo->path_repository, ref->name) < 0) + return -1; /* critical; do not try to recover on oom */ - if (an_error == GIT_SUCCESS && - git_path_exists(full_path.ptr) == GIT_SUCCESS && - p_unlink(full_path.ptr) < GIT_SUCCESS) - an_error = GIT_EOSERR; + if (git_path_exists(full_path.ptr) == true && p_unlink(full_path.ptr) < 0) { + if (failed) + continue; - /* keep the error if we haven't seen one yet */ - if (error > an_error) - error = an_error; + giterr_set(GITERR_REFERENCE, + "Failed to remove loose reference '%s' after packing: %s", + full_path.ptr, strerror(errno)); + + failed = 1; + } /* * if we fail to remove a single file, this is *not* good, @@ -737,10 +700,7 @@ static int packed_remove_loose(git_repository *repo, git_vector *packing_list) } git_buf_free(&full_path); - - return error == GIT_SUCCESS ? - GIT_SUCCESS : - git__rethrow(error, "Failed to remove loose packed reference"); + return failed ? -1 : 0; } static int packed_sort(const void *a, const void *b) @@ -757,8 +717,6 @@ static int packed_sort(const void *a, const void *b) static int packed_write(git_repository *repo) { git_filebuf pack_file = GIT_FILEBUF_INIT; - int error; - const char *errmsg = "Failed to write packed references file"; unsigned int i; git_buf pack_file_path = GIT_BUF_INIT; git_vector packing_list; @@ -767,9 +725,9 @@ static int packed_write(git_repository *repo) assert(repo && repo->references.packfile); total_refs = repo->references.packfile->key_count; - if ((error = - git_vector_init(&packing_list, total_refs, packed_sort)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to init packed references list"); + + if (git_vector_init(&packing_list, total_refs, packed_sort) < 0) + return -1; /* Load all the packfile into a vector */ { @@ -785,63 +743,58 @@ static int packed_write(git_repository *repo) git_vector_sort(&packing_list); /* Now we can open the file! */ - error = git_buf_joinpath(&pack_file_path, repo->path_repository, GIT_PACKEDREFS_FILE); - if (error < GIT_SUCCESS) - goto cleanup; + if (git_buf_joinpath(&pack_file_path, repo->path_repository, GIT_PACKEDREFS_FILE) < 0) + goto cleanup_memory; - if ((error = git_filebuf_open(&pack_file, pack_file_path.ptr, 0)) < GIT_SUCCESS) { - errmsg = "Failed to open packed references file"; - goto cleanup; - } + if (git_filebuf_open(&pack_file, pack_file_path.ptr, 0) < 0) + goto cleanup_packfile; /* Packfiles have a header... apparently * This is in fact not required, but we might as well print it * just for kicks */ - if ((error = - git_filebuf_printf(&pack_file, "%s\n", GIT_PACKEDREFS_HEADER)) < GIT_SUCCESS) { - errmsg = "Failed to write packed references file header"; - goto cleanup; - } + if (git_filebuf_printf(&pack_file, "%s\n", GIT_PACKEDREFS_HEADER) < 0) + goto cleanup_packfile; for (i = 0; i < packing_list.length; ++i) { struct packref *ref = (struct packref *)git_vector_get(&packing_list, i); - if ((error = packed_find_peel(repo, ref)) < GIT_SUCCESS) { - error = git__throw(GIT_EOBJCORRUPTED, - "A reference cannot be peeled"); - goto cleanup; - } + if (packed_find_peel(repo, ref) < 0) + goto cleanup_packfile; - if ((error = packed_write_ref(ref, &pack_file)) < GIT_SUCCESS) - goto cleanup; + if (packed_write_ref(ref, &pack_file) < 0) + goto cleanup_packfile; } -cleanup: /* if we've written all the references properly, we can commit * the packfile to make the changes effective */ - if (error == GIT_SUCCESS) { - error = git_filebuf_commit(&pack_file, GIT_PACKEDREFS_FILE_MODE); - - /* when and only when the packfile has been properly written, - * we can go ahead and remove the loose refs */ - if (error == GIT_SUCCESS) { - struct stat st; + if (git_filebuf_commit(&pack_file, GIT_PACKEDREFS_FILE_MODE) < 0) + goto cleanup_memory; - error = packed_remove_loose(repo, &packing_list); + /* when and only when the packfile has been properly written, + * we can go ahead and remove the loose refs */ + if (packed_remove_loose(repo, &packing_list) < 0) + goto cleanup_memory; - if (p_stat(pack_file_path.ptr, &st) == GIT_SUCCESS) - repo->references.packfile_time = st.st_mtime; - } - } - else git_filebuf_cleanup(&pack_file); + { + struct stat st; + if (p_stat(pack_file_path.ptr, &st) == 0) + repo->references.packfile_time = st.st_mtime; + } git_vector_free(&packing_list); git_buf_free(&pack_file_path); - if (error < GIT_SUCCESS) - git__rethrow(error, "%s", errmsg); + /* we're good now */ + return 0; + +cleanup_packfile: + git_filebuf_cleanup(&pack_file); - return error; +cleanup_memory: + git_vector_free(&packing_list); + git_buf_free(&pack_file_path); + + return -1; } struct reference_available_t { @@ -855,9 +808,7 @@ static int _reference_available_cb(const char *ref, void *data) struct reference_available_t *d; assert(ref && data); - - d = (reference_available_t *)data; - refs = (const char **)data; + d = (struct reference_available_t *)data; if (!d->old_ref || strcmp(d->old_ref, ref)) { int reflen = strlen(ref); @@ -874,12 +825,10 @@ static int _reference_available_cb(const char *ref, void *data) return 0; } -static int reference_available( - int *available, +static int reference_path_available( git_repository *repo, const char *ref, - const char* old_ref, - git_error **error) + const char* old_ref) { struct reference_available_t data; @@ -888,27 +837,29 @@ static int reference_available( data.available = 1; if (git_reference_foreach(repo, GIT_REF_LISTALL, - _reference_available_cb, (void *)&data, error) < 0) + _reference_available_cb, (void *)&data) < 0) + return -1; + + if (!data.available) { + giterr_set(GITERR_REFERENCE, + "The path to reference '%s' collides with an existing one"); return -1; + } - *available = data.available; return 0; } static int reference_exists(int *exists, git_repository *repo, const char *ref_name) { - int error; git_buf ref_path = GIT_BUF_INIT; - error = packed_load(repo); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Cannot resolve if a reference exists"); + if (packed_load(repo) < 0) + return -1; - error = git_buf_joinpath(&ref_path, repo->path_repository, ref_name); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Cannot resolve if a reference exists"); + if (git_buf_joinpath(&ref_path, repo->path_repository, ref_name) < 0) + return -1; - if (git_path_isfile(ref_path.ptr) == GIT_SUCCESS || + if (git_path_isfile(ref_path.ptr) == true || git_hashtable_lookup(repo->references.packfile, ref_path.ptr) != NULL) { *exists = 1; } else { @@ -916,23 +867,74 @@ static int reference_exists(int *exists, git_repository *repo, const char *ref_n } git_buf_free(&ref_path); + return 0; +} - return GIT_SUCCESS; +/* + * Check if a reference could be written to disk, based on: + * + * - Whether a reference with the same name already exists, + * and we are allowing or disallowing overwrites + * + * - Whether the name of the reference would collide with + * an existing path + */ +static int reference_can_write( + git_repository *repo, + const char *refname, + const char *previous_name, + int force) +{ + /* see if the reference shares a path with an existing reference; + * if a path is shared, we cannot create the reference, even when forcing */ + if (reference_path_available(repo, refname, previous_name) < 0) + return -1; + + /* check if the reference actually exists, but only if we are not forcing + * the rename. If we are forcing, it's OK to overwrite */ + if (!force) { + int exists; + + if (reference_exists(&exists, repo, refname) < 0) + return -1; + + /* We cannot proceed if the reference already exists and we're not forcing + * the rename; the existing one would be overwritten */ + if (exists) { + giterr_set(GITERR_REFERENCE, + "A reference with that name (%s) already exists"); + return GIT_EEXISTS; + } + } + + /* FIXME: if the reference exists and we are forcing, do we really need to + * remove the reference first? + * + * Two cases: + * + * - the reference already exists and is loose: not a problem, the file + * gets overwritten on disk + * + * - the reference already exists and is packed: we write a new one as + * loose, which by all means renders the packed one useless + */ + + return 0; } + static int packed_lookup(git_reference *ref) { - int error; struct packref *pack_ref = NULL; - error = packed_load(ref->owner); - if (error < GIT_SUCCESS) - return git__rethrow(error, - "Failed to lookup reference from packfile"); + if (packed_load(ref->owner) < 0) + return -1; - if (ref->flags & GIT_REF_PACKED && + /* maybe the packfile hasn't changed at all, so we don't + * have to re-lookup the reference */ + if ((ref->flags & GIT_REF_PACKED) && ref->mtime == ref->owner->references.packfile_time) - return GIT_SUCCESS; + return 0; if (ref->flags & GIT_REF_SYMBOLIC) { git__free(ref->target.symbolic); @@ -941,38 +943,38 @@ static int packed_lookup(git_reference *ref) /* Look up on the packfile */ pack_ref = git_hashtable_lookup(ref->owner->references.packfile, ref->name); - if (pack_ref == NULL) - return git__throw(GIT_ENOTFOUND, - "Failed to lookup reference from packfile"); + if (pack_ref == NULL) { + giterr_set(GITERR_REFERENCE, "Reference '%s' not found", ref->name); + return GIT_ENOTFOUND; + } ref->flags = GIT_REF_OID | GIT_REF_PACKED; ref->mtime = ref->owner->references.packfile_time; git_oid_cpy(&ref->target.oid, &pack_ref->oid); - return GIT_SUCCESS; + return 0; } -static int reference_lookup(git_reference *ref, git_error **error) +static int reference_lookup(git_reference *ref) { int result; - result = loose_lookup(ref, error); - if (result != GIT_ENOTFOUND) - return result; + result = loose_lookup(ref); + if (result == 0) + return 0; - result = packed_lookup(ref, error); - if (result != GIT_ENOTFOUND) - return result; + /* only try to lookup this reference on the packfile if it + * wasn't found on the loose refs; not if there was a critical error */ + if (result == GIT_ENOTFOUND) { + giterr_clear(); + result = packed_lookup(ref); + if (result == 0) + return 0; + } + /* unexpected error; free the reference */ git_reference_free(ref); - - if (error_loose != GIT_ENOTFOUND) - return git__rethrow(error_loose, "Failed to lookup reference"); - - if (error_packed != GIT_ENOTFOUND) - return git__rethrow(error_packed, "Failed to lookup reference"); - - return git__throw(GIT_ENOTFOUND, "Reference not found"); + return result; } /* @@ -980,7 +982,7 @@ static int reference_lookup(git_reference *ref, git_error **error) * This is an internal method; the reference is removed * from disk or the packfile, but the pointer is not freed */ -static int reference_delete(git_reference *ref, git_error **error) +static int reference_delete(git_reference *ref) { int result; @@ -992,18 +994,18 @@ static int reference_delete(git_reference *ref, git_error **error) if (ref->flags & GIT_REF_PACKED) { struct packref *packref; /* load the existing packfile */ - if (packed_load(ref->owner, error) < 0) + if (packed_load(ref->owner) < 0) return -1; if (git_hashtable_remove2(ref->owner->references.packfile, ref->name, (void **) &packref) < 0) { - giterr_set(error, GITERR_REFERENCE, + giterr_set(GITERR_REFERENCE, "Reference %s stopped existing in the packfile", ref->name); return -1; } git__free(packref); - if (packed_write(ref->owner, error) < 0) + if (packed_write(ref->owner) < 0) return -1; /* If the reference is loose, we can just remove the reference @@ -1012,55 +1014,59 @@ static int reference_delete(git_reference *ref, git_error **error) git_reference *ref_in_pack; git_buf full_path = GIT_BUF_INIT; - if (git_buf_joinpath(&full_path, ref->owner->path_repository, ref->name) < 0) { - giterr_set_oom(error); + if (git_buf_joinpath(&full_path, ref->owner->path_repository, ref->name) < 0) return -1; - } result = p_unlink(full_path.ptr); git_buf_free(&full_path); /* done with path at this point */ if (result < 0) { - giterr_set(error, GITERR_OS, + giterr_set(GITERR_OS, "Failed to unlink '%s': %s", full_path.ptr, strerror(errno)); return -1; } /* When deleting a loose reference, we have to ensure that an older * packed version of it doesn't exist */ - if (git_reference_lookup(&ref_in_pack, ref->owner, ref->name, NULL) == GIT_SUCCESS) { + if (git_reference_lookup(&ref_in_pack, ref->owner, ref->name) == 0) { assert((ref_in_pack->flags & GIT_REF_PACKED) != 0); - return git_reference_delete(ref_in_pack, error); + return git_reference_delete(ref_in_pack); } + + giterr_clear(); } return 0; } -int git_reference_delete(git_reference *ref, git_error **error) +int git_reference_delete(git_reference *ref) { - int result = reference_delete(ref, error); + int result = reference_delete(ref); git_reference_free(ref); return result; } - int git_reference_lookup(git_reference **ref_out, - git_repository *repo, const char *name, git_error **error) + git_repository *repo, const char *name) { char normalized_name[GIT_REFNAME_MAX]; git_reference *ref = NULL; + int result; assert(ref_out && repo && name); + *ref_out = NULL; - if (normalize_name(normalized_name, sizeof(normalized_name), name, 0, error) < 0) + if (normalize_name(normalized_name, sizeof(normalized_name), name, 0) < 0) return -1; - if (reference_alloc(&ref, repo, normalized_name, error) < 0) + if (reference_alloc(&ref, repo, normalized_name) < 0) return -1; - *ref_out = ref; - return reference_lookup(ref, error); + result = reference_lookup(ref); + if (result == 0) + *ref_out = ref; + + return result; } /** @@ -1122,33 +1128,25 @@ int git_reference_create_symbolic( git_repository *repo, const char *name, const char *target, - int force, - git_error **error) + int force) { char normalized[GIT_REFNAME_MAX]; - int exists; git_reference *ref = NULL; - if (normalize_name(normalized, sizeof(normalized), name, 0, error) < 0) + if (normalize_name(normalized, sizeof(normalized), name, 0) < 0) return -1; - if (reference_exists(&exists, repo, normalized, error) < 0) + if (reference_can_write(repo, normalized, NULL, force) < 0) return -1; - if (exists && !force) { - giterr_set(error, GITERR_REFERENCE, - "A reference with that name (%s) already exists"); - return GIT_EEXISTS; - } - - if (reference_alloc(&ref, repo, normalized, error) < 0) + if (reference_alloc(&ref, repo, normalized) < 0) return -1; ref->flags |= GIT_REF_SYMBOLIC; /* set the target; this will normalize the name automatically * and write the reference on disk */ - if (git_reference_set_target(ref, target, error) < 0) { + if (git_reference_set_target(ref, target) < 0) { git_reference_free(ref); return -1; } @@ -1166,32 +1164,24 @@ int git_reference_create_oid( git_repository *repo, const char *name, const git_oid *id, - int force, - git_error **error) + int force) { - int exists; git_reference *ref = NULL; char normalized[GIT_REFNAME_MAX]; - if (normalize_name(normalized, sizeof(normalized), name, 1, error) < 0) + if (normalize_name(normalized, sizeof(normalized), name, 1) < 0) return -1; - if (reference_exists(&exists, repo, normalized, error) < 0) + if (reference_can_write(repo, normalized, NULL, force) < 0) return -1; - if (exists && !force) { - giterr_set(error, GITERR_REFERENCE, - "A reference with that name (%s) already exists"); - return GIT_EEXISTS; - } - - if (reference_alloc(&ref, repo, name, error) < 0) + if (reference_alloc(&ref, repo, name) < 0) return -1; ref->flags |= GIT_REF_OID; /* set the oid; this will write the reference on disk */ - if (git_reference_set_oid(ref, id, error) < 0) { + if (git_reference_set_oid(ref, id) < 0) { git_reference_free(ref); return -1; } @@ -1213,25 +1203,24 @@ int git_reference_create_oid( * We do not repack packed references because of performance * reasons. */ -int git_reference_set_oid(git_reference *ref, const git_oid *id, git_error **error) +int git_reference_set_oid(git_reference *ref, const git_oid *id) { git_odb *odb = NULL; if ((ref->flags & GIT_REF_OID) == 0) { - giterr_set(error, GITERR_REFERENCE, - "Cannot set OID on symbolic reference"); + giterr_set(GITERR_REFERENCE, "Cannot set OID on symbolic reference"); return -1; } assert(ref->owner); - if (git_repository_odb__weakptr(&odb, ref->owner, error) < 0) + if (git_repository_odb__weakptr(&odb, ref->owner) < 0) return -1; /* Don't let the user create references to OIDs that * don't exist in the ODB */ if (!git_odb_exists(odb, id)) { - giterr_set(error, GITERR_REFERENCE, + giterr_set(GITERR_REFERENCE, "Target OID for the reference doesn't exist on the repository"); return -1; } @@ -1240,7 +1229,7 @@ int git_reference_set_oid(git_reference *ref, const git_oid *id, git_error **err git_oid_cpy(&ref->target.oid, id); /* Write back to disk */ - return loose_write(ref, error); + return loose_write(ref); } /* @@ -1250,72 +1239,46 @@ int git_reference_set_oid(git_reference *ref, const git_oid *id, git_error **err * a pack. We just change the target in memory * and overwrite the file on disk. */ -int git_reference_set_target(git_reference *ref, const char *target, git_error **error) +int git_reference_set_target(git_reference *ref, const char *target) { char normalized[GIT_REFNAME_MAX]; if ((ref->flags & GIT_REF_SYMBOLIC) == 0) { - giterr_set(error, GITERR_REFERENCE, + giterr_set(GITERR_REFERENCE, "Cannot set symbolic target on a direct reference"); return -1; } - if (normalize_name(normalized, sizeof(normalized), target, 0, error)) + if (normalize_name(normalized, sizeof(normalized), target, 0)) return -1; git__free(ref->target.symbolic); ref->target.symbolic = git__strdup(normalized); - GITERR_CHECK_ALLOC(ref->target.symbolic, error); + GITERR_CHECK_ALLOC(ref->target.symbolic); - return loose_write(ref, error); + return loose_write(ref); } -int git_reference_rename(git_reference *ref, const char *new_name, int force, git_error **error) +int git_reference_rename(git_reference *ref, const char *new_name, int force) { - int result, ref_available; + int result; git_buf aux_path = GIT_BUF_INIT; char normalized[GIT_REFNAME_MAX]; const char *head_target = NULL; - git_reference *existing_ref = NULL, *head = NULL; + git_reference *head = NULL; if (normalize_name(normalized, sizeof(normalized), - new_name, ref->flags & GIT_REF_OID, error) < 0) + new_name, ref->flags & GIT_REF_OID) < 0) return -1; - new_name = normalized; - - /* see if the reference already exists */ - if (reference_available(&ref_available, ref->owner, new_name, ref->name, error) < 0) + if (reference_can_write(ref->owner, normalized, ref->name, force) < 0) return -1; - /* We cannot proceed if the reference already exists and we're not forcing - * the rename; the existing one would be overwritten */ - if (!force && !ref_available) { - giterr_set(error, GITERR_REFERENCE, - "A reference with the same name (%s) already exists", normalized); - return GIT_EEXISTS; - } - - /* FIXME: if the reference exists and we are forcing, do we really need to - * remove the reference first? - * - * Two cases: - * - * - the reference already exists and is loose: not a problem, the file - * gets overwritten on disk - * - * - the reference already exists and is packed: we write a new one as - * loose, which by all means renders the packed one useless - */ - /* Initialize path now so we won't get an allocation failure once - * we actually start removing things. - */ - if (git_buf_joinpath(&aux_path, ref->owner->path_repository, new_name) < 0) { - giterr_set_oom(error); + * we actually start removing things. */ + if (git_buf_joinpath(&aux_path, ref->owner->path_repository, new_name) < 0) return -1; - } /* * Now delete the old ref and remove an possibly existing directory @@ -1323,25 +1286,18 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force, gi * method deletes the ref from disk but doesn't free the pointer, so * we can still access the ref's attributes for creating the new one */ - if (reference_delete(ref, error) < 0) + if (reference_delete(ref) < 0) goto cleanup; - if (git_path_exists(aux_path.ptr) == GIT_SUCCESS) { - if (git_path_isdir(aux_path.ptr) == GIT_SUCCESS) { - if (git_futils_rmdir_r(aux_path.ptr, 0, error) < 0) - goto rollback; - } else goto rollback; - } - /* * Finally we can create the new reference. */ if (ref->flags & GIT_REF_SYMBOLIC) { result = git_reference_create_symbolic( - NULL, ref->owner, new_name, ref->target.symbolic, force, error); + NULL, ref->owner, new_name, ref->target.symbolic, force); } else { result = git_reference_create_oid( - NULL, ref->owner, new_name, &ref->target.oid, force, error); + NULL, ref->owner, new_name, &ref->target.oid, force); } if (result < 0) @@ -1350,8 +1306,8 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force, gi /* * Check if we have to update HEAD. */ - if (git_reference_lookup(&head, ref->owner, GIT_HEAD_FILE, NULL) < 0) { - giterr_set(error, GITERR_REFERENCE, + if (git_reference_lookup(&head, ref->owner, GIT_HEAD_FILE) < 0) { + giterr_set(GITERR_REFERENCE, "Failed to update HEAD after renaming reference"); goto cleanup; } @@ -1359,8 +1315,8 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force, gi head_target = git_reference_target(head); if (head_target && !strcmp(head_target, ref->name)) { - if (git_reference_create_symbolic(&head, ref->owner, "HEAD", new_name, 1, NULL) < 0) { - giterr_set(error, GITERR_REFERENCE, + if (git_reference_create_symbolic(&head, ref->owner, "HEAD", new_name, 1) < 0) { + giterr_set(GITERR_REFERENCE, "Failed to update HEAD after renaming reference"); goto cleanup; } @@ -1369,15 +1325,14 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force, gi /* * Rename the reflog file. */ - if (git_buf_join_n(&aux_path, '/', 3, ref->owner->path_repository, - GIT_REFLOG_DIR, ref->name) < 0) { - giterr_set_oom(error); + if (git_buf_join_n(&aux_path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name) < 0) goto cleanup; - } - if (git_path_exists(aux_path.ptr) == 0) { - if (git_reflog_rename(ref, new_name, error) < 0) + if (git_path_exists(aux_path.ptr) == true) { + if (git_reflog_rename(ref, new_name) < 0) goto cleanup; + } else { + giterr_clear(); } /* @@ -1404,20 +1359,19 @@ rollback: */ if (ref->flags & GIT_REF_SYMBOLIC) git_reference_create_symbolic( - NULL, ref->owner, ref->name, ref->target.symbolic, 0, NULL); + NULL, ref->owner, ref->name, ref->target.symbolic, 0); else git_reference_create_oid( - NULL, ref->owner, ref->name, &ref->target.oid, 0. NULL); + NULL, ref->owner, ref->name, &ref->target.oid, 0); /* The reference is no longer packed */ ref->flags &= ~GIT_REF_PACKED; git_buf_free(&aux_path); - return -1; } -int git_reference_resolve(git_reference **ref_out, git_reference *ref, git_error **error) +int git_reference_resolve(git_reference **ref_out, git_reference *ref) { int result, i = 0; git_repository *repo; @@ -1431,13 +1385,13 @@ int git_reference_resolve(git_reference **ref_out, git_reference *ref, git_error * copy. Instead of duplicating `ref`, we look it up again to * ensure the copy is out to date */ if (ref->flags & GIT_REF_OID) - return git_reference_lookup(ref_out, ref->owner, ref->name, error); + return git_reference_lookup(ref_out, ref->owner, ref->name); /* Otherwise, keep iterating until the reference is resolved */ for (i = 0; i < MAX_NESTING_LEVEL; ++i) { git_reference *new_ref; - result = git_reference_lookup(&new_ref, repo, ref->target.symbolic, error); + result = git_reference_lookup(&new_ref, repo, ref->target.symbolic); if (result < 0) return result; @@ -1452,21 +1406,21 @@ int git_reference_resolve(git_reference **ref_out, git_reference *ref, git_error * successfully resolved the symbolic ref */ if (ref->flags & GIT_REF_OID) { *ref_out = ref; - return GIT_SUCCESS; + return 0; } } - giterr_set(error, GITERR_REFERENCE, + giterr_set(GITERR_REFERENCE, "Symbolic reference too nested (%d levels deep)", MAX_NESTING_LEVEL); return -1; } -int git_reference_packall(git_repository *repo, git_error **error) +int git_reference_packall(git_repository *repo) { - if (packed_load(repo, error) < 0 || /* load the existing packfile */ - packed_loadloose(repo, error) < 0 || /* add all the loose refs */ - packed_write(repo, error) < 0) /* write back to disk */ + if (packed_load(repo) < 0 || /* load the existing packfile */ + packed_loadloose(repo) < 0 || /* add all the loose refs */ + packed_write(repo) < 0) /* write back to disk */ return -1; return 0; @@ -1476,8 +1430,7 @@ int git_reference_foreach( git_repository *repo, unsigned int list_flags, int (*callback)(const char *, void *), - void *payload, - git_error **error) + void *payload) { int result; struct dirent_list_data data; @@ -1487,10 +1440,10 @@ int git_reference_foreach( if (list_flags & GIT_REF_PACKED) { const char *ref_name; - if (packed_load(repo, error) < 0) + if (packed_load(repo) < 0) return -1; - GIT_HASHTABLE_FOREACH(repo->references.packfile, ref_name, _unused, + GIT_HASHTABLE_FOREACH_KEY(repo->references.packfile, ref_name, if (callback(ref_name, payload) < 0) return 0; ); @@ -1505,12 +1458,10 @@ int git_reference_foreach( data.callback = callback; data.callback_payload = payload; - if (git_buf_joinpath(&refs_path, repo->path_repository, GIT_REFS_DIR) < 0) { - giterr_set_oom(error); + if (git_buf_joinpath(&refs_path, repo->path_repository, GIT_REFS_DIR) < 0) return -1; - } - result = git_path_direach(&refs_path, _dirent_loose_listall, &data, error); + result = git_path_direach(&refs_path, _dirent_loose_listall, &data); git_buf_free(&refs_path); return result; @@ -1524,10 +1475,8 @@ static int cb__reflist_add(const char *ref, void *data) int git_reference_listall( git_strarray *array, git_repository *repo, - unsigned int list_flags, - git_error **error) + unsigned int list_flags) { - int result; git_vector ref_list; assert(array && repo); @@ -1535,25 +1484,23 @@ int git_reference_listall( array->strings = NULL; array->count = 0; - if (git_vector_init(&ref_list, 8, NULL) < GIT_SUCCESS) { - giterr_set_oom(error); + if (git_vector_init(&ref_list, 8, NULL) < GIT_SUCCESS) return -1; - } if (git_reference_foreach( - repo, list_flags, &cb__reflist_add, (void *)&ref_list, error) < 0) { + repo, list_flags, &cb__reflist_add, (void *)&ref_list) < 0) { git_vector_free(&ref_list); return -1; } array->strings = (char **)ref_list.contents; array->count = ref_list.length; - return GIT_SUCCESS; + return 0; } -int git_reference_reload(git_reference *ref, git_error **error) +int git_reference_reload(git_reference *ref) { - return reference_lookup(ref, error); + return reference_lookup(ref); } void git_repository__refcache_free(git_refcache *refs) @@ -1610,33 +1557,26 @@ static int normalize_name( /* A refname can not be empty */ if (name_end == name) - return git__throw(GIT_EINVALIDREFNAME, - "Failed to normalize name. Reference name is empty"); + goto invalid_name; /* A refname can not end with a dot or a slash */ if (*(name_end - 1) == '.' || *(name_end - 1) == '/') - return git__throw(GIT_EINVALIDREFNAME, - "Failed to normalize name. Reference name ends with dot or slash"); + goto invalid_name; while (current < name_end && out_size) { if (!is_valid_ref_char(*current)) - return git__throw(GIT_EINVALIDREFNAME, - "Failed to normalize name. " - "Reference name contains invalid characters"); + goto invalid_name; if (buffer_out > buffer_out_start) { char prev = *(buffer_out - 1); /* A refname can not start with a dot nor contain a double dot */ if (*current == '.' && ((prev == '.') || (prev == '/'))) - return git__throw(GIT_EINVALIDREFNAME, - "Failed to normalize name. " - "Reference name starts with a dot or contains a double dot"); + goto invalid_name; /* '@{' is forbidden within a refname */ if (*current == '{' && prev == '@') - return git__throw(GIT_EINVALIDREFNAME, - "Failed to normalize name. Reference name contains '@{'"); + goto invalid_name; /* Prevent multiple slashes from being added to the output */ if (*current == '/' && prev == '/') { @@ -1653,7 +1593,7 @@ static int normalize_name( } if (!out_size) - return git__throw(GIT_EINVALIDREFNAME, "Reference name is too long"); + goto invalid_name; /* Object id refname have to contain at least one slash, except * for HEAD in a detached state or MERGE_HEAD if we're in the @@ -1663,13 +1603,11 @@ static int normalize_name( strcmp(name, GIT_HEAD_FILE) != 0 && strcmp(name, GIT_MERGE_HEAD_FILE) != 0 && strcmp(name, GIT_FETCH_HEAD_FILE) != 0) - return git__throw(GIT_EINVALIDREFNAME, - "Failed to normalize name. Reference name contains no slashes"); + goto invalid_name; /* A refname can not end with ".lock" */ if (!git__suffixcmp(name, GIT_FILELOCK_EXTENSION)) - return git__throw(GIT_EINVALIDREFNAME, - "Failed to normalize name. Reference name ends with '.lock'"); + goto invalid_name; *buffer_out = '\0'; @@ -1679,11 +1617,13 @@ static int normalize_name( */ if (is_oid_ref && !(git__prefixcmp(buffer_out_start, GIT_REFS_DIR) || strcmp(buffer_out_start, GIT_HEAD_FILE))) - return git__throw(GIT_EINVALIDREFNAME, - "Failed to normalize name. " - "Reference name does not start with 'refs/'"); + goto invalid_name; - return GIT_SUCCESS; + return 0; + +invalid_name: + giterr_set(GITERR_REFERENCE, "The given reference name is not valid"); + return -1; } int git_reference__normalize_name( diff --git a/src/repository.c b/src/repository.c index 1f8306991..fe785cfcd 100644 --- a/src/repository.c +++ b/src/repository.c @@ -83,14 +83,14 @@ void git_repository_free(git_repository *repo) static int quickcheck_repository_dir(git_buf *repository_path) { /* Check OBJECTS_DIR first, since it will generate the longest path name */ - if (git_path_contains_dir(repository_path, GIT_OBJECTS_DIR) < 0) + if (git_path_contains_dir(repository_path, GIT_OBJECTS_DIR) == false) return GIT_ERROR; /* Ensure HEAD file exists */ - if (git_path_contains_file(repository_path, GIT_HEAD_FILE) < 0) + if (git_path_contains_file(repository_path, GIT_HEAD_FILE) == false) return GIT_ERROR; - if (git_path_contains_dir(repository_path, GIT_REFS_DIR) < 0) + if (git_path_contains_dir(repository_path, GIT_REFS_DIR) == false) return GIT_ERROR; return GIT_SUCCESS; @@ -498,7 +498,7 @@ static int read_gitfile(git_buf *path_out, const char *file_path, const char *ba git_buf_free(&file); - if (error == GIT_SUCCESS && git_path_exists(path_out->ptr) == 0) + if (error == GIT_SUCCESS && git_path_exists(path_out->ptr) == true) return GIT_SUCCESS; return git__throw(GIT_EOBJCORRUPTED, "The `.git` file points to a nonexistent path"); @@ -542,7 +542,7 @@ int git_repository_discover( * If the `.git` file is regular instead of * a directory, it should contain the path of the actual git repository */ - if (git_path_isfile(normal_path.ptr) == GIT_SUCCESS) { + if (git_path_isfile(normal_path.ptr) == true) { git_buf gitfile_path = GIT_BUF_INIT; error = read_gitfile(&gitfile_path, normal_path.ptr, bare_path.ptr); @@ -564,7 +564,7 @@ int git_repository_discover( /** * If the `.git` file is a folder, we check inside of it */ - if (git_path_isdir(normal_path.ptr) == GIT_SUCCESS) { + if (git_path_isdir(normal_path.ptr) == true) { error = quickcheck_repository_dir(&normal_path); if (error == GIT_SUCCESS) { found_path = &normal_path; @@ -774,7 +774,7 @@ int git_repository_init(git_repository **repo_out, const char *path, unsigned is if (error < GIT_SUCCESS) return error; - if (git_path_isdir(repository_path.ptr) == GIT_SUCCESS) { + if (git_path_isdir(repository_path.ptr) == true) { if (quickcheck_repository_dir(&repository_path) == GIT_SUCCESS) { error = repo_init_reinit(repo_out, repository_path.ptr, is_bare); git_buf_free(&repository_path); diff --git a/src/status.c b/src/status.c index 106e5005d..e80fc02c0 100644 --- a/src/status.c +++ b/src/status.c @@ -505,7 +505,7 @@ int git_status_foreach( dirent_st.index_position = 0; dirent_st.is_dir = 1; - if (git_path_isdir(workdir)) { + if (git_path_isdir(workdir) == false) { error = git__throw(GIT_EINVALIDPATH, "Failed to determine status of file '%s'. " "The given path doesn't lead to a folder", workdir); @@ -608,7 +608,7 @@ int git_status_file(unsigned int *status_flags, git_repository *repo, const char return git__rethrow(error, "Failed to determine status of file '%s'", path); - if (git_path_isdir(temp_path.ptr) == GIT_SUCCESS) { + if (git_path_isdir(temp_path.ptr) == true) { git_buf_free(&temp_path); return git__throw(GIT_EINVALIDPATH, "Failed to determine status of file '%s'. " @@ -622,7 +622,7 @@ int git_status_file(unsigned int *status_flags, git_repository *repo, const char } /* Find file in Workdir */ - if (git_path_exists(temp_path.ptr) == GIT_SUCCESS) { + if (git_path_exists(temp_path.ptr) == true) { if ((error = status_entry_update_from_workdir(e, temp_path.ptr)) < GIT_SUCCESS) goto cleanup; /* The callee has already set the error message */ } @@ -702,7 +702,7 @@ static char *alphasorted_dirent_info_new(const git_buf *path) git_buf_copy_cstr(di, path->size + 1, path); - if (git_path_isdir(path->ptr) == GIT_SUCCESS) { + if (git_path_isdir(path->ptr) == true) { /* * Append a forward slash to the name to force folders * to be ordered in a similar way than in a tree diff --git a/tests-clar/config/stress.c b/tests-clar/config/stress.c index e3b1114f0..25c2c66db 100644 --- a/tests-clar/config/stress.c +++ b/tests-clar/config/stress.c @@ -27,7 +27,7 @@ void test_config_stress__dont_break_on_invalid_input(void) struct git_config_file *file; git_config *config; - cl_git_pass(git_path_exists("git-test-config")); + cl_assert(git_path_exists("git-test-config")); cl_git_pass(git_config_file__ondisk(&file, "git-test-config")); cl_git_pass(git_config_new(&config)); cl_git_pass(git_config_add_file(config, file, 0)); diff --git a/tests-clar/core/filebuf.c b/tests-clar/core/filebuf.c index 29d6bca74..eab8a26eb 100644 --- a/tests-clar/core/filebuf.c +++ b/tests-clar/core/filebuf.c @@ -14,7 +14,7 @@ void test_core_filebuf__0(void) cl_must_pass(p_close(fd)); cl_git_fail(git_filebuf_open(&file, test, 0)); - cl_git_pass(git_path_exists(testlock)); + cl_assert(git_path_exists(testlock)); cl_must_pass(p_unlink(testlock)); } @@ -56,20 +56,6 @@ void test_core_filebuf__2(void) cl_must_pass(p_unlink(test)); } - -/* make sure git_filebuf_open won't reopen an open buffer */ -void test_core_filebuf__3(void) -{ - git_filebuf file = GIT_FILEBUF_INIT; - char test[] = "test"; - - cl_git_pass(git_filebuf_open(&file, test, 0)); - cl_git_fail(git_filebuf_open(&file, test, 0)); - - git_filebuf_cleanup(&file); -} - - /* make sure git_filebuf_cleanup clears the buffer */ void test_core_filebuf__4(void) { diff --git a/tests/t00-core.c b/tests/t00-core.c index 7f142ba21..10e6aaebf 100644 --- a/tests/t00-core.c +++ b/tests/t00-core.c @@ -485,7 +485,7 @@ BEGIN_TEST(filebuf0, "make sure git_filebuf_open doesn't delete an existing lock must_pass(fd); must_pass(p_close(fd)); must_fail(git_filebuf_open(&file, test, 0)); - must_pass(git_path_exists(testlock)); + must_be_true(git_path_exists(testlock)); must_pass(p_unlink(testlock)); END_TEST diff --git a/tests/t03-objwrite.c b/tests/t03-objwrite.c index 1650b8060..2d4fb1ad7 100644 --- a/tests/t03-objwrite.c +++ b/tests/t03-objwrite.c @@ -44,9 +44,9 @@ static int make_odb_dir(void) static int check_object_files(object_data *d) { - if (git_path_exists(d->dir) < 0) + if (git_path_exists(d->dir) == false) return -1; - if (git_path_exists(d->file) < 0) + if (git_path_exists(d->file) == false) return -1; return 0; } diff --git a/tests/t08-tag.c b/tests/t08-tag.c index eacbb3ae1..4cbd48379 100644 --- a/tests/t08-tag.c +++ b/tests/t08-tag.c @@ -337,8 +337,6 @@ BEGIN_TEST(delete0, "Delete an already existing tag") must_fail(git_reference_lookup(&ref_tag, repo, "refs/tags/e90810b")); close_temp_repo(repo); - - git_reference_free(ref_tag); END_TEST BEGIN_SUITE(tag) diff --git a/tests/t10-refs.c b/tests/t10-refs.c index 63d1cb7d1..ad881726e 100644 --- a/tests/t10-refs.c +++ b/tests/t10-refs.c @@ -69,7 +69,6 @@ BEGIN_TEST(readtag1, "lookup a loose tag reference that doesn't exist") must_fail(git_reference_lookup(&reference, repo, non_existing_tag_ref_name)); git_repository_free(repo); - git_reference_free(reference); END_TEST @@ -530,7 +529,7 @@ BEGIN_TEST(pack1, "create a packfile from all the loose rn a repo") /* Ensure the packed-refs file exists */ must_pass(git_buf_joinpath(&temp_path, repo->path_repository, GIT_PACKEDREFS_FILE)); - must_pass(git_path_exists(temp_path.ptr)); + must_be_true(git_path_exists(temp_path.ptr)); /* Ensure the known ref can still be looked up but is now packed */ must_pass(git_reference_lookup(&reference, repo, loose_tag_ref_name)); @@ -539,7 +538,7 @@ BEGIN_TEST(pack1, "create a packfile from all the loose rn a repo") /* Ensure the known ref has been removed from the loose folder structure */ must_pass(git_buf_joinpath(&temp_path, repo->path_repository, loose_tag_ref_name)); - must_pass(!git_path_exists(temp_path.ptr)); + must_be_true(!git_path_exists(temp_path.ptr)); close_temp_repo(repo); @@ -557,7 +556,7 @@ BEGIN_TEST(rename0, "rename a loose reference") /* Ensure the ref doesn't exist on the file system */ must_pass(git_buf_joinpath(&temp_path, repo->path_repository, new_name)); - must_pass(!git_path_exists(temp_path.ptr)); + must_be_true(!git_path_exists(temp_path.ptr)); /* Retrieval of the reference to rename */ must_pass(git_reference_lookup(&looked_up_ref, repo, loose_tag_ref_name)); @@ -582,7 +581,7 @@ BEGIN_TEST(rename0, "rename a loose reference") /* ...and the ref can be found in the file system */ must_pass(git_buf_joinpath(&temp_path, repo->path_repository, new_name)); - must_pass(git_path_exists(temp_path.ptr)); + must_be_true(git_path_exists(temp_path.ptr)); close_temp_repo(repo); @@ -601,7 +600,7 @@ BEGIN_TEST(rename1, "rename a packed reference (should make it loose)") /* Ensure the ref doesn't exist on the file system */ must_pass(git_buf_joinpath(&temp_path, repo->path_repository, packed_head_name)); - must_pass(!git_path_exists(temp_path.ptr)); + must_be_true(!git_path_exists(temp_path.ptr)); /* The reference can however be looked-up... */ must_pass(git_reference_lookup(&looked_up_ref, repo, packed_head_name)); @@ -626,7 +625,7 @@ BEGIN_TEST(rename1, "rename a packed reference (should make it loose)") /* ...and the ref now happily lives in the file system */ must_pass(git_buf_joinpath(&temp_path, repo->path_repository, brand_new_name)); - must_pass(git_path_exists(temp_path.ptr)); + must_be_true(git_path_exists(temp_path.ptr)); close_temp_repo(repo); @@ -645,7 +644,7 @@ BEGIN_TEST(rename2, "renaming a packed reference does not pack another reference /* Ensure the other reference exists on the file system */ must_pass(git_buf_joinpath(&temp_path, repo->path_repository, packed_test_head_name)); - must_pass(git_path_exists(temp_path.ptr)); + must_be_true(git_path_exists(temp_path.ptr)); /* Lookup the other reference */ must_pass(git_reference_lookup(&another_looked_up_ref, repo, packed_test_head_name)); @@ -670,7 +669,7 @@ BEGIN_TEST(rename2, "renaming a packed reference does not pack another reference must_be_true(git_reference_is_packed(another_looked_up_ref) == 0); /* Ensure the other ref still exists on the file system */ - must_pass(git_path_exists(temp_path.ptr)); + must_be_true(git_path_exists(temp_path.ptr)); close_temp_repo(repo); @@ -857,6 +856,18 @@ BEGIN_TEST(rename8, "can be renamed to a new name prefixed with the old name") END_TEST BEGIN_TEST(rename9, "can move a reference to a upper reference hierarchy") + /* + * I'm killing this test because it adds a shitton of complexity + * to the reference renaming code for an use case which I'd consider + * "clinically stupid". + * + * If somebody shows me a workflow which involves turning a reference + * into a folder with its same name, we'll bring back the ridiculous + * logic. + * + * -Vicent + */ +#if 0 git_reference *ref, *ref_two, *looked_up_ref; git_repository *repo; git_oid id; @@ -888,6 +899,7 @@ BEGIN_TEST(rename9, "can move a reference to a upper reference hierarchy") git_reference_free(looked_up_ref); close_temp_repo(repo); +#endif END_TEST BEGIN_TEST(delete0, "deleting a ref which is both packed and loose should remove both tracks in the filesystem") @@ -899,7 +911,7 @@ BEGIN_TEST(delete0, "deleting a ref which is both packed and loose should remove /* Ensure the loose reference exists on the file system */ must_pass(git_buf_joinpath(&temp_path, repo->path_repository, packed_test_head_name)); - must_pass(git_path_exists(temp_path.ptr)); + must_be_true(git_path_exists(temp_path.ptr)); /* Lookup the reference */ must_pass(git_reference_lookup(&looked_up_ref, repo, packed_test_head_name)); @@ -914,7 +926,7 @@ BEGIN_TEST(delete0, "deleting a ref which is both packed and loose should remove must_fail(git_reference_lookup(&another_looked_up_ref, repo, packed_test_head_name)); /* Ensure the loose reference doesn't exist any longer on the file system */ - must_pass(!git_path_exists(temp_path.ptr)); + must_be_true(!git_path_exists(temp_path.ptr)); close_temp_repo(repo); diff --git a/tests/t12-repo.c b/tests/t12-repo.c index 6a080ecb3..7c45e0126 100644 --- a/tests/t12-repo.c +++ b/tests/t12-repo.c @@ -68,7 +68,7 @@ static int write_file(const char *path, const char *content) int error; git_file file; - if (git_path_exists(path) == GIT_SUCCESS) { + if (git_path_exists(path) == true) { error = p_unlink(path); if (error < GIT_SUCCESS) diff --git a/tests/test_helpers.c b/tests/test_helpers.c index 837358453..9ed0d79d8 100644 --- a/tests/test_helpers.c +++ b/tests/test_helpers.c @@ -239,7 +239,7 @@ static int copy_filesystem_element_recurs(void *_data, git_buf *source) git_buf_truncate(&data->dst, data->dst_baselen); git_buf_puts(&data->dst, source->ptr + data->src_baselen); - if (git_path_isdir(source->ptr) == GIT_SUCCESS) + if (git_path_isdir(source->ptr) == true) return git_path_direach(source, copy_filesystem_element_recurs, _data); else return copy_file(source->ptr, data->dst.ptr); @@ -253,8 +253,8 @@ int copydir_recurs( copydir_data data = { GIT_BUF_INIT, 0, GIT_BUF_INIT, 0 }; /* Source has to exist, Destination hast to _not_ exist */ - if (git_path_isdir(source_directory_path) != GIT_SUCCESS || - git_path_isdir(destination_directory_path) == GIT_SUCCESS) + if (git_path_isdir(source_directory_path) == false || + git_path_isdir(destination_directory_path) == true) return GIT_EINVALIDPATH; git_buf_joinpath(&data.src, source_directory_path, ""); @@ -299,7 +299,7 @@ static int remove_placeholders_recurs(void *_data, git_buf *path) remove_data *data = (remove_data *)_data; size_t pathlen; - if (!git_path_isdir(path->ptr)) + if (git_path_isdir(path->ptr) == true) return git_path_direach(path, remove_placeholders_recurs, data); pathlen = path->size; @@ -322,7 +322,7 @@ int remove_placeholders(const char *directory_path, const char *filename) remove_data data; git_buf buffer = GIT_BUF_INIT; - if (git_path_isdir(directory_path)) + if (git_path_isdir(directory_path) == false) return GIT_EINVALIDPATH; if ((error = git_buf_sets(&buffer, directory_path)) < GIT_SUCCESS) |