From e54d8d8972ddfa886bfcf1a078d253b632debc72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vicent=20Mart=C3=AD?= Date: Wed, 7 Mar 2012 01:37:09 +0100 Subject: [PATCH] error-handling: Config --- include/git2/errors.h | 1 + src/config.c | 194 ++++++++++++++++++++++-------------------- src/fileops.c | 15 ++-- 3 files changed, 112 insertions(+), 98 deletions(-) diff --git a/include/git2/errors.h b/include/git2/errors.h index 7daa0670f..bc420d1d4 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -126,6 +126,7 @@ typedef enum { GITERR_REFERENCE, GITERR_ZLIB, GITERR_REPOSITORY, + GITERR_CONFIG, } git_error_class; #define GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { return -1; } diff --git a/src/config.c b/src/config.c index 912224158..850c9b15f 100644 --- a/src/config.c +++ b/src/config.c @@ -67,77 +67,71 @@ int git_config_new(git_config **out) if (git_vector_init(&cfg->files, 3, config_backend_cmp) < 0) { git__free(cfg); - return GIT_ENOMEM; + return -1; } *out = cfg; GIT_REFCOUNT_INC(cfg); - return GIT_SUCCESS; + return 0; } int git_config_add_file_ondisk(git_config *cfg, const char *path, int priority) { git_config_file *file = NULL; - int error; - error = git_config_file__ondisk(&file, path); - if (error < GIT_SUCCESS) - return error; + if (git_config_file__ondisk(&file, path) < 0) + return -1; - error = git_config_add_file(cfg, file, priority); - if (error < GIT_SUCCESS) { + if (git_config_add_file(cfg, file, priority) < 0) { /* * free manually; the file is not owned by the config * instance yet and will not be freed on cleanup */ file->free(file); - return error; + return -1; } - return GIT_SUCCESS; + return 0; } int git_config_open_ondisk(git_config **cfg, const char *path) { - int error; + if (git_config_new(cfg) < 0) + return -1; - error = git_config_new(cfg); - if (error < GIT_SUCCESS) - return error; - - error = git_config_add_file_ondisk(*cfg, path, 1); - if (error < GIT_SUCCESS) + if (git_config_add_file_ondisk(*cfg, path, 1) < 0) { git_config_free(*cfg); + return -1; + } - return error; + return 0; } int git_config_add_file(git_config *cfg, git_config_file *file, int priority) { file_internal *internal; - int error; + int result; assert(cfg && file); - if ((error = file->open(file)) < GIT_SUCCESS) - return git__throw(error, "Failed to open config file"); + if ((result = file->open(file)) < 0) + return result; internal = git__malloc(sizeof(file_internal)); - if (internal == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(internal); internal->file = file; internal->priority = priority; if (git_vector_insert(&cfg->files, internal) < 0) { git__free(internal); - return GIT_ENOMEM; + return -1; } git_vector_sort(&cfg->files); internal->file->cfg = cfg; - return GIT_SUCCESS; + return 0; } /* @@ -165,8 +159,7 @@ int git_config_delete(git_config *cfg, const char *name) file_internal *internal; git_config_file *file; - if (cfg->files.length == 0) - return git__throw(GIT_EINVALIDARGS, "Cannot delete variable; no files open in the `git_config` instance"); + assert(cfg->files.length); internal = git_vector_get(&cfg->files, 0); file = internal->file; @@ -200,8 +193,7 @@ int git_config_set_string(git_config *cfg, const char *name, const char *value) file_internal *internal; git_config_file *file; - if (cfg->files.length == 0) - return git__throw(GIT_EINVALIDARGS, "Cannot set variable value; no files open in the `git_config` instance"); + assert(cfg->files.length); internal = git_vector_get(&cfg->files, 0); file = internal->file; @@ -239,7 +231,7 @@ static int parse_int64(int64_t *out, const char *value) int64_t num; if (git__strtol64(&num, value, &num_end, 0) < 0) - return GIT_EINVALIDTYPE; + return -1; switch (*num_end) { case 'g': @@ -259,7 +251,7 @@ static int parse_int64(int64_t *out, const char *value) /* check that that there are no more characters after the * given modifier suffix */ if (num_end[1] != '\0') - return GIT_EINVALIDTYPE; + return -1; /* fallthrough */ @@ -268,7 +260,7 @@ static int parse_int64(int64_t *out, const char *value) return 0; default: - return GIT_EINVALIDTYPE; + return -1; } } @@ -278,11 +270,11 @@ static int parse_int32(int32_t *out, const char *value) int32_t truncate; if (parse_int64(&tmp, value) < 0) - return GIT_EINVALIDTYPE; + return -1; truncate = tmp & 0xFFFFFFFF; if (truncate != tmp) - return GIT_EOVERFLOW; + return -1; *out = truncate; return 0; @@ -332,8 +324,9 @@ int git_config_get_mapped(git_config *cfg, const char *name, git_cvar_map *maps, } } - return git__throw(GIT_ENOTFOUND, + giterr_set(GITERR_CONFIG, "Failed to map the '%s' config variable with a valid value", name); + return -1; } int git_config_get_int64(git_config *cfg, const char *name, int64_t *out) @@ -342,69 +335,80 @@ int git_config_get_int64(git_config *cfg, const char *name, int64_t *out) int ret; ret = git_config_get_string(cfg, name, &value); - if (ret < GIT_SUCCESS) - return git__rethrow(ret, "Failed to retrieve value for '%s'", name); + if (ret < 0) + return ret; - if (parse_int64(out, value) < 0) - return git__throw(GIT_EINVALIDTYPE, "Failed to parse '%s' as an integer", value); + if (parse_int64(out, value) < 0) { + giterr_set(GITERR_CONFIG, "Failed to parse '%s' as an integer", value); + return -1; + } - return GIT_SUCCESS; + return 0; } int git_config_get_int32(git_config *cfg, const char *name, int32_t *out) { const char *value; - int error; + int ret; - error = git_config_get_string(cfg, name, &value); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to get value for %s", name); + ret = git_config_get_string(cfg, name, &value); + if (ret < 0) + return ret; - error = parse_int32(out, value); - if (error < GIT_SUCCESS) - return git__throw(GIT_EINVALIDTYPE, "Failed to parse '%s' as a 32-bit integer", value); + if (parse_int32(out, value) < 0) { + giterr_set(GITERR_CONFIG, "Failed to parse '%s' as a 32-bit integer", value); + return -1; + } - return GIT_SUCCESS; + return 0; } int git_config_get_bool(git_config *cfg, const char *name, int *out) { const char *value; - int error = GIT_SUCCESS; + int ret; - error = git_config_get_string(cfg, name, &value); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to get value for %s", name); + ret = git_config_get_string(cfg, name, &value); + if (ret < 0) + return ret; if (parse_bool(out, value) == 0) - return GIT_SUCCESS; + return 0; if (parse_int32(out, value) == 0) { *out = !!(*out); - return GIT_SUCCESS; + return 0; } - return git__throw(GIT_EINVALIDTYPE, "Failed to parse '%s' as a boolean value", value); + giterr_set(GITERR_CONFIG, "Failed to parse '%s' as a boolean value", value); + return -1; } int git_config_get_string(git_config *cfg, const char *name, const char **out) { file_internal *internal; git_config_file *file; - int error = GIT_ENOTFOUND; + int ret = GIT_ENOTFOUND; unsigned int i; - if (cfg->files.length == 0) - return git__throw(GIT_EINVALIDARGS, "Cannot get variable value; no files open in the `git_config` instance"); + assert(cfg->files.length); for (i = 0; i < cfg->files.length; ++i) { internal = git_vector_get(&cfg->files, i); file = internal->file; - if ((error = file->get(file, name, out)) == GIT_SUCCESS) - return GIT_SUCCESS; + + ret = file->get(file, name, out); + if (ret == 0) + return 0; + + if (ret == GIT_ENOTFOUND) + continue; + + return ret; } - return git__throw(error, "Config value '%s' not found", name); + giterr_set(GITERR_CONFIG, "Config value '%s' not found", name); + return GIT_ENOTFOUND; } int git_config_get_multivar(git_config *cfg, const char *name, const char *regexp, @@ -412,12 +416,10 @@ int git_config_get_multivar(git_config *cfg, const char *name, const char *regex { file_internal *internal; git_config_file *file; - int error = GIT_ENOTFOUND; + int ret = GIT_ENOTFOUND; unsigned int i; - - if (cfg->files.length == 0) - return git__throw(GIT_EINVALIDARGS, "Cannot get variable value; no files open in the `git_config` instance"); + assert(cfg->files.length); /* * This loop runs the "wrong" way 'round because we need to @@ -426,30 +428,30 @@ int git_config_get_multivar(git_config *cfg, const char *name, const char *regex for (i = cfg->files.length; i > 0; --i) { internal = git_vector_get(&cfg->files, i - 1); file = internal->file; - error = file->get_multivar(file, name, regexp, fn, data); - if (error < GIT_SUCCESS && error != GIT_ENOTFOUND) - git__rethrow(error, "Failed to get multivar"); + ret = file->get_multivar(file, name, regexp, fn, data); + if (ret < 0 && ret != GIT_ENOTFOUND) + return ret; } - return GIT_SUCCESS; + return 0; } int git_config_set_multivar(git_config *cfg, const char *name, const char *regexp, const char *value) { file_internal *internal; git_config_file *file; - int error = GIT_ENOTFOUND; + int ret = GIT_ENOTFOUND; unsigned int i; for (i = cfg->files.length; i > 0; --i) { internal = git_vector_get(&cfg->files, i - 1); file = internal->file; - error = file->set_multivar(file, name, regexp, value); - if (error < GIT_SUCCESS && error != GIT_ENOTFOUND) - git__rethrow(error, "Failed to replace multivar"); + ret = file->set_multivar(file, name, regexp, value); + if (ret < GIT_SUCCESS && ret != GIT_ENOTFOUND) + return ret; } - return GIT_SUCCESS; + return 0; } int git_config_find_global_r(git_buf *path) @@ -460,18 +462,23 @@ int git_config_find_global_r(git_buf *path) int git_config_find_global(char *global_config_path) { git_buf path = GIT_BUF_INIT; - int error = git_config_find_global_r(&path); + int ret = git_config_find_global_r(&path); - if (error == GIT_SUCCESS) { - if (path.size > GIT_PATH_MAX) - error = git__throw(GIT_ESHORTBUFFER, "Path is too long"); - else - git_buf_copy_cstr(global_config_path, GIT_PATH_MAX, &path); + if (ret < 0) { + git_buf_free(&path); + return ret; } - git_buf_free(&path); + if (path.size > GIT_PATH_MAX) { + git_buf_free(&path); + giterr_set(GITERR_NOMEMORY, + "Path is to long to fit on the given buffer"); + return -1; + } - return error; + git_buf_copy_cstr(global_config_path, GIT_PATH_MAX, &path); + git_buf_free(&path); + return 0; } int git_config_find_system_r(git_buf *path) @@ -482,18 +489,23 @@ int git_config_find_system_r(git_buf *path) int git_config_find_system(char *system_config_path) { git_buf path = GIT_BUF_INIT; - int error = git_config_find_system_r(&path); + int ret = git_config_find_system_r(&path); - if (error == GIT_SUCCESS) { - if (path.size > GIT_PATH_MAX) - error = git__throw(GIT_ESHORTBUFFER, "Path is too long"); - else - git_buf_copy_cstr(system_config_path, GIT_PATH_MAX, &path); + if (ret < 0) { + git_buf_free(&path); + return ret; } - git_buf_free(&path); + if (path.size > GIT_PATH_MAX) { + git_buf_free(&path); + giterr_set(GITERR_NOMEMORY, + "Path is to long to fit on the given buffer"); + return -1; + } - return error; + git_buf_copy_cstr(system_config_path, GIT_PATH_MAX, &path); + git_buf_free(&path); + return 0; } int git_config_open_global(git_config **out) @@ -501,7 +513,7 @@ int git_config_open_global(git_config **out) int error; char global_path[GIT_PATH_MAX]; - if ((error = git_config_find_global(global_path)) < GIT_SUCCESS) + if ((error = git_config_find_global(global_path)) < 0) return error; return git_config_open_ondisk(out, global_path); diff --git a/src/fileops.c b/src/fileops.c index 4414c86c6..c6a3be73d 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -295,7 +295,6 @@ int git_futils_rmdir_r(const char *path, int force) int git_futils_find_global_file(git_buf *path, const char *filename) { - int error; const char *home = getenv("HOME"); #ifdef GIT_WIN32 @@ -303,19 +302,21 @@ int git_futils_find_global_file(git_buf *path, const char *filename) home = getenv("USERPROFILE"); #endif - if (home == NULL) - return git__throw(GIT_EOSERR, "Failed to open global %s file. " - "Cannot locate the user's home directory.", filename); + if (home == NULL) { + giterr_set(GITERR_OS, "Global file lookup failed. " + "Cannot locate the user's home directory"); + return -1; + } - if ((error = git_buf_joinpath(path, home, filename)) < GIT_SUCCESS) - return error; + if (git_buf_joinpath(path, home, filename) < 0) + return -1; if (git_path_exists(path->ptr) == false) { git_buf_clear(path); return GIT_ENOTFOUND; } - return GIT_SUCCESS; + return 0; } #ifdef GIT_WIN32