From dda708e78f3c3f43d814d46c29ab9f2b9d47ed5c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vicent=20Mart=C3=AD?= Date: Fri, 9 Mar 2012 19:55:50 +0100 Subject: [PATCH] error-handling: On-disk config file backend Includes: - Proper error reporting when encountering syntax errors in a config file (file, line number, column). - Rewritten `config_write`, now with 99% less goto-spaghetti - Error state in `git_filebuf`: filebuf write functions no longer need to be checked for error returns. If any of the writes performed on a buffer fail, the last call to `git_filebuf_commit` or `git_filebuf_hash` will fail accordingly and set the appropiate error message. Baller! --- include/git2/errors.h | 7 +- src/common.h | 12 + src/config.c | 4 +- src/config_file.c | 707 +++++++++++++++++++----------------------- src/errors.c | 28 +- src/filebuf.c | 150 ++++++--- src/filebuf.h | 20 +- src/hashtable.c | 4 +- src/posix.c | 18 +- src/util.h | 16 +- 10 files changed, 480 insertions(+), 486 deletions(-) diff --git a/include/git2/errors.h b/include/git2/errors.h index bc420d1d4..085dd52f0 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -127,14 +127,9 @@ typedef enum { GITERR_ZLIB, GITERR_REPOSITORY, GITERR_CONFIG, + GITERR_REGEX, } git_error_class; -#define GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { return -1; } - -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 * that occurred in the library. diff --git a/src/common.h b/src/common.h index 0b4dc2e49..30757de70 100644 --- a/src/common.h +++ b/src/common.h @@ -46,6 +46,8 @@ #include "thread-utils.h" #include "bswap.h" +#include + extern void git___throw(const char *, ...) GIT_FORMAT_PRINTF(1, 2); #define git__throw(error, ...) \ (git___throw(__VA_ARGS__), error) @@ -54,6 +56,16 @@ extern void git___rethrow(const char *, ...) GIT_FORMAT_PRINTF(1, 2); #define git__rethrow(error, ...) \ (git___rethrow(__VA_ARGS__), error) + +#define GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { return -1; } + +void giterr_set_oom(void); +void giterr_set(int error_class, const char *string, ...); +void giterr_clear(void); +void giterr_set_str(int error_class, const char *string); +void giterr_set_regex(const regex_t *regex, int error_code); + + #include "util.h" diff --git a/src/config.c b/src/config.c index 850c9b15f..77598d6a6 100644 --- a/src/config.c +++ b/src/config.c @@ -401,13 +401,15 @@ int git_config_get_string(git_config *cfg, const char *name, const char **out) if (ret == 0) return 0; + /* File backend doesn't set error message on variable + * not found */ if (ret == GIT_ENOTFOUND) continue; return ret; } - giterr_set(GITERR_CONFIG, "Config value '%s' not found", name); + giterr_set(GITERR_CONFIG, "Config variable '%s' not found", name); return GIT_ENOTFOUND; } diff --git a/src/config_file.c b/src/config_file.c index 3c7c593ec..077e2c03f 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -86,6 +86,12 @@ static int config_parse(diskfile_backend *cfg_file); static int parse_variable(diskfile_backend *cfg, char **var_name, char **var_value); static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char *value); +static void set_parse_error(diskfile_backend *backend, int col, const char *error_str) +{ + giterr_set(GITERR_CONFIG, "Failed to parse config file: %s (in %s:%d, column %d)", + error_str, backend->file_path, backend->reader.line_number, col); +} + static void cvar_free(cvar_t *var) { if (var == NULL) @@ -104,15 +110,16 @@ static int normalize_name(const char *in, char **out) assert(in && out); name = git__strdup(in); - if (name == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(name); fdot = strchr(name, '.'); ldot = strrchr(name, '.'); if (fdot == NULL || ldot == NULL) { git__free(name); - return git__throw(GIT_EINVALIDARGS, "Bad format. No dot in '%s'", in); + giterr_set(GITERR_CONFIG, + "Invalid variable name: '%s'", in); + return -1; } /* Downcase up to the first dot and after the last one */ @@ -120,7 +127,7 @@ static int normalize_name(const char *in, char **out) git__strtolower(ldot); *out = name; - return GIT_SUCCESS; + return 0; } static void free_vars(git_hashtable *values) @@ -143,37 +150,28 @@ static void free_vars(git_hashtable *values) static int config_open(git_config_file *cfg) { - int error; + int res; diskfile_backend *b = (diskfile_backend *)cfg; b->values = git_hashtable_alloc (20, git_hash__strhash_cb, git_hash__strcmp_cb); - if (b->values == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(b->values); git_buf_init(&b->reader.buffer, 0); - error = git_futils_readbuffer(&b->reader.buffer, b->file_path); + res = git_futils_readbuffer(&b->reader.buffer, b->file_path); /* It's fine if the file doesn't exist */ - if (error == GIT_ENOTFOUND) - return GIT_SUCCESS; - - if (error < GIT_SUCCESS) - goto cleanup; - - error = config_parse(b); - if (error < GIT_SUCCESS) - goto cleanup; - - git_buf_free(&b->reader.buffer); - - return GIT_SUCCESS; + if (res == GIT_ENOTFOUND) + return 0; + + if (res < 0 || config_parse(b) < 0) { + free_vars(b->values); + b->values = NULL; + git_buf_free(&b->reader.buffer); + return -1; + } - cleanup: - free_vars(b->values); - b->values = NULL; git_buf_free(&b->reader.buffer); - - return git__rethrow(error, "Failed to open config"); + return 0; } static void backend_free(git_config_file *_backend) @@ -184,42 +182,37 @@ static void backend_free(git_config_file *_backend) return; git__free(backend->file_path); - free_vars(backend->values); - git__free(backend); } static int file_foreach(git_config_file *backend, int (*fn)(const char *, const char *, void *), void *data) { - int ret = GIT_SUCCESS; - cvar_t *var; diskfile_backend *b = (diskfile_backend *)backend; + cvar_t *var; const char *key; GIT_HASHTABLE_FOREACH(b->values, key, var, - do { - ret = fn(key, var->value, data); - if (ret) - break; - var = CVAR_LIST_NEXT(var); - } while (var != NULL); - ) + do { + if (fn(key, var->value, data) < 0) + break; + var = CVAR_LIST_NEXT(var); + } while (var != NULL); + ) - return ret; + return 0; } static int config_set(git_config_file *cfg, const char *name, const char *value) { cvar_t *var = NULL; cvar_t *existing = NULL, *old_value = NULL; - int error = GIT_SUCCESS; diskfile_backend *b = (diskfile_backend *)cfg; char *key; - if ((error = normalize_name(name, &key)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to normalize variable name '%s'", name); + if (normalize_name(name, &key) < 0) + return -1; /* * Try to find it in the existing values and update it if it @@ -227,15 +220,18 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) */ existing = git_hashtable_lookup(b->values, key); if (existing != NULL) { - char *tmp; + char *tmp = NULL; git__free(key); - if (existing->next != NULL) - return git__throw(GIT_EINVALIDARGS, "Multivar incompatible with simple set"); + if (existing->next != NULL) { + giterr_set(GITERR_CONFIG, "Multivar incompatible with simple set"); + return -1; + } - tmp = value ? git__strdup(value) : NULL; - if (tmp == NULL && value != NULL) - return GIT_ENOMEM; + if (value) { + tmp = git__strdup(value); + GITERR_CHECK_ALLOC(tmp); + } git__free(existing->value); existing->value = tmp; @@ -244,32 +240,29 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) } var = git__malloc(sizeof(cvar_t)); - if (var == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(var); memset(var, 0x0, sizeof(cvar_t)); var->key = key; + var->value = NULL; - var->value = value ? git__strdup(value) : NULL; - if (var->value == NULL && value != NULL) { - error = GIT_ENOMEM; - goto out; + if (value) { + var->value = git__strdup(value); + GITERR_CHECK_ALLOC(var->value); } - error = git_hashtable_insert2(b->values, key, var, (void **)&old_value); - if (error < GIT_SUCCESS) - goto out; + if (git_hashtable_insert2(b->values, key, var, (void **)&old_value) < 0) + return -1; cvar_free(old_value); - error = config_write(b, key, NULL, value); - - out: - if (error < GIT_SUCCESS) + if (config_write(b, key, NULL, value) < 0) { cvar_free(var); + return -1; + } - return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to set config value"); + return 0; } /* @@ -278,171 +271,172 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) static int config_get(git_config_file *cfg, const char *name, const char **out) { cvar_t *var; - int error = GIT_SUCCESS; diskfile_backend *b = (diskfile_backend *)cfg; char *key; - if ((error = normalize_name(name, &key)) < GIT_SUCCESS) - return error; + if (normalize_name(name, &key) < 0) + return -1; var = git_hashtable_lookup(b->values, key); git__free(key); + /* no error message; the config system will write one */ if (var == NULL) - return git__throw(GIT_ENOTFOUND, "Variable '%s' not found", name); + return GIT_ENOTFOUND; *out = var->value; - - return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to get config value for %s", name); + return 0; } -static int config_get_multivar(git_config_file *cfg, const char *name, const char *regexp, int (*fn)(const char *, void *), void *data) +static int config_get_multivar( + git_config_file *cfg, + const char *name, + const char *regex_str, + int (*fn)(const char *, void *), + void *data) { cvar_t *var; - int error = GIT_SUCCESS; diskfile_backend *b = (diskfile_backend *)cfg; char *key; - regex_t preg; - if ((error = normalize_name(name, &key)) < GIT_SUCCESS) - return error; + if (normalize_name(name, &key) < 0) + return -1; var = git_hashtable_lookup(b->values, key); git__free(key); if (var == NULL) - return git__throw(GIT_ENOTFOUND, "Variable '%s' not found", name); + return GIT_ENOTFOUND; - if (regexp != NULL) { - error = regcomp(&preg, regexp, REG_EXTENDED); - if (error < 0) - return git__throw(GIT_EINVALIDARGS, "Failed to compile regex"); - } + if (regex_str != NULL) { + regex_t regex; + int result; - do { - if (regexp == NULL || !regexec(&preg, var->value, 0, NULL, 0)) { - error = fn(var->value, data); - if (error < GIT_SUCCESS) - goto exit; + /* regex matching; build the regex */ + result = regcomp(®ex, regex_str, REG_EXTENDED); + if (result < 0) { + giterr_set_regex(®ex, result); + return -1; } - var = var->next; - } while (var != NULL); + /* and throw the callback only on the variables that + * match the regex */ + do { + if (regexec(®ex, var->value, 0, NULL, 0) == 0) { + /* early termination by the user is not an error; + * just break and return successfully */ + if (fn(var->value, data) < 0) + break; + } - exit: - if (regexp != NULL) - regfree(&preg); - return error; + var = var->next; + } while (var != NULL); + } else { + /* no regex; go through all the variables */ + do { + /* early termination by the user is not an error; + * just break and return successfully */ + if (fn(var->value, data) < 0) + break; + + var = var->next; + } while (var != NULL); + } + + return 0; } static int config_set_multivar(git_config_file *cfg, const char *name, const char *regexp, const char *value) { - int error, replaced = 0; + int replaced = 0; cvar_t *var, *newvar; diskfile_backend *b = (diskfile_backend *)cfg; char *key; regex_t preg; + int result; - if (regexp == NULL) - return git__throw(GIT_EINVALIDARGS, "No regex supplied"); + assert(regexp); - if ((error = normalize_name(name, &key)) < GIT_SUCCESS) - return error; + if (normalize_name(name, &key) < 0) + return -1; var = git_hashtable_lookup(b->values, key); - if (var == NULL) { - free(key); - return git__throw(GIT_ENOTFOUND, "Variable '%s' not found", name); - } + if (var == NULL) + return GIT_ENOTFOUND; - error = regcomp(&preg, regexp, REG_EXTENDED); - if (error < 0) { + result = regcomp(&preg, regexp, REG_EXTENDED); + if (result < 0) { free(key); - return git__throw(GIT_EINVALIDARGS, "Failed to compile regex"); + giterr_set_regex(&preg, result); + return -1; } - do { - if (!regexec(&preg, var->value, 0, NULL, 0)) { + for (;;) { + if (regexec(&preg, var->value, 0, NULL, 0) == 0) { char *tmp = git__strdup(value); - if (tmp == NULL) { - error = GIT_ENOMEM; - goto exit; - } + GITERR_CHECK_ALLOC(tmp); free(var->value); var->value = tmp; replaced = 1; } - if (var->next != NULL) - var = var->next; - else + if (var->next == NULL) break; - } while (var != NULL); + + var = var->next; + } /* If we've reached the end of the variables and we haven't found it yet, we need to append it */ if (!replaced) { newvar = git__malloc(sizeof(cvar_t)); - if (newvar == NULL) { - error = GIT_ENOMEM; - goto exit; - } + GITERR_CHECK_ALLOC(newvar); memset(newvar, 0x0, sizeof(cvar_t)); + newvar->key = git__strdup(var->key); - if (newvar->key == NULL) { - error = GIT_ENOMEM; - goto exit; - } + GITERR_CHECK_ALLOC(newvar->key); + newvar->value = git__strdup(value); - if (newvar->value == NULL) { - error = GIT_ENOMEM; - goto exit; - } + GITERR_CHECK_ALLOC(newvar->value); var->next = newvar; } - error = config_write(b, key, &preg, value); - if (error < GIT_SUCCESS) { - error = git__rethrow(error, "Failed to update value in file"); - goto exit; - } + result = config_write(b, key, &preg, value); - exit: free(key); regfree(&preg); - return error; + + return result; } static int config_delete(git_config_file *cfg, const char *name) { - int error; - const cvar_t *var; - cvar_t *old_value; + cvar_t *var; diskfile_backend *b = (diskfile_backend *)cfg; char *key; + int result; - if ((error = normalize_name(name, &key)) < GIT_SUCCESS) - return error; + if (normalize_name(name, &key) < 0) + return -1; var = git_hashtable_lookup(b->values, key); free(key); if (var == NULL) - return git__throw(GIT_ENOTFOUND, "Variable '%s' not found", name); - - if (var->next != NULL) - return git__throw(GIT_EINVALIDARGS, "Multivar incompatible with simple delete"); + return GIT_ENOTFOUND; + if (var->next != NULL) { + giterr_set(GITERR_CONFIG, "Cannot delete multivar with a single delete"); + return -1; + } - if ((error = git_hashtable_remove2(b->values, var->key, (void **)&old_value)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to remove %s from hashtable", key); - - error = config_write(b, var->key, NULL, NULL); - cvar_free(old_value); + git_hashtable_remove(b->values, var->key); + result = config_write(b, var->key, NULL, NULL); - return error; + cvar_free(var); + return result; } int git_config_file__ondisk(git_config_file **out, const char *path) @@ -450,16 +444,12 @@ int git_config_file__ondisk(git_config_file **out, const char *path) diskfile_backend *backend; backend = git__malloc(sizeof(diskfile_backend)); - if (backend == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(backend); memset(backend, 0x0, sizeof(diskfile_backend)); backend->file_path = git__strdup(path); - if (backend->file_path == NULL) { - git__free(backend); - return GIT_ENOMEM; - } + GITERR_CHECK_ALLOC(backend->file_path); backend->parent.open = config_open; backend->parent.get = config_get; @@ -472,7 +462,7 @@ int git_config_file__ondisk(git_config_file **out, const char *path) *out = (git_config_file *)backend; - return GIT_SUCCESS; + return 0; } static int cfg_getchar_raw(diskfile_backend *cfg) @@ -621,12 +611,11 @@ GIT_INLINE(int) config_keychar(int c) return isalnum(c) || c == '-'; } -static int parse_section_header_ext(const char *line, const char *base_name, char **section_name) +static int parse_section_header_ext(diskfile_backend *cfg, const char *line, const char *base_name, char **section_name) { int c, rpos; char *first_quote, *last_quote; git_buf buf = GIT_BUF_INIT; - int error = GIT_SUCCESS; int quote_marks; /* * base_name is what came before the space. We should be at the @@ -637,8 +626,10 @@ static int parse_section_header_ext(const char *line, const char *base_name, cha first_quote = strchr(line, '"'); last_quote = strrchr(line, '"'); - if (last_quote - first_quote == 0) - return git__throw(GIT_EOBJCORRUPTED, "Failed to parse ext header. There is no final quotation mark"); + if (last_quote - first_quote == 0) { + set_parse_error(cfg, 0, "Missing closing quotation mark in section header"); + return -1; + } git_buf_grow(&buf, strlen(base_name) + last_quote - first_quote + 2); git_buf_printf(&buf, "%s.", base_name); @@ -655,27 +646,30 @@ static int parse_section_header_ext(const char *line, const char *base_name, cha */ do { if (quote_marks == 2) { - puts("too many marks"); - error = git__throw(GIT_EOBJCORRUPTED, "Falied to parse ext header. Text after closing quote"); - goto out; - + set_parse_error(cfg, rpos, "Unexpected text after closing quotes"); + git_buf_free(&buf); + return -1; } switch (c) { case '"': ++quote_marks; continue; + case '\\': c = line[rpos++]; + switch (c) { case '"': case '\\': break; + default: - error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse ext header. Unsupported escape char \\%c", c); - goto out; + set_parse_error(cfg, rpos, "Unsupported escape sequence"); + git_buf_free(&buf); + return -1; } - break; + default: break; } @@ -683,61 +677,53 @@ static int parse_section_header_ext(const char *line, const char *base_name, cha git_buf_putc(&buf, c); } while ((c = line[rpos++]) != ']'); - *section_name = git__strdup(git_buf_cstr(&buf)); - out: - git_buf_free(&buf); - - return error; + *section_name = git_buf_detach(&buf); + return 0; } static int parse_section_header(diskfile_backend *cfg, char **section_out) { char *name, *name_end; int name_length, c, pos; - int error = GIT_SUCCESS; + int result; char *line; line = cfg_readline(cfg); if (line == NULL) - return GIT_ENOMEM; + return -1; /* find the end of the variable's name */ name_end = strchr(line, ']'); if (name_end == NULL) { git__free(line); - return git__throw(GIT_EOBJCORRUPTED, "Failed to parse header. Can't find header name end"); + set_parse_error(cfg, 0, "Missing ']' in section header"); + return -1; } name = (char *)git__malloc((size_t)(name_end - line) + 1); - if (name == NULL) { - git__free(line); - return GIT_ENOMEM; - } + GITERR_CHECK_ALLOC(name); name_length = 0; pos = 0; /* Make sure we were given a section header */ c = line[pos++]; - if (c != '[') { - error = git__throw(GIT_ERROR, "Failed to parse header. Didn't get section header. This is a bug"); - goto error; - } + assert(c == '['); c = line[pos++]; do { if (isspace(c)){ name[name_length] = '\0'; - error = parse_section_header_ext(line, name, section_out); + result = parse_section_header_ext(cfg, line, name, section_out); git__free(line); git__free(name); - return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to parse header"); + return result; } if (!config_keychar(c) && c != '.') { - error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse header. Wrong format on header"); - goto error; + set_parse_error(cfg, pos, "Unexpected character in header"); + goto fail_parse; } name[name_length++] = (char) tolower(c); @@ -745,20 +731,21 @@ static int parse_section_header(diskfile_backend *cfg, char **section_out) } while ((c = line[pos++]) != ']'); if (line[pos - 1] != ']') { - error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse header. Config file ended unexpectedly"); - goto error; + set_parse_error(cfg, pos, "Unexpected end of file"); + goto fail_parse; } - name[name_length] = 0; git__free(line); - git__strtolower(name); + + name[name_length] = 0; *section_out = name; - return GIT_SUCCESS; -error: + return 0; + +fail_parse: git__free(line); git__free(name); - return error; + return -1; } static int skip_bom(diskfile_backend *cfg) @@ -766,7 +753,7 @@ static int skip_bom(diskfile_backend *cfg) static const char utf8_bom[] = "\xef\xbb\xbf"; if (cfg->reader.buffer.size < sizeof(utf8_bom)) - return GIT_SUCCESS; + return 0; if (memcmp(cfg->reader.read_ptr, utf8_bom, sizeof(utf8_bom)) == 0) cfg->reader.read_ptr += sizeof(utf8_bom); @@ -775,7 +762,7 @@ static int skip_bom(diskfile_backend *cfg) shit with the BoM */ - return GIT_SUCCESS; + return 0; } /* @@ -839,12 +826,13 @@ static void strip_comments(char *line) static int config_parse(diskfile_backend *cfg_file) { - int error = GIT_SUCCESS, c; + int c; char *current_section = NULL; char *var_name; char *var_value; cvar_t *var, *existing; git_buf buf = GIT_BUF_INIT; + int result = 0; /* Initialize the reading position */ cfg_file->reader.read_ptr = cfg_file->reader.buffer.ptr; @@ -852,11 +840,11 @@ static int config_parse(diskfile_backend *cfg_file) /* If the file is empty, there's nothing for us to do */ if (*cfg_file->reader.read_ptr == '\0') - return GIT_SUCCESS; + return 0; skip_bom(cfg_file); - while (error == GIT_SUCCESS && !cfg_file->reader.eof) { + while (result == 0 && !cfg_file->reader.eof) { c = cfg_peek(cfg_file, SKIP_WHITESPACE); @@ -868,7 +856,7 @@ static int config_parse(diskfile_backend *cfg_file) case '[': /* section header, new section begins */ git__free(current_section); current_section = NULL; - error = parse_section_header(cfg_file, ¤t_section); + result = parse_section_header(cfg_file, ¤t_section); break; case ';': @@ -877,16 +865,12 @@ static int config_parse(diskfile_backend *cfg_file) break; default: /* assume variable declaration */ - error = parse_variable(cfg_file, &var_name, &var_value); - - if (error < GIT_SUCCESS) + result = parse_variable(cfg_file, &var_name, &var_value); + if (result < 0) break; var = git__malloc(sizeof(cvar_t)); - if (var == NULL) { - error = GIT_ENOMEM; - break; - } + GITERR_CHECK_ALLOC(var); memset(var, 0x0, sizeof(cvar_t)); @@ -894,10 +878,8 @@ static int config_parse(diskfile_backend *cfg_file) git_buf_printf(&buf, "%s.%s", current_section, var_name); git__free(var_name); - if (git_buf_oom(&buf)) { - error = GIT_ENOMEM; - break; - } + if (git_buf_oom(&buf)) + return -1; var->key = git_buf_detach(&buf); var->value = var_value; @@ -905,7 +887,7 @@ static int config_parse(diskfile_backend *cfg_file) /* Add or append the new config option */ existing = git_hashtable_lookup(cfg_file->values, var->key); if (existing == NULL) { - error = git_hashtable_insert(cfg_file->values, var->key, var); + result = git_hashtable_insert(cfg_file->values, var->key, var); } else { while (existing->next != NULL) { existing = existing->next; @@ -918,13 +900,12 @@ static int config_parse(diskfile_backend *cfg_file) } git__free(current_section); - - return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to parse config"); + return result; } static int write_section(git_filebuf *file, const char *key) { - int error; + int result; const char *fdot, *ldot; git_buf buf = GIT_BUF_INIT; @@ -943,13 +924,14 @@ static int write_section(git_filebuf *file, const char *key) git_buf_putc(&buf, '"'); } git_buf_puts(&buf, "]\n"); + if (git_buf_oom(&buf)) - return GIT_ENOMEM; + return -1; - error = git_filebuf_write(file, git_buf_cstr(&buf), buf.size); + result = git_filebuf_write(file, git_buf_cstr(&buf), buf.size); git_buf_free(&buf); - return error; + return result; } /* @@ -957,50 +939,45 @@ static int write_section(git_filebuf *file, const char *key) */ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char* value) { - int error = GIT_SUCCESS, c; - int section_matches = 0, last_section_matched = 0, preg_replaced = 0; + int result, c; + int section_matches = 0, last_section_matched = 0, preg_replaced = 0, write_trailer = 0; + const char *pre_end = NULL, *post_start = NULL, *data_start; char *current_section = NULL, *section, *name, *ldot; - char *var_name, *var_value; git_filebuf file = GIT_FILEBUF_INIT; - const char *pre_end = NULL, *post_start = NULL, *data_start; /* We need to read in our own config file */ - error = git_futils_readbuffer(&cfg->reader.buffer, cfg->file_path); - if (error < GIT_SUCCESS && error != GIT_ENOTFOUND) { - return git__rethrow(error, "Failed to read existing config file %s", cfg->file_path); - } + result = git_futils_readbuffer(&cfg->reader.buffer, cfg->file_path); /* Initialise the reading position */ - if (error == GIT_ENOTFOUND) { - error = GIT_SUCCESS; + if (result == GIT_ENOTFOUND) { cfg->reader.read_ptr = NULL; cfg->reader.eof = 1; data_start = NULL; git_buf_clear(&cfg->reader.buffer); - } else { + } else if (result == 0) { cfg->reader.read_ptr = cfg->reader.buffer.ptr; cfg->reader.eof = 0; data_start = cfg->reader.read_ptr; + } else { + return -1; /* OS error when reading the file */ } /* Lock the file */ - error = git_filebuf_open(&file, cfg->file_path, 0); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to lock config file"); + if (git_filebuf_open(&file, cfg->file_path, 0) < 0) + return -1; skip_bom(cfg); ldot = strrchr(key, '.'); name = ldot + 1; section = git__strndup(key, ldot - key); - while (error == GIT_SUCCESS && !cfg->reader.eof) { + while (!cfg->reader.eof) { c = cfg_peek(cfg, SKIP_WHITESPACE); - switch (c) { - case '\0': /* We've arrived at the end of the file */ + if (c == '\0') { /* We've arrived at the end of the file */ break; - case '[': /* section header, new section begins */ + } else if (c == '[') { /* section header, new section begins */ /* * We set both positions to the current one in case we * need to add a variable to the end of a section. In that @@ -1009,23 +986,21 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p * default case will take care of updating them. */ pre_end = post_start = cfg->reader.read_ptr; - if (current_section) - git__free(current_section); - error = parse_section_header(cfg, ¤t_section); - if (error < GIT_SUCCESS) - break; + + git__free(current_section); + if (parse_section_header(cfg, ¤t_section) < 0) + goto rewrite_fail; /* Keep track of when it stops matching */ last_section_matched = section_matches; section_matches = !strcmp(current_section, section); - break; + } - case ';': - case '#': + else if (c == ';' || c == '#') { cfg_consume_line(cfg); - break; + } - default: + else { /* * If the section doesn't match, but the last section did, * it means we need to add a variable (so skip the line @@ -1039,67 +1014,54 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p if (!section_matches) { if (!last_section_matched) { cfg_consume_line(cfg); - break; + continue; } } else { - int cmp = -1; + int has_matched = 0; + char *var_name, *var_value; pre_end = cfg->reader.read_ptr; - if ((error = parse_variable(cfg, &var_name, &var_value)) == GIT_SUCCESS) - cmp = strcasecmp(name, var_name); + if (parse_variable(cfg, &var_name, &var_value) < 0) + goto rewrite_fail; - if (cmp == 0 && preg != NULL) - cmp = regexec(preg, var_value, 0, NULL, 0); + /* First try to match the name of the variable */ + if (strcasecmp(name, var_name) == 0) + has_matched = 1; + + /* If the name matches, and we have a regex to match the + * value, try to match it */ + if (has_matched && preg != NULL) + has_matched = (regexec(preg, var_value, 0, NULL, 0) == 0); git__free(var_name); git__free(var_value); - if (cmp != 0) - break; + /* if there is no match, keep going */ + if (!has_matched) + continue; post_start = cfg->reader.read_ptr; } - /* - * We've found the variable we wanted to change, so - * write anything up to it - */ + /* We've found the variable we wanted to change, so + * write anything up to it */ + git_filebuf_write(&file, data_start, pre_end - data_start); preg_replaced = 1; - error = git_filebuf_write(&file, data_start, pre_end - data_start); - if (error < GIT_SUCCESS) { - git__rethrow(error, "Failed to write the first part of the file"); - break; - } - /* - * Then replace the variable. If the value is NULL, it - * means we want to delete it, so pretend everything went - * fine - */ - if (value == NULL) - error = GIT_SUCCESS; - else - error = git_filebuf_printf(&file, "\t%s = %s\n", name, value); - if (error < GIT_SUCCESS) { - git__rethrow(error, "Failed to overwrite the variable"); - break; + /* Then replace the variable. If the value is NULL, it + * means we want to delete it, so don't write anything. */ + if (value != NULL) { + git_filebuf_printf(&file, "\t%s = %s\n", name, value); } + /* multiline variable? we need to keep reading lines to match */ if (preg != NULL) { data_start = post_start; continue; } - /* And then the write out rest of the file */ - error = git_filebuf_write(&file, post_start, - cfg->reader.buffer.size - (post_start - data_start)); - - if (error < GIT_SUCCESS) { - git__rethrow(error, "Failed to write the rest of the file"); - break; - } - - goto cleanup; + write_trailer = 1; + break; /* break from the loop */ } } @@ -1119,132 +1081,108 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p * want to write the rest of the file. Otherwise we need to write * out the whole file and then the new variable. */ - if (preg_replaced) { - error = git_filebuf_printf(&file, "\n%s", data_start); - if (error < GIT_SUCCESS) - error = git__rethrow(error, "Failed to write the rest of the file"); - - goto cleanup; - } + if (write_trailer) { + /* Write out rest of the file */ + git_filebuf_write(&file, post_start, cfg->reader.buffer.size - (post_start - data_start)); + } else { + if (preg_replaced) { + git_filebuf_printf(&file, "\n%s", data_start); + } else { + git_filebuf_write(&file, cfg->reader.buffer.ptr, cfg->reader.buffer.size); + + /* And now if we just need to add a variable */ + if (!section_matches && write_section(&file, section) < 0) + goto rewrite_fail; + + /* Sanity check: if we are here, and value is NULL, that means that somebody + * touched the config file after our intial read. We should probably assert() + * this, but instead we'll handle it gracefully with an error. */ + if (value == NULL) { + giterr_set(GITERR_CONFIG, + "Race condition when writing a config file (a cvar has been removed)"); + goto rewrite_fail; + } - error = git_filebuf_write(&file, cfg->reader.buffer.ptr, cfg->reader.buffer.size); - if (error < GIT_SUCCESS) { - git__rethrow(error, "Failed to write original config content"); - goto cleanup; + git_filebuf_printf(&file, "\t%s = %s\n", name, value); + } } - /* And now if we just need to add a variable */ - if (section_matches) { - error = git_filebuf_printf(&file, "\t%s = %s\n", name, value); - goto cleanup; - } + git__free(section); + git__free(current_section); - /* Or maybe we need to write out a whole section */ - error = write_section(&file, section); - if (error < GIT_SUCCESS) - git__rethrow(error, "Failed to write new section"); + result = git_filebuf_commit(&file, GIT_CONFIG_FILE_MODE); + git_buf_free(&cfg->reader.buffer); + return result; - error = git_filebuf_printf(&file, "\t%s = %s\n", name, value); - cleanup: +rewrite_fail: git__free(section); git__free(current_section); - if (error < GIT_SUCCESS) - git_filebuf_cleanup(&file); - else - error = git_filebuf_commit(&file, GIT_CONFIG_FILE_MODE); - + git_filebuf_cleanup(&file); git_buf_free(&cfg->reader.buffer); - return error; + return -1; } static int is_multiline_var(const char *str) { - char *end = strrchr(str, '\0') - 1; - - while (isspace(*end)) - --end; - - return *end == '\\'; + const char *end = str + strlen(str); + return (end > str) && (end[-1] == '\\'); } -static int parse_multiline_variable(diskfile_backend *cfg, const char *first, char **out) +static int parse_multiline_variable(diskfile_backend *cfg, git_buf *value) { - char *line = NULL, *end; - int error = GIT_SUCCESS, ret; - size_t len; - char *buf; + char *line = NULL; /* Check that the next line exists */ line = cfg_readline(cfg); if (line == NULL) - return GIT_ENOMEM; + return -1; /* We've reached the end of the file, there is input missing */ if (line[0] == '\0') { - error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse multiline var. File ended unexpectedly"); - goto out; + set_parse_error(cfg, 0, "Unexpected end of file while parsing multine var"); + git__free(line); + return -1; } strip_comments(line); /* If it was just a comment, pretend it didn't exist */ if (line[0] == '\0') { - error = parse_multiline_variable(cfg, first, out); - goto out; + git__free(line); + return parse_multiline_variable(cfg, value); + /* TODO: unbounded recursion. This **could** be exploitable */ } - /* Find the continuation character '\' and strip the whitespace */ - end = strrchr(first, '\\'); - while (isspace(end[-1])) - --end; - - *end = '\0'; /* Terminate the string here */ + /* Drop the continuation character '\': to closely follow the UNIX + * standard, this character **has** to be last one in the buf, with + * no whitespace after it */ + assert(is_multiline_var(value->ptr)); + git_buf_truncate(value, value->size - 1); - len = strlen(first) + strlen(line) + 2; - buf = git__malloc(len); - if (buf == NULL) { - error = GIT_ENOMEM; - goto out; - } - - ret = p_snprintf(buf, len, "%s %s", first, line); - if (ret < 0) { - error = git__throw(GIT_EOSERR, "Failed to parse multiline var. Failed to put together two lines. OS err: %s", strerror(errno)); - git__free(buf); - goto out; - } + /* add this line to the multiline var */ + git_buf_puts(value, line); + git__free(line); /* - * If we need to continue reading the next line, pretend - * everything we've read up to now was in one line and call - * ourselves. + * If we need to continue reading the next line, let's just + * keep putting stuff in the buffer */ - if (is_multiline_var(buf)) { - char *final_val; - error = parse_multiline_variable(cfg, buf, &final_val); - git__free(buf); - buf = final_val; - } - - *out = buf; + if (is_multiline_var(value->ptr)) + return parse_multiline_variable(cfg, value); - out: - git__free(line); - return error; + return 0; } static int parse_variable(diskfile_backend *cfg, char **var_name, char **var_value) { - char *tmp; - int error = GIT_SUCCESS; const char *var_end = NULL; const char *value_start = NULL; char *line; line = cfg_readline(cfg); if (line == NULL) - return GIT_ENOMEM; + return -1; strip_comments(line); @@ -1260,52 +1198,39 @@ static int parse_variable(diskfile_backend *cfg, char **var_name, char **var_val while (isspace(var_end[0])); } - tmp = git__strndup(line, var_end - line + 1); - if (tmp == NULL) { - error = GIT_ENOMEM; - goto out; - } + *var_name = git__strndup(line, var_end - line + 1); + GITERR_CHECK_ALLOC(*var_name); - *var_name = tmp; + /* If there is no value, boolean true is assumed */ + *var_value = NULL; /* * Now, let's try to parse the value */ if (value_start != NULL) { - while (isspace(value_start[0])) value_start++; - if (value_start[0] == '\0') { - *var_value = NULL; - goto out; - } - if (is_multiline_var(value_start)) { - error = parse_multiline_variable(cfg, value_start, var_value); - if (error != GIT_SUCCESS) - { - *var_value = NULL; + git_buf multi_value = GIT_BUF_INIT; + git_buf_puts(&multi_value, value_start); + + if (parse_multiline_variable(cfg, &multi_value) < 0 || git_buf_oom(&multi_value)) { git__free(*var_name); + git__free(line); + git_buf_free(&multi_value); + return -1; } - goto out; - } - tmp = git__strdup(value_start); - if (tmp == NULL) { - git__free(*var_name); - *var_value = NULL; - error = GIT_ENOMEM; - goto out; + *var_value = git_buf_detach(&multi_value); + } + else if (value_start[0] != '\0') { + *var_value = git__strdup(value_start); + GITERR_CHECK_ALLOC(*var_value); } - *var_value = tmp; - } else { - /* If there is no value, boolean true is assumed */ - *var_value = NULL; } - out: git__free(line); - return error; + return 0; } diff --git a/src/errors.c b/src/errors.c index c25fa7519..6fb7777f0 100644 --- a/src/errors.c +++ b/src/errors.c @@ -122,25 +122,28 @@ void giterr_set(int error_class, const char *string, ...) { char error_str[1024]; va_list arglist; - git_error *error; - const char *oserr = - (error_class == GITERR_OS && errno != 0) ? strerror(errno) : NULL; - - error = &GIT_GLOBAL->error_t; - free(error->message); va_start(arglist, string); p_vsnprintf(error_str, sizeof(error_str), string, arglist); va_end(arglist); /* automatically suffix strerror(errno) for GITERR_OS errors */ - if (oserr != NULL) { + if (error_class == GITERR_OS) { strncat(error_str, ": ", sizeof(error_str)); - strncat(error_str, oserr, sizeof(error_str)); + strncat(error_str, strerror(errno), sizeof(error_str)); errno = 0; } - error->message = git__strdup(error_str); + giterr_set_str(error_class, error_str); +} + +void giterr_set_str(int error_class, const char *string) +{ + git_error *error = &GIT_GLOBAL->error_t; + + free(error->message); + + error->message = git__strdup(string); error->klass = error_class; if (error->message == NULL) { @@ -151,6 +154,13 @@ void giterr_set(int error_class, const char *string, ...) GIT_GLOBAL->last_error = error; } +void giterr_set_regex(const regex_t *regex, int error_code) +{ + char error_buf[1024]; + regerror(error_code, regex, error_buf, sizeof(error_buf)); + giterr_set_str(GITERR_REGEX, error_buf); +} + void giterr_clear(void) { GIT_GLOBAL->last_error = NULL; diff --git a/src/filebuf.c b/src/filebuf.c index 8297b4fcf..5e206c5d8 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -14,6 +14,36 @@ static const size_t WRITE_BUFFER_SIZE = (4096 * 2); +enum buferr_t { + BUFERR_OK = 0, + BUFERR_WRITE, + BUFERR_ZLIB, + BUFERR_MEM +}; + +#define ENSURE_BUF_OK(buf) if ((buf)->last_error != BUFERR_OK) { return -1; } + +static int verify_last_error(git_filebuf *file) +{ + switch (file->last_error) { + case BUFERR_WRITE: + giterr_set(GITERR_OS, "Failed to write out file"); + return -1; + + case BUFERR_MEM: + giterr_set_oom(); + return -1; + + case BUFERR_ZLIB: + giterr_set(GITERR_ZLIB, + "Buffer error when writing out ZLib data"); + return -1; + + default: + return 0; + } +} + static int lock_file(git_filebuf *file, int flags) { if (git_path_exists(file->path_lock) == true) { @@ -100,20 +130,21 @@ GIT_INLINE(int) flush_buffer(git_filebuf *file) static int write_normal(git_filebuf *file, void *source, size_t len) { - int result = 0; - if (len > 0) { - result = p_write(file->fd, (void *)source, len); + if (p_write(file->fd, (void *)source, len) < 0) { + file->last_error = BUFERR_WRITE; + return -1; + } + if (file->digest) git_hash_update(file->digest, source, len); } - return result; + return 0; } static int write_deflate(git_filebuf *file, void *source, size_t len) { - int result = Z_OK; z_stream *zs = &file->zs; if (len > 0 || file->flush_mode == Z_FINISH) { @@ -126,14 +157,17 @@ static int write_deflate(git_filebuf *file, void *source, size_t len) zs->next_out = file->z_buf; zs->avail_out = (uInt)file->buf_size; - result = deflate(zs, file->flush_mode); - if (result == Z_STREAM_ERROR) - return git__throw(GIT_ERROR, "Failed to deflate input"); + if (deflate(zs, file->flush_mode) == Z_STREAM_ERROR) { + file->last_error = BUFERR_ZLIB; + return -1; + } have = file->buf_size - (size_t)zs->avail_out; - if (p_write(file->fd, file->z_buf, have) < GIT_SUCCESS) - return git__throw(GIT_EOSERR, "Failed to write to file"); + if (p_write(file->fd, file->z_buf, have) < 0) { + file->last_error = BUFERR_WRITE; + return -1; + } } while (zs->avail_out == 0); @@ -143,7 +177,7 @@ static int write_deflate(git_filebuf *file, void *source, size_t len) git_hash_update(file->digest, source, len); } - return GIT_SUCCESS; + return 0; } int git_filebuf_open(git_filebuf *file, const char *path, int flags) @@ -161,6 +195,7 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags) file->buf_size = WRITE_BUFFER_SIZE; file->buf_pos = 0; file->fd = -1; + file->last_error = BUFERR_OK; /* Allocate the main cache buffer */ file->buffer = git__malloc(file->buf_size); @@ -237,58 +272,61 @@ cleanup: int git_filebuf_hash(git_oid *oid, git_filebuf *file) { - int error; - assert(oid && file && file->digest); - if ((error = flush_buffer(file)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to get hash for file"); + flush_buffer(file); + + if (verify_last_error(file) < 0) + return -1; git_hash_final(oid, file->digest); git_hash_free_ctx(file->digest); file->digest = NULL; - return GIT_SUCCESS; + return 0; } int git_filebuf_commit_at(git_filebuf *file, const char *path, mode_t mode) { git__free(file->path_original); file->path_original = git__strdup(path); - if (file->path_original == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(file->path_original); return git_filebuf_commit(file, mode); } int git_filebuf_commit(git_filebuf *file, mode_t mode) { - int error; - /* temporary files cannot be committed */ assert(file && file->path_original); file->flush_mode = Z_FINISH; - if ((error = flush_buffer(file)) < GIT_SUCCESS) - goto cleanup; + flush_buffer(file); + + if (verify_last_error(file) < 0) + goto on_error; p_close(file->fd); file->fd = -1; if (p_chmod(file->path_lock, mode)) { - error = git__throw(GIT_EOSERR, "Failed to chmod locked file before committing"); - goto cleanup; + giterr_set(GITERR_OS, "Failed to set attributes for file at '%s'", file->path_lock); + goto on_error; } p_unlink(file->path_original); - error = p_rename(file->path_lock, file->path_original); + if (p_rename(file->path_lock, file->path_original) < 0) { + giterr_set(GITERR_OS, "Failed to rename lockfile to '%s'", file->path_original); + goto on_error; + } -cleanup: git_filebuf_cleanup(file); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to commit locked file from buffer"); - return GIT_SUCCESS; + return 0; + +on_error: + git_filebuf_cleanup(file); + return -1; } GIT_INLINE(void) add_to_cache(git_filebuf *file, const void *buf, size_t len) @@ -299,22 +337,22 @@ GIT_INLINE(void) add_to_cache(git_filebuf *file, const void *buf, size_t len) int git_filebuf_write(git_filebuf *file, const void *buff, size_t len) { - int error; const unsigned char *buf = buff; + ENSURE_BUF_OK(file); + for (;;) { size_t space_left = file->buf_size - file->buf_pos; /* cache if it's small */ if (space_left > len) { add_to_cache(file, buf, len); - return GIT_SUCCESS; + return 0; } add_to_cache(file, buf, space_left); - - if ((error = flush_buffer(file)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to write to buffer"); + if (flush_buffer(file) < 0) + return -1; len -= space_left; buf += space_left; @@ -323,32 +361,37 @@ int git_filebuf_write(git_filebuf *file, const void *buff, size_t len) int git_filebuf_reserve(git_filebuf *file, void **buffer, size_t len) { - int error; size_t space_left = file->buf_size - file->buf_pos; *buffer = NULL; - if (len > file->buf_size) - return GIT_ENOMEM; + ENSURE_BUF_OK(file); + + if (len > file->buf_size) { + file->last_error = BUFERR_MEM; + return -1; + } if (space_left <= len) { - if ((error = flush_buffer(file)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to reserve buffer"); + if (flush_buffer(file) < 0) + return -1; } *buffer = (file->buffer + file->buf_pos); file->buf_pos += len; - return GIT_SUCCESS; + return 0; } int git_filebuf_printf(git_filebuf *file, const char *format, ...) { va_list arglist; size_t space_left; - int len, error; + int len, res; char *tmp_buffer; + ENSURE_BUF_OK(file); + space_left = file->buf_size - file->buf_pos; do { @@ -356,24 +399,28 @@ int git_filebuf_printf(git_filebuf *file, const char *format, ...) len = p_vsnprintf((char *)file->buffer + file->buf_pos, space_left, format, arglist); va_end(arglist); - if (len < 0) - return git__throw(GIT_EOSERR, "Failed to format string"); + if (len < 0) { + file->last_error = BUFERR_MEM; + return -1; + } if ((size_t)len + 1 <= space_left) { file->buf_pos += len; - return GIT_SUCCESS; + return 0; } - if ((error = flush_buffer(file)) < GIT_SUCCESS) - return git__rethrow(error, "Failed to output to buffer"); + if (flush_buffer(file) < 0) + return -1; space_left = file->buf_size - file->buf_pos; } while ((size_t)len + 1 <= space_left); tmp_buffer = git__malloc(len + 1); - if (!tmp_buffer) - return GIT_ENOMEM; + if (!tmp_buffer) { + file->last_error = BUFERR_MEM; + return -1; + } va_start(arglist, format); len = p_vsnprintf(tmp_buffer, len + 1, format, arglist); @@ -381,12 +428,13 @@ int git_filebuf_printf(git_filebuf *file, const char *format, ...) if (len < 0) { git__free(tmp_buffer); - return git__throw(GIT_EOSERR, "Failed to format string"); + file->last_error = BUFERR_MEM; + return -1; } - error = git_filebuf_write(file, tmp_buffer, len); + res = git_filebuf_write(file, tmp_buffer, len); git__free(tmp_buffer); - return error; + return res; } diff --git a/src/filebuf.h b/src/filebuf.h index 371215391..5f9d4ad9d 100644 --- a/src/filebuf.h +++ b/src/filebuf.h @@ -40,25 +40,35 @@ struct git_filebuf { size_t buf_size, buf_pos; git_file fd; + int last_error; }; typedef struct git_filebuf git_filebuf; #define GIT_FILEBUF_INIT {0} -/* The git_filebuf object lifecycle is: +/* + * The git_filebuf object lifecycle is: * - Allocate git_filebuf, preferably using GIT_FILEBUF_INIT. + * * - Call git_filebuf_open() to initialize the filebuf for use. + * * - Make as many calls to git_filebuf_write(), git_filebuf_printf(), - * git_filebuf_reserve() as you like. + * git_filebuf_reserve() as you like. The error codes for these + * functions don't need to be checked. They are stored internally + * by the file buffer. + * * - While you are writing, you may call git_filebuf_hash() to get - * the hash of all you have written so far. + * the hash of all you have written so far. This function will + * fail if any of the previous writes to the buffer failed. + * * - To close the git_filebuf, you may call git_filebuf_commit() or * git_filebuf_commit_at() to save the file, or * git_filebuf_cleanup() to abandon the file. All of these will - * clear the git_filebuf object. + * free the git_filebuf object. Likewise, all of these will fail + * if any of the previous writes to the buffer failed, and set + * an error code accordingly. */ - int git_filebuf_write(git_filebuf *lock, const void *buff, size_t len); int git_filebuf_reserve(git_filebuf *file, void **buff, size_t len); int git_filebuf_printf(git_filebuf *file, const char *format, ...) GIT_FORMAT_PRINTF(2, 3); diff --git a/src/hashtable.c b/src/hashtable.c index 73a6336c4..c081fc9a7 100644 --- a/src/hashtable.c +++ b/src/hashtable.c @@ -227,11 +227,11 @@ int git_hashtable_remove2(git_hashtable *self, const void *key, void **old_value node->key = NULL; node->value = NULL; self->key_count--; - return GIT_SUCCESS; + return 0; } } - return git__throw(GIT_ENOTFOUND, "Entry not found in hash table"); + return GIT_ENOTFOUND; } int git_hashtable_merge(git_hashtable *self, git_hashtable *other) diff --git a/src/posix.c b/src/posix.c index d2364d9b4..9d96d3013 100644 --- a/src/posix.c +++ b/src/posix.c @@ -31,10 +31,9 @@ int p_getcwd(char *buffer_out, size_t size) cwd_buffer = getcwd(buffer_out, size); if (cwd_buffer == NULL) - return git__throw(GIT_EOSERR, "Failed to retrieve current working directory"); + return -1; git_path_mkposix(buffer_out); - git_path_string_to_dir(buffer_out, size); //Ensure the path ends with a trailing slash return GIT_SUCCESS; @@ -44,14 +43,13 @@ int p_rename(const char *from, const char *to) { if (!link(from, to)) { p_unlink(from); - return GIT_SUCCESS; + return 0; } if (!rename(from, to)) - return GIT_SUCCESS; - - return GIT_ERROR; + return 0; + return -1; } #endif @@ -64,7 +62,7 @@ int p_read(git_file fd, void *buf, size_t cnt) if (r < 0) { if (errno == EINTR || errno == EAGAIN) continue; - return GIT_EOSERR; + return -1; } if (!r) break; @@ -82,14 +80,14 @@ int p_write(git_file fd, const void *buf, size_t cnt) if (r < 0) { if (errno == EINTR || errno == EAGAIN) continue; - return GIT_EOSERR; + return -1; } if (!r) { errno = EPIPE; - return GIT_EOSERR; + return -1; } cnt -= r; b += r; } - return GIT_SUCCESS; + return 0; } diff --git a/src/util.h b/src/util.h index 01755b59e..afa3f7205 100644 --- a/src/util.h +++ b/src/util.h @@ -7,8 +7,6 @@ #ifndef INCLUDE_util_h__ #define INCLUDE_util_h__ -#include "git2/errors.h" - #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0])) #define bitsizeof(x) (CHAR_BIT * sizeof(x)) #define MSB(x, bits) ((x) & (~0ULL << (bitsizeof(x) - (bits)))) @@ -24,24 +22,21 @@ GIT_INLINE(void *) git__malloc(size_t len) { void *ptr = malloc(len); - if (!ptr) - giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to allocate %d bytes.", (int)len); + if (!ptr) giterr_set_oom(); return ptr; } GIT_INLINE(void *) git__calloc(size_t nelem, size_t elsize) { void *ptr = calloc(nelem, elsize); - if (!ptr) - giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to allocate %d bytes.", (int)nelem*elsize); + if (!ptr) giterr_set_oom(); return ptr; } GIT_INLINE(char *) git__strdup(const char *str) { char *ptr = strdup(str); - if (!ptr) - giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to duplicate string"); + if (!ptr) giterr_set_oom(); return ptr; } @@ -56,7 +51,7 @@ GIT_INLINE(char *) git__strndup(const char *str, size_t n) ptr = (char*)malloc(length + 1); if (!ptr) { - giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to duplicate string"); + giterr_set_oom(); return NULL; } @@ -69,8 +64,7 @@ GIT_INLINE(char *) git__strndup(const char *str, size_t n) GIT_INLINE(void *) git__realloc(void *ptr, size_t size) { void *new_ptr = realloc(ptr, size); - if (!new_ptr) - giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to allocate %d bytes.", (int)size); + if (!new_ptr) giterr_set_oom(); return new_ptr; } -- 2.39.2