From 773bc20dd3f78d4e307ff54edd1e09f508f2a06f Mon Sep 17 00:00:00 2001 From: "Jason R. McNeil" Date: Tue, 3 May 2011 22:22:42 -0700 Subject: [PATCH 01/34] Fix misspelling of git_index_append2 (was git_index_apppend2). --- include/git2/index.h | 4 ++-- src/index.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/git2/index.h b/include/git2/index.h index 7991de92e..2d8975ca1 100644 --- a/include/git2/index.h +++ b/include/git2/index.h @@ -232,10 +232,10 @@ GIT_EXTERN(int) git_index_append(git_index *index, const char *path, int stage); * @param source_entry new entry object * @return 0 on success, otherwise an error code */ -GIT_EXTERN(int) git_index_apppend2(git_index *index, const git_index_entry *source_entry); +GIT_EXTERN(int) git_index_append2(git_index *index, const git_index_entry *source_entry); /** - * Remove an entry from the index + * Remove an entry from the index * * @param index an existing index object * @param position position of the entry to remove diff --git a/src/index.c b/src/index.c index 130d1fd36..f643e73fb 100644 --- a/src/index.c +++ b/src/index.c @@ -421,7 +421,7 @@ int git_index_add2(git_index *index, const git_index_entry *source_entry) return index_insert(index, source_entry, 1); } -int git_index_apppend2(git_index *index, const git_index_entry *source_entry) +int git_index_append2(git_index *index, const git_index_entry *source_entry) { return index_insert(index, source_entry, 0); } From bbd68c6768dc351da7d327682cc904b16c6d73ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 5 May 2011 11:38:23 +0200 Subject: [PATCH 02/34] ref test: update a forgotten repo -> repo2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 34e5d87e0512f2a3dfd6 left one of these unchanged we're trying to read from a free'd repository. Signed-off-by: Carlos Martín Nieto --- tests/t10-refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/t10-refs.c b/tests/t10-refs.c index db767a107..ee006a8ce 100644 --- a/tests/t10-refs.c +++ b/tests/t10-refs.c @@ -292,7 +292,7 @@ BEGIN_TEST(create2, "create a new OID reference") /* Similar test with a fresh new repository */ must_pass(git_repository_open(&repo2, TEMP_REPO_FOLDER)); - must_pass(git_reference_lookup(&looked_up_ref, repo, new_head)); + must_pass(git_reference_lookup(&looked_up_ref, repo2, new_head)); must_be_true(git_oid_cmp(&id, git_reference_oid(looked_up_ref)) == 0); close_temp_repo(repo2); From 39e1032cfcc27ddfeb9e8d2d7ab5d7d1c51da0cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 6 May 2011 12:43:37 +0200 Subject: [PATCH 03/34] odb backend_sort_cmp should be static MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Martín Nieto --- src/odb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/odb.c b/src/odb.c index 43d0ef9a4..e9e495eb1 100644 --- a/src/odb.c +++ b/src/odb.c @@ -221,7 +221,7 @@ static int init_fake_wstream(git_odb_stream **stream_p, git_odb_backend *backend * ***********************************************************/ -int backend_sort_cmp(const void *a, const void *b) +static int backend_sort_cmp(const void *a, const void *b) { const backend_internal *backend_a = *(const backend_internal **)(a); const backend_internal *backend_b = *(const backend_internal **)(b); From d8e1d038b3e77fdcec01e662dc9bc6708ebcd24c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 6 May 2011 12:47:21 +0200 Subject: [PATCH 04/34] Fix two warnings from Clang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are about not reading the value stored in a variable. Signed-off-by: Carlos Martín Nieto --- src/commit.c | 2 +- src/odb_loose.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/commit.c b/src/commit.c index 0c37ec59b..54d7a47fe 100644 --- a/src/commit.c +++ b/src/commit.c @@ -286,7 +286,7 @@ int commit_parse_buffer(git_commit *commit, const void *data, size_t len) if (buffer < buffer_end) { const char *line_end; - size_t message_len = buffer_end - buffer; + size_t message_len; /* Long message */ message_len = buffer_end - buffer; diff --git a/src/odb_loose.c b/src/odb_loose.c index 4f475f2c3..873dbfa0a 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -336,7 +336,6 @@ static int inflate_disk_obj(git_rawobj *out, gitfo_buf *obj) { unsigned char head[64], *buf; z_stream zs; - int z_status; obj_hdr hdr; size_t used; @@ -350,7 +349,7 @@ static int inflate_disk_obj(git_rawobj *out, gitfo_buf *obj) * inflate the initial part of the io buffer in order * to parse the object header (type and size). */ - if ((z_status = start_inflate(&zs, obj, head, sizeof(head))) < Z_OK) + if (start_inflate(&zs, obj, head, sizeof(head)) < Z_OK) return GIT_ERROR; if ((used = get_object_header(&hdr, head)) == 0) From 560cd27ef3bbaac66a858ab86b3be04148281c6e Mon Sep 17 00:00:00 2001 From: "kelly.leahy" Date: Sun, 8 May 2011 12:30:16 -0700 Subject: [PATCH 05/34] Add git attributes settings for *.c and *.h to force line endings to LF. --- .gitattributes | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..f90540b55 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +*.c eol=lf +*.h eol=lf From 16a5c3046595284c1f2f8b9eb8621d76239e4b61 Mon Sep 17 00:00:00 2001 From: "kelly.leahy" Date: Sun, 8 May 2011 12:32:35 -0700 Subject: [PATCH 06/34] Fix bug in the way pthead_mutex_t was being destroyed in win32. Win32 critical section objects (CRITICAL_SECTION) are not kernel objects. Only kernel objects are destroyed by using CloseHandle. Critical sections are supposed to be deleted with the DeleteCriticalSection API (http://msdn.microsoft.com/en-us/library/ms682552(VS.85).aspx). --- src/win32/pthread.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/win32/pthread.c b/src/win32/pthread.c index f47364a76..7e17b6bdf 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -48,16 +48,15 @@ int pthread_join(pthread_t thread, void **value_ptr) int pthread_mutex_init(pthread_mutex_t *GIT_RESTRICT mutex, const pthread_mutexattr_t *GIT_RESTRICT GIT_UNUSED(mutexattr)) { - GIT_UNUSED_ARG(mutexattr); + GIT_UNUSED_ARG(mutexattr); InitializeCriticalSection(mutex); return 0; } int pthread_mutex_destroy(pthread_mutex_t *mutex) { - int ret; - ret = CloseHandle(mutex); - return -(!ret); + DeleteCriticalSection(mutex); + return 0; } int pthread_mutex_lock(pthread_mutex_t *mutex) From 02f9e637a1d282d03ce3362e7c3467e73c87bf2e Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 5 May 2011 01:12:17 +0300 Subject: [PATCH 07/34] errors: Add error handling function --- include/git2/thread-utils.h | 1 + src/common.h | 2 ++ src/errors.c | 40 +++++++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/include/git2/thread-utils.h b/include/git2/thread-utils.h index fb8644b93..e26876bea 100644 --- a/include/git2/thread-utils.h +++ b/include/git2/thread-utils.h @@ -35,6 +35,7 @@ #if defined(__APPLE__) && defined(__MACH__) # undef GIT_TLS +# define GIT_TLS #elif defined(__GNUC__) || \ defined(__SUNPRO_C) || \ diff --git a/src/common.h b/src/common.h index 5ad878e26..b3f5253a9 100644 --- a/src/common.h +++ b/src/common.h @@ -56,4 +56,6 @@ typedef SSIZE_T ssize_t; #define GIT_PATH_MAX 4096 +extern int git__error(int error, const char *, ...) GIT_FORMAT_PRINTF(2, 3); + #endif /* INCLUDE_common_h__ */ diff --git a/src/errors.c b/src/errors.c index c3a495cc9..b0c1c1519 100644 --- a/src/errors.c +++ b/src/errors.c @@ -1,6 +1,33 @@ +/* + * This file is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, + * as published by the Free Software Foundation. + * + * In addition to the permissions in the GNU General Public License, + * the authors give you unlimited permission to link the compiled + * version of this file into combinations with other programs, + * and to distribute those combinations without any restriction + * coming from the use of this file. (The General Public License + * restrictions do apply in other respects; for example, they cover + * modification of the file, and distribution when not linked into + * a combined executable.) + * + * This file is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; see the file COPYING. If not, write to + * the Free Software Foundation, 51 Franklin Street, Fifth Floor, + * Boston, MA 02110-1301, USA. + */ #include "common.h" +#include "git2/thread-utils.h" /* for GIT_TLS */ #include "thread-utils.h" /* for GIT_TLS */ +#include + static struct { int num; const char *str; @@ -46,3 +73,16 @@ const char *git_strerror(int num) return "Unknown error"; } + +static GIT_TLS char g_last_error[1024]; + +int git__error(int error, const char *msg, ...) +{ + va_list va; + + va_start(va, msg); + vsnprintf(g_last_error, sizeof(g_last_error), msg, va); + va_end(va); + + return error; +} From 3f53c97113deb986b58aac8bb89f76b47e76507b Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 5 May 2011 01:20:27 +0300 Subject: [PATCH 08/34] errors: Set error messages on memory allocation --- src/common.h | 4 ++-- src/util.h | 43 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/common.h b/src/common.h index b3f5253a9..351d6696f 100644 --- a/src/common.h +++ b/src/common.h @@ -50,12 +50,12 @@ typedef SSIZE_T ssize_t; #include "git2/common.h" #include "git2/types.h" -#include "util.h" #include "thread-utils.h" #include "bswap.h" #define GIT_PATH_MAX 4096 - extern int git__error(int error, const char *, ...) GIT_FORMAT_PRINTF(2, 3); +#include "util.h" + #endif /* INCLUDE_common_h__ */ diff --git a/src/util.h b/src/util.h index 3c606493f..f5f0b8662 100644 --- a/src/util.h +++ b/src/util.h @@ -6,16 +6,41 @@ #define MSB(x, bits) ((x) & (~0ULL << (bitsizeof(x) - (bits)))) /* - * Don't wrap malloc/calloc. - * Use the default versions in glibc, and make - * sure that any methods that allocate memory - * return a GIT_ENOMEM error when allocation - * fails. + * Custom memory allocation wrappers + * that set error code and error message + * on allocation failure */ -#define git__malloc malloc -#define git__calloc calloc -#define git__realloc realloc -#define git__strdup strdup +GIT_INLINE(void *) git__malloc(size_t len) +{ + void *ptr = malloc(len); + if (!ptr) + git__error(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)len); + return ptr; +} + +GIT_INLINE(void *) git__calloc(size_t nelem, size_t elsize) +{ + void *ptr = calloc(nelem, elsize); + if (!ptr) + git__error(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)elsize); + return ptr; +} + +GIT_INLINE(char *) git__strdup(const char *str) +{ + char *ptr = strdup(str); + if (!ptr) + git__error(GIT_ENOMEM, "Out of memory. Failed to duplicate string"); + return ptr; +} + +GIT_INLINE(void *) git__realloc(void *ptr, size_t size) +{ + void *new_ptr = realloc(ptr, size); + if (!new_ptr) + git__error(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)size); + return new_ptr; +} extern int git__fmt(char *, size_t, const char *, ...) GIT_FORMAT_PRINTF(3, 4); From 5eb0fab846d6b2f8bcf3caf7a9e33afa36753850 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 5 May 2011 01:49:27 +0300 Subject: [PATCH 09/34] errors: Update external API with new `git_lasterror` --- include/git2/common.h | 3 +++ include/git2/errors.h | 8 +++---- src/errors.c | 52 +++++-------------------------------------- 3 files changed, 13 insertions(+), 50 deletions(-) diff --git a/include/git2/common.h b/include/git2/common.h index 22c7cc466..1d2d3a3e9 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -170,6 +170,9 @@ /** The given literal is not a valid number */ #define GIT_ENOTNUM (GIT_ERROR - 25) +/** Streaming error */ +#define GIT_ESTREAM (GIT_ERROR - 26) + GIT_BEGIN_DECL typedef struct { diff --git a/include/git2/errors.h b/include/git2/errors.h index 627e67c70..fde0dc73d 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -34,11 +34,11 @@ GIT_BEGIN_DECL /** - * strerror() for the Git library - * @param num The error code to explain - * @return a string explaining the error code + * Return a detailed error string with the latest error + * that occurred in the library. + * @return a string explaining the error */ -GIT_EXTERN(const char *) git_strerror(int num); +GIT_EXTERN(const char *) git_lasterror(void); /** @} */ GIT_END_DECL diff --git a/src/errors.c b/src/errors.c index b0c1c1519..73df2e209 100644 --- a/src/errors.c +++ b/src/errors.c @@ -28,52 +28,6 @@ #include -static struct { - int num; - const char *str; -} error_codes[] = { - {GIT_ERROR, "Unspecified error"}, - {GIT_ENOTOID, "Input was not a properly formatted Git object id."}, - {GIT_ENOTFOUND, "Object does not exist in the scope searched."}, - {GIT_ENOMEM, "Not enough space available."}, - {GIT_EOSERR, "Consult the OS error information."}, - {GIT_EOBJTYPE, "The specified object is of invalid type"}, - {GIT_EOBJCORRUPTED, "The specified object has its data corrupted"}, - {GIT_ENOTAREPO, "The specified repository is invalid"}, - {GIT_EINVALIDTYPE, "The object type is invalid or doesn't match"}, - {GIT_EMISSINGOBJDATA, "The object cannot be written that because it's missing internal data"}, - {GIT_EPACKCORRUPTED, "The packfile for the ODB is corrupted"}, - {GIT_EFLOCKFAIL, "Failed to adquire or release a file lock"}, - {GIT_EZLIB, "The Z library failed to inflate/deflate an object's data"}, - {GIT_EBUSY, "The queried object is currently busy"}, - {GIT_EINVALIDPATH, "The path is invalid"}, - {GIT_EBAREINDEX, "The index file is not backed up by an existing repository"}, - {GIT_EINVALIDREFNAME, "The name of the reference is not valid"}, - {GIT_EREFCORRUPTED, "The specified reference has its data corrupted"}, - {GIT_ETOONESTEDSYMREF, "The specified symbolic reference is too deeply nested"}, - {GIT_EPACKEDREFSCORRUPTED, "The pack-refs file is either corrupted of its format is not currently supported"}, - {GIT_EINVALIDPATH, "The path is invalid" }, - {GIT_EREVWALKOVER, "The revision walker is empty; there are no more commits left to iterate"}, - {GIT_EINVALIDREFSTATE, "The state of the reference is not valid"}, - {GIT_ENOTIMPLEMENTED, "This feature has not been implemented yet"}, - {GIT_EEXISTS, "A reference with this name already exists"}, - {GIT_EOVERFLOW, "The given integer literal is too large to be parsed"}, - {GIT_ENOTNUM, "The given literal is not a valid number"}, -}; - -const char *git_strerror(int num) -{ - size_t i; - - if (num == GIT_EOSERR) - return strerror(errno); - for (i = 0; i < ARRAY_SIZE(error_codes); i++) - if (num == error_codes[i].num) - return error_codes[i].str; - - return "Unknown error"; -} - static GIT_TLS char g_last_error[1024]; int git__error(int error, const char *msg, ...) @@ -86,3 +40,9 @@ int git__error(int error, const char *msg, ...) return error; } + +const char *git_lasterror(void) +{ + return g_last_error; +} + From fa59f18d0ddbbb98d45e33934fb0efc3e2bf1557 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Mon, 9 May 2011 20:54:04 +0300 Subject: [PATCH 10/34] Change error handling mechanism once again Ok, this is the real deal. Hopefully. Here's how it's going to work: - One main method, called `git__throw`, that sets the error code and error message when an error happens. This method must be called in every single place where an error code was being returned previously, setting an error message instead. Example, instead of: return GIT_EOBJCORRUPTED; Use: return git__throw(GIT_EOBJCORRUPTED, "The object is missing a finalizing line feed"); And instead of: [...] { error = GIT_EOBJCORRUPTED; goto cleanup; } Use: [...] { error = git__throw(GIT_EOBJCORRUPTED, "What an error!"); goto cleanup; } The **only** exception to this are the allocation methods, which return NULL on failure but already set the message manually. /* only place where an error code can be returned directly, because the error message has already been set by the wrapper */ if (foo == NULL) return GIT_ENOMEM; - One secondary method, called `git__rethrow`, which can be used to fine-grain an error message and build an error stack. Example, instead of: if ((error = foobar(baz)) < GIT_SUCCESS) return error; You can now do: if ((error = foobar(baz)) < GIT_SUCCESS) return git__rethrow(error, "Failed to do a major operation"); The return of the `git_lasterror` method will be a string in the shape of: "Failed to do a major operation. (Failed to do an internal operation)" E.g. "Failed to open the index. (Not enough permissions to access '/path/to/index')." NOTE: do not abuse this method. Try to write all `git__throw` messages in a descriptive manner, to avoid having to rethrow them to clarify their meaning. This method should only be used in the places where the original error message set by a subroutine is not specific enough. It is encouraged to continue using this style as much possible to enforce error propagation: if ((error = foobar(baz)) < GIT_SUCCESS) return error; /* `foobar` has set an error message, and we are just propagating it */ The error handling revamp will take place in two phases: - Phase 1: Replace all pieces of code that return direct error codes with calls to `git__throw`. This can be done semi-automatically using `ack` to locate all the error codes that must be replaced. - Phase 2: Add some `git__rethrow` calls in those cases where the original error messages are not specific enough. Phase 1 is the main goal. A minor libgit2 release will be shipped once Phase 1 is ready, and the work will start on gradually improving the error handling mechanism by refining specific error messages. OTHER NOTES: - When writing error messages, please refrain from using weasel words. They add verbosity to the message without giving any real information. (<3 Emeric) E.g. "The reference file appears to be missing a carriage return" Nope. "The reference file is missing a carriage return" Yes. - When calling `git__throw`, please try to use more generic error codes so we can eventually reduce the list of error codes to something more reasonable. Feel free to add new, more generic error codes if these are going to replace several of the old ones. E.g. return GIT_EREFCORRUPTED; Can be turned into: return git__throw(GIT_EOBJCORRUPTED, "The reference is corrupted"); --- include/git2/common.h | 3 +++ src/common.h | 3 ++- src/errors.c | 20 +++++++++++++++++++- src/refs.c | 33 +++++++++++++++++++-------------- src/util.h | 8 ++++---- 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/include/git2/common.h b/include/git2/common.h index 1d2d3a3e9..2aae648fb 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -173,6 +173,9 @@ /** Streaming error */ #define GIT_ESTREAM (GIT_ERROR - 26) +/** invalid arguments to function */ +#define GIT_EINVALIDARGS (GIT_ERROR - 27) + GIT_BEGIN_DECL typedef struct { diff --git a/src/common.h b/src/common.h index 351d6696f..e5b9f15ed 100644 --- a/src/common.h +++ b/src/common.h @@ -54,7 +54,8 @@ typedef SSIZE_T ssize_t; #include "bswap.h" #define GIT_PATH_MAX 4096 -extern int git__error(int error, const char *, ...) GIT_FORMAT_PRINTF(2, 3); +extern int git__throw(int error, const char *, ...) GIT_FORMAT_PRINTF(2, 3); +extern int git__rethrow(int error, const char *, ...) GIT_FORMAT_PRINTF(2, 3); #include "util.h" diff --git a/src/errors.c b/src/errors.c index 73df2e209..40b0feb91 100644 --- a/src/errors.c +++ b/src/errors.c @@ -30,7 +30,25 @@ static GIT_TLS char g_last_error[1024]; -int git__error(int error, const char *msg, ...) +int git__rethrow(int error, const char *msg, ...) +{ + char new_error[1024]; + char *old_error = NULL; + + va_list va; + + va_start(va, msg); + vsnprintf(new_error, sizeof(new_error), msg, va); + va_end(va); + + old_error = strdup(g_last_error); + snprintf(g_last_error, sizeof(g_last_error), "%s \n - %s", new_error, old_error); + free(old_error); + + return error; +} + +int git__throw(int error, const char *msg, ...) { va_list va; diff --git a/src/refs.c b/src/refs.c index ea968196f..c4d3d6ae6 100644 --- a/src/refs.c +++ b/src/refs.c @@ -122,7 +122,8 @@ static int reference_create( else if (type == GIT_REF_OID) size = sizeof(reference_oid); else - return GIT_EINVALIDREFSTATE; + return git__throw(GIT_EINVALIDARGS, + "Invalid reference type. Use either GIT_REF_OID or GIT_REF_SYMBOLIC as type specifier"); reference = git__malloc(size); if (reference == NULL) @@ -159,11 +160,9 @@ static int reference_read(gitfo_buf *file_content, time_t *mtime, const char *re /* Determine the full path of the file */ git__joinpath(path, repo_path, ref_name); - if (gitfo_stat(path, &st) < 0) - return GIT_ENOTFOUND; - - if (S_ISDIR(st.st_mode)) - return GIT_EOBJCORRUPTED; + if (gitfo_stat(path, &st) < 0 || S_ISDIR(st.st_mode)) + return git__throw(GIT_ENOTFOUND, + "Cannot read reference file '%s'", ref_name); if (mtime) *mtime = st.st_mtime; @@ -205,7 +204,8 @@ static int loose_update(git_reference *ref) else if (ref->type == GIT_REF_OID) error = loose_parse_oid(ref, &ref_file); else - error = GIT_EINVALIDREFSTATE; + error = git__throw(GIT_EOBJCORRUPTED, + "Invalid reference type (%d) for loose reference", ref->type); gitfo_free_buf(&ref_file); @@ -229,7 +229,8 @@ static int loose_parse_symbolic(git_reference *ref, gitfo_buf *file_content) ref_sym = (reference_symbolic *)ref; if (file_content->len < (header_len + 1)) - return GIT_EREFCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, + "Failed to parse loose reference. Object too short"); /* * Assume we have already checked for the header @@ -246,7 +247,8 @@ static int loose_parse_symbolic(git_reference *ref, gitfo_buf *file_content) /* remove newline at the end of file */ eol = strchr(ref_sym->target, '\n'); if (eol == NULL) - return GIT_EREFCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, + "Failed to parse loose reference. Missing EOL"); *eol = '\0'; if (eol[-1] == '\r') @@ -257,6 +259,7 @@ static int loose_parse_symbolic(git_reference *ref, gitfo_buf *file_content) static int loose_parse_oid(git_reference *ref, gitfo_buf *file_content) { + int error; reference_oid *ref_oid; char *buffer; @@ -265,17 +268,19 @@ static int loose_parse_oid(git_reference *ref, gitfo_buf *file_content) /* File format: 40 chars (OID) + newline */ if (file_content->len < GIT_OID_HEXSZ + 1) - return GIT_EREFCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, + "Failed to parse loose reference. Reference too short"); - if (git_oid_mkstr(&ref_oid->oid, buffer) < GIT_SUCCESS) - return GIT_EREFCORRUPTED; + if ((error = git_oid_mkstr(&ref_oid->oid, buffer)) < GIT_SUCCESS) + return git__rethrow(GIT_EOBJCORRUPTED, "Failed to parse loose reference."); buffer = buffer + GIT_OID_HEXSZ; if (*buffer == '\r') buffer++; if (*buffer != '\n') - return GIT_EREFCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, + "Failed to parse loose reference. Missing EOL"); return GIT_SUCCESS; } @@ -387,7 +392,7 @@ static int loose_write(git_reference *ref) strcpy(ref_contents, GIT_SYMREF); strcat(ref_contents, ref_sym->target); } else { - error = GIT_EINVALIDREFSTATE; + error = git__throw(GIT_EOBJCORRUPTED, "Failed to write reference. Invalid reference type"); goto unlock; } diff --git a/src/util.h b/src/util.h index f5f0b8662..6724e8d41 100644 --- a/src/util.h +++ b/src/util.h @@ -14,7 +14,7 @@ GIT_INLINE(void *) git__malloc(size_t len) { void *ptr = malloc(len); if (!ptr) - git__error(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)len); + git__throw(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)len); return ptr; } @@ -22,7 +22,7 @@ GIT_INLINE(void *) git__calloc(size_t nelem, size_t elsize) { void *ptr = calloc(nelem, elsize); if (!ptr) - git__error(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)elsize); + git__throw(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)elsize); return ptr; } @@ -30,7 +30,7 @@ GIT_INLINE(char *) git__strdup(const char *str) { char *ptr = strdup(str); if (!ptr) - git__error(GIT_ENOMEM, "Out of memory. Failed to duplicate string"); + git__throw(GIT_ENOMEM, "Out of memory. Failed to duplicate string"); return ptr; } @@ -38,7 +38,7 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size) { void *new_ptr = realloc(ptr, size); if (!new_ptr) - git__error(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)size); + git__throw(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)size); return new_ptr; } From ab86f159e5edbf0ffaf3ec2eb86584a6758a7a49 Mon Sep 17 00:00:00 2001 From: "kelly.leahy" Date: Mon, 9 May 2011 23:39:32 -0700 Subject: [PATCH 11/34] Fix issue #79 - git_lasterror() isn't appearing in git2.dll in windows. The GIT_EXPORT macro is used to declare a function to be externally accessible to other libraries. This commit uses GIT_EXPORT to declare the git_lasterror() function as externally exported. I verified with depends.exe that the function is available to external callers (i.e. in the exports table of the PE file). --- include/git2/common.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/git2/common.h b/include/git2/common.h index 2aae648fb..54af72fbf 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -185,6 +185,8 @@ typedef struct { GIT_EXTERN(void) git_strarray_free(git_strarray *array); +GIT_EXTERN(const char*) git_lasterror(void); + /** @} */ GIT_END_DECL #endif From 5de24ec7365cbf2894f40a59576745a2951859e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 10 May 2011 17:38:41 +0200 Subject: [PATCH 12/34] Move signature.c to the new error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Martín Nieto --- src/signature.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/signature.c b/src/signature.c index e8014620a..62bd28b9a 100644 --- a/src/signature.c +++ b/src/signature.c @@ -115,22 +115,22 @@ static int parse_timezone_offset(const char *buffer, long *offset_out) } if (offset_start[0] != '-' && offset_start[0] != '+') - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse TZ offset. It doesn't start with '+' or '-'"); if (git__strtol32(&dec_offset, offset_start + 1, &offset_end, 10) < GIT_SUCCESS) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse TZ offset. It isn't a number"); if (offset_end - offset_start != 5) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse TZ offset. Invalid length"); hours = dec_offset / 100; mins = dec_offset % 100; if (hours > 14) // see http://www.worldtimezone.com/faq.html - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse TZ offset. Hour value too large");; if (mins > 59) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse TZ offset. Minute value too large"); offset = (hours * 60) + mins; @@ -157,19 +157,19 @@ int git_signature__parse(git_signature *sig, const char **buffer_out, line_end = memchr(buffer, '\n', buffer_end - buffer); if (!line_end) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. No newline found");; if (buffer + (header_len + 1) > line_end) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Signature too short"); if (memcmp(buffer, header, header_len) != 0) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Expected prefix '%s' doesn't match actual", header); buffer += header_len; /* Parse name */ if ((name_end = memchr(buffer, '<', buffer_end - buffer)) == NULL) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Can't find e-mail start"); name_length = name_end - buffer - 1; sig->name = git__malloc(name_length + 1); @@ -181,11 +181,11 @@ int git_signature__parse(git_signature *sig, const char **buffer_out, buffer = name_end + 1; if (buffer >= line_end) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Ended unexpectedly"); /* Parse email */ if ((email_end = memchr(buffer, '>', buffer_end - buffer)) == NULL) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Can't find e-mail end"); email_length = email_end - buffer; sig->email = git__malloc(email_length + 1); @@ -197,10 +197,10 @@ int git_signature__parse(git_signature *sig, const char **buffer_out, buffer = email_end + 1; if (buffer >= line_end) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Ended unexpectedly"); if (git__strtol32(&time, buffer, &buffer, 10) < GIT_SUCCESS) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Timestamp isn't a number"); sig->when.time = (time_t)time; From 44dc0d261bb273d468e80d4a8242c6a4847b55b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 10 May 2011 18:56:44 +0200 Subject: [PATCH 13/34] Move tag.c to the new error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Martín Nieto --- src/tag.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/tag.c b/src/tag.c index 148eb280c..849429b7e 100644 --- a/src/tag.c +++ b/src/tag.c @@ -90,13 +90,13 @@ static int parse_tag_buffer(git_tag *tag, const char *buffer, const char *buffer int error; if ((error = git__parse_oid(&tag->target, &buffer, buffer_end, "object ")) < 0) - return error; + return git__rethrow(error, "Failed to parse tag. Object field invalid"); if (buffer + 5 >= buffer_end) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. Object too short"); if (memcmp(buffer, "type ", 5) != 0) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. Type field not found"); buffer += 5; tag->type = GIT_OBJ_BAD; @@ -105,7 +105,7 @@ static int parse_tag_buffer(git_tag *tag, const char *buffer, const char *buffer size_t type_length = strlen(tag_types[i]); if (buffer + type_length >= buffer_end) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. Object too short"); if (memcmp(buffer, tag_types[i], type_length) == 0) { tag->type = i; @@ -115,18 +115,19 @@ static int parse_tag_buffer(git_tag *tag, const char *buffer, const char *buffer } if (tag->type == GIT_OBJ_BAD) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. Invalid object type"); if (buffer + 4 >= buffer_end) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. Object too short"); if (memcmp(buffer, "tag ", 4) != 0) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. Tag field not found"); + buffer += 4; search = memchr(buffer, '\n', buffer_end - buffer); if (search == NULL) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. Object too short"); text_len = search - buffer; @@ -218,7 +219,7 @@ static int tag_create( } if (!git_odb_exists(repo->db, target)) - return GIT_ENOTFOUND; + return git__throw(GIT_ENOTFOUND, "Failed to create tag. Object to tag doesn't exist"); /* Try to find out what the type is */ if (target_type == GIT_OBJ_ANY) { From f4a936b56a6bb0cbe6d5f4b5c7c73066ad2f3197 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 11 May 2011 00:35:05 +0300 Subject: [PATCH 14/34] Bring back `git_strerror` We cannot totally deprecate this until the new error handling mechanisms are all in place. --- include/git2/errors.h | 12 +++++++++++ src/errors.c | 46 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/include/git2/errors.h b/include/git2/errors.h index fde0dc73d..47b0e240f 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -40,6 +40,18 @@ GIT_BEGIN_DECL */ GIT_EXTERN(const char *) git_lasterror(void); +/** + * strerror() for the Git library + * + * Get a string description for a given error code. + * NOTE: This method will be eventually deprecated in favor + * of the new `git_lasterror`. + * + * @param num The error code to explain + * @return a string explaining the error code + */ +GIT_EXTERN(const char *) git_strerror(int num); + /** @} */ GIT_END_DECL #endif diff --git a/src/errors.c b/src/errors.c index 40b0feb91..bf3810174 100644 --- a/src/errors.c +++ b/src/errors.c @@ -30,6 +30,52 @@ static GIT_TLS char g_last_error[1024]; +static struct { + int num; + const char *str; +} error_codes[] = { + {GIT_ERROR, "Unspecified error"}, + {GIT_ENOTOID, "Input was not a properly formatted Git object id."}, + {GIT_ENOTFOUND, "Object does not exist in the scope searched."}, + {GIT_ENOMEM, "Not enough space available."}, + {GIT_EOSERR, "Consult the OS error information."}, + {GIT_EOBJTYPE, "The specified object is of invalid type"}, + {GIT_EOBJCORRUPTED, "The specified object has its data corrupted"}, + {GIT_ENOTAREPO, "The specified repository is invalid"}, + {GIT_EINVALIDTYPE, "The object type is invalid or doesn't match"}, + {GIT_EMISSINGOBJDATA, "The object cannot be written that because it's missing internal data"}, + {GIT_EPACKCORRUPTED, "The packfile for the ODB is corrupted"}, + {GIT_EFLOCKFAIL, "Failed to adquire or release a file lock"}, + {GIT_EZLIB, "The Z library failed to inflate/deflate an object's data"}, + {GIT_EBUSY, "The queried object is currently busy"}, + {GIT_EINVALIDPATH, "The path is invalid"}, + {GIT_EBAREINDEX, "The index file is not backed up by an existing repository"}, + {GIT_EINVALIDREFNAME, "The name of the reference is not valid"}, + {GIT_EREFCORRUPTED, "The specified reference has its data corrupted"}, + {GIT_ETOONESTEDSYMREF, "The specified symbolic reference is too deeply nested"}, + {GIT_EPACKEDREFSCORRUPTED, "The pack-refs file is either corrupted of its format is not currently supported"}, + {GIT_EINVALIDPATH, "The path is invalid" }, + {GIT_EREVWALKOVER, "The revision walker is empty; there are no more commits left to iterate"}, + {GIT_EINVALIDREFSTATE, "The state of the reference is not valid"}, + {GIT_ENOTIMPLEMENTED, "This feature has not been implemented yet"}, + {GIT_EEXISTS, "A reference with this name already exists"}, + {GIT_EOVERFLOW, "The given integer literal is too large to be parsed"}, + {GIT_ENOTNUM, "The given literal is not a valid number"}, +}; + +const char *git_strerror(int num) +{ + size_t i; + + if (num == GIT_EOSERR) + return strerror(errno); + for (i = 0; i < ARRAY_SIZE(error_codes); i++) + if (num == error_codes[i].num) + return error_codes[i].str; + + return "Unknown error"; +} + int git__rethrow(int error, const char *msg, ...) { char new_error[1024]; From 6810bf28f8cecaeaf9cf32bee4bf0d5b55439819 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 11 May 2011 00:40:07 +0300 Subject: [PATCH 15/34] Move all error-related defines to `git2/errors.h` --- include/git2/common.h | 94 ------------------------------------------- include/git2/errors.h | 92 ++++++++++++++++++++++++++++++++++++++++++ src/common.h | 1 + 3 files changed, 93 insertions(+), 94 deletions(-) diff --git a/include/git2/common.h b/include/git2/common.h index 54af72fbf..9a27ac2e5 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -84,98 +84,6 @@ * @{ */ -/** Operation completed successfully. */ -#define GIT_SUCCESS 0 - -/** - * Operation failed, with unspecified reason. - * This value also serves as the base error code; all other - * error codes are subtracted from it such that all errors - * are < 0, in typical POSIX C tradition. - */ -#define GIT_ERROR -1 - -/** Input was not a properly formatted Git object id. */ -#define GIT_ENOTOID (GIT_ERROR - 1) - -/** Input does not exist in the scope searched. */ -#define GIT_ENOTFOUND (GIT_ERROR - 2) - -/** Not enough space available. */ -#define GIT_ENOMEM (GIT_ERROR - 3) - -/** Consult the OS error information. */ -#define GIT_EOSERR (GIT_ERROR - 4) - -/** The specified object is of invalid type */ -#define GIT_EOBJTYPE (GIT_ERROR - 5) - -/** The specified object has its data corrupted */ -#define GIT_EOBJCORRUPTED (GIT_ERROR - 6) - -/** The specified repository is invalid */ -#define GIT_ENOTAREPO (GIT_ERROR - 7) - -/** The object type is invalid or doesn't match */ -#define GIT_EINVALIDTYPE (GIT_ERROR - 8) - -/** The object cannot be written because it's missing internal data */ -#define GIT_EMISSINGOBJDATA (GIT_ERROR - 9) - -/** The packfile for the ODB is corrupted */ -#define GIT_EPACKCORRUPTED (GIT_ERROR - 10) - -/** Failed to acquire or release a file lock */ -#define GIT_EFLOCKFAIL (GIT_ERROR - 11) - -/** The Z library failed to inflate/deflate an object's data */ -#define GIT_EZLIB (GIT_ERROR - 12) - -/** The queried object is currently busy */ -#define GIT_EBUSY (GIT_ERROR - 13) - -/** The index file is not backed up by an existing repository */ -#define GIT_EBAREINDEX (GIT_ERROR - 14) - -/** The name of the reference is not valid */ -#define GIT_EINVALIDREFNAME (GIT_ERROR - 15) - -/** The specified reference has its data corrupted */ -#define GIT_EREFCORRUPTED (GIT_ERROR - 16) - -/** The specified symbolic reference is too deeply nested */ -#define GIT_ETOONESTEDSYMREF (GIT_ERROR - 17) - -/** The pack-refs file is either corrupted or its format is not currently supported */ -#define GIT_EPACKEDREFSCORRUPTED (GIT_ERROR - 18) - -/** The path is invalid */ -#define GIT_EINVALIDPATH (GIT_ERROR - 19) - -/** The revision walker is empty; there are no more commits left to iterate */ -#define GIT_EREVWALKOVER (GIT_ERROR - 20) - -/** The state of the reference is not valid */ -#define GIT_EINVALIDREFSTATE (GIT_ERROR - 21) - -/** This feature has not been implemented yet */ -#define GIT_ENOTIMPLEMENTED (GIT_ERROR - 22) - -/** A reference with this name already exists */ -#define GIT_EEXISTS (GIT_ERROR - 23) - -/** The given integer literal is too large to be parsed */ -#define GIT_EOVERFLOW (GIT_ERROR - 24) - -/** The given literal is not a valid number */ -#define GIT_ENOTNUM (GIT_ERROR - 25) - -/** Streaming error */ -#define GIT_ESTREAM (GIT_ERROR - 26) - -/** invalid arguments to function */ -#define GIT_EINVALIDARGS (GIT_ERROR - 27) - GIT_BEGIN_DECL typedef struct { @@ -185,8 +93,6 @@ typedef struct { GIT_EXTERN(void) git_strarray_free(git_strarray *array); -GIT_EXTERN(const char*) git_lasterror(void); - /** @} */ GIT_END_DECL #endif diff --git a/include/git2/errors.h b/include/git2/errors.h index 47b0e240f..dbe565aab 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -33,6 +33,98 @@ */ GIT_BEGIN_DECL +/** Operation completed successfully. */ +#define GIT_SUCCESS 0 + +/** + * Operation failed, with unspecified reason. + * This value also serves as the base error code; all other + * error codes are subtracted from it such that all errors + * are < 0, in typical POSIX C tradition. + */ +#define GIT_ERROR -1 + +/** Input was not a properly formatted Git object id. */ +#define GIT_ENOTOID (GIT_ERROR - 1) + +/** Input does not exist in the scope searched. */ +#define GIT_ENOTFOUND (GIT_ERROR - 2) + +/** Not enough space available. */ +#define GIT_ENOMEM (GIT_ERROR - 3) + +/** Consult the OS error information. */ +#define GIT_EOSERR (GIT_ERROR - 4) + +/** The specified object is of invalid type */ +#define GIT_EOBJTYPE (GIT_ERROR - 5) + +/** The specified object has its data corrupted */ +#define GIT_EOBJCORRUPTED (GIT_ERROR - 6) + +/** The specified repository is invalid */ +#define GIT_ENOTAREPO (GIT_ERROR - 7) + +/** The object type is invalid or doesn't match */ +#define GIT_EINVALIDTYPE (GIT_ERROR - 8) + +/** The object cannot be written because it's missing internal data */ +#define GIT_EMISSINGOBJDATA (GIT_ERROR - 9) + +/** The packfile for the ODB is corrupted */ +#define GIT_EPACKCORRUPTED (GIT_ERROR - 10) + +/** Failed to acquire or release a file lock */ +#define GIT_EFLOCKFAIL (GIT_ERROR - 11) + +/** The Z library failed to inflate/deflate an object's data */ +#define GIT_EZLIB (GIT_ERROR - 12) + +/** The queried object is currently busy */ +#define GIT_EBUSY (GIT_ERROR - 13) + +/** The index file is not backed up by an existing repository */ +#define GIT_EBAREINDEX (GIT_ERROR - 14) + +/** The name of the reference is not valid */ +#define GIT_EINVALIDREFNAME (GIT_ERROR - 15) + +/** The specified reference has its data corrupted */ +#define GIT_EREFCORRUPTED (GIT_ERROR - 16) + +/** The specified symbolic reference is too deeply nested */ +#define GIT_ETOONESTEDSYMREF (GIT_ERROR - 17) + +/** The pack-refs file is either corrupted or its format is not currently supported */ +#define GIT_EPACKEDREFSCORRUPTED (GIT_ERROR - 18) + +/** The path is invalid */ +#define GIT_EINVALIDPATH (GIT_ERROR - 19) + +/** The revision walker is empty; there are no more commits left to iterate */ +#define GIT_EREVWALKOVER (GIT_ERROR - 20) + +/** The state of the reference is not valid */ +#define GIT_EINVALIDREFSTATE (GIT_ERROR - 21) + +/** This feature has not been implemented yet */ +#define GIT_ENOTIMPLEMENTED (GIT_ERROR - 22) + +/** A reference with this name already exists */ +#define GIT_EEXISTS (GIT_ERROR - 23) + +/** The given integer literal is too large to be parsed */ +#define GIT_EOVERFLOW (GIT_ERROR - 24) + +/** The given literal is not a valid number */ +#define GIT_ENOTNUM (GIT_ERROR - 25) + +/** Streaming error */ +#define GIT_ESTREAM (GIT_ERROR - 26) + +/** invalid arguments to function */ +#define GIT_EINVALIDARGS (GIT_ERROR - 27) + /** * 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 e5b9f15ed..f4f11fd2f 100644 --- a/src/common.h +++ b/src/common.h @@ -50,6 +50,7 @@ typedef SSIZE_T ssize_t; #include "git2/common.h" #include "git2/types.h" +#include "git2/errors.h" #include "thread-utils.h" #include "bswap.h" From 40774549e14e2d9f24b9271173d58b44f82d5254 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 11 May 2011 00:42:43 +0300 Subject: [PATCH 16/34] libgit2 v0.12.0 "absolutely no reason" Hey, welcome to yet another minor libgit2 release. Sorry for the delay from the last one. As you'll see the changelog is quite extensive -- hopefully from now on we'll stick to more frequent minor releases. Together with the usual bugfixes, here's a list of the new key features: * Distfiles This version comes with proper distfiles as requested in #131. These are available in the Downloads section of the GitHub project. * Error handling A new error handling API has been implemented that allows the library to return detailed error messages together with the generic error codes. We hope this will be a great when wrapping and integrating the library New external method to get the last detailed error message: + git_lasterror(void) The old `git_strerror` still exists, but will be deprecated in the future as soon as every method in the library returns a valid error message. The task of writing error messages for every method is quite daunting. We appreciate pull requests with more error messages. Check the new error handling documentation in the following commit: https://github.com/libgit2/libgit2/commit/fa59f18d0ddbbb98d45e33934fb0efc3e2bf1557 * Redis backend We now have a Redis backend courtesy of Dmitry Kovega. Just like the SQLite backend, this allows the library to store Git objects in a Redis key-value store. The backend requires the `hiredis` library. Use `--with-redis` when building libgit2 to enable building the backend if `hiredis` is available. * Commits New methods to access tree and parent data as a raw OID value instead of forcing a repository lookup + git_commit_tree_oid(git_commit *commit) + git_commit_parent_oid(git_commit *commit, unsigned int n) * Index The `git_index_add` method has been split into 4 different calls which allow for appending and replacing in-memory entries and on-disk files to the index. + git_index_add(git_index *index, const char *path, int stage) + git_index_add2(git_index *index, const git_index_entry *source_entry) + git_index_append(git_index *index, const char *path, int stage) + git_index_append2(git_index *index, const git_index_entry *source_entry) Index entries can now also be efficiently removed from the index: + git_index_remove(git_index *index, int position) * References Methods to force the creation and renaming of references, even if those already exist on the repository. + git_reference_create_symbolic_f(git_reference **ref_out, git_repository *repo, const char *name, const char *target) + git_reference_create_oid_f(git_reference **ref_out, git_repository *repo, const char *name, const git_oid *id) + git_reference_rename_f(git_reference *ref, const char *new_name) * Repository New auxiliary methods with repository information + git_repository_is_empty(git_repository *repo) + git_repository_path(git_repository *repo) + git_repository_workdir(git_repository *repo) * Signatures New method to create a signature with the current date/time + git_signature_now(const char *name, const char *email) * Tags Several wrappers to automate tag creation. + git_tag_create_frombuffer(git_oid *oid, git_repository *repo, const char *buffer) + git_tag_create_f(git_oid *oid, git_repository *repo, const char *tag_name, const git_oid *target, git_otype target_type, const git_signature *tagger, const char *message); + git_tag_create_fo(git_oid *oid, git_repository *repo, const char *tag_name, const git_object *target, const git_signature *tagger, const char *message) New functionality to delete and list tags in a repository without having to resort to the `references` API. + git_tag_delete(git_repository *repo, const char *tag_name) + git_tag_list(git_strarray *tag_names, git_repository *repo) * Trees All instances of `git_tree_entry` are now returned and handled as constant, to remind the user that these opaque types are not supposed to be manually free'd. The `git_tree_entry_2object` method now takes a `git_repository` argument which defines in which repository the resolved object should be looked up. (It is expected to be the same repository that contains the parent `git_tree` for the entry). + git_tree_entry_2object(git_object **object_out, git_repository *repo, const git_tree_entry *entry) New opaque type `git_treebuilder` with functionality to create and write trees on memory + git_treebuilder_create(git_treebuilder **builder_p, const git_tree *source) + git_treebuilder_clear(git_treebuilder *bld) + git_treebuilder_free(git_treebuilder *bld) + git_treebuilder_get(git_treebuilder *bld, const char *filename) + git_treebuilder_insert(git_tree_entry **entry_out, git_treebuilder *bld, const char *filename, const git_oid *id, unsigned int attributes) + git_treebuilder_remove(git_treebuilder *bld, const char *filename) + git_treebuilder_filter(git_treebuilder *bld, int (*filter)(const git_tree_entry *, void *), void *payload) + git_treebuilder_write(git_oid *oid, git_repository *repo, git_treebuilder *bld) New method to write an index file as a tree to the ODB. + git_tree_create_fromindex(git_oid *oid, git_index *index) Thanks to the usual guility parties that make this this happen, to all the new contributors who are starting to submit pull requests, and to the bindings developers who have to keep up with our shit. Feedback and questions welcome on libgit2@librelist.org Signed-off-by: Vicent Marti --- include/git2.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/git2.h b/include/git2.h index 5756d7e90..d44c3f8df 100644 --- a/include/git2.h +++ b/include/git2.h @@ -26,9 +26,9 @@ #ifndef INCLUDE_git_git_h__ #define INCLUDE_git_git_h__ -#define LIBGIT2_VERSION "0.11.0" +#define LIBGIT2_VERSION "0.12.0" #define LIBGIT2_VER_MAJOR 0 -#define LIBGIT2_VER_MINOR 11 +#define LIBGIT2_VER_MINOR 12 #define LIBGIT2_VER_REVISION 0 #include "git2/common.h" From 0e0f4fdeee8e6009187b84a26b4b8c02452ff224 Mon Sep 17 00:00:00 2001 From: Daniel Huckstep Date: Tue, 10 May 2011 15:33:07 -0700 Subject: [PATCH 17/34] Add --with-redis flag to list of available build flags in README. --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index dae6a76bf..b6779bd84 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,9 @@ The waf build system for libgit2 accepts the following flags: --with-sqlite Enable sqlite support. + --with-redis + Enable redis support. + You can run `./waf --help` to see a full list of install options and targets. From cbcaf0c09cd50ee705ea9740d57b722900f4e723 Mon Sep 17 00:00:00 2001 From: schu Date: Wed, 11 May 2011 12:09:40 +0200 Subject: [PATCH 18/34] Move blob.c to the new error handling Signed-off-by: schu --- src/blob.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/blob.c b/src/blob.c index 5e3c22fbf..987169358 100644 --- a/src/blob.c +++ b/src/blob.c @@ -62,7 +62,7 @@ int git_blob_create_frombuffer(git_oid *oid, git_repository *repo, const void *b git_odb_stream *stream; if ((error = git_odb_open_wstream(&stream, repo->db, len, GIT_OBJ_BLOB)) < GIT_SUCCESS) - return error; + return git__rethrow(error, "Failed to create blob. Can't open write stream"); stream->write(stream, buffer, len); @@ -81,7 +81,7 @@ int git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *pat git_odb_stream *stream; if (repo->path_workdir == NULL) - return GIT_ENOTFOUND; + return git__throw(GIT_ENOTFOUND, "Failed to create blob. No workdir given"); git__joinpath(full_path, repo->path_workdir, path); @@ -106,7 +106,7 @@ int git_blob_create_fromfile(git_oid *oid, git_repository *repo, const char *pat if (read_len < 0) { gitfo_close(fd); stream->free(stream); - return GIT_EOSERR; + return git__throw(GIT_EOSERR, "Failed to create blob. Can't read full file"); } stream->write(stream, buffer, read_len); From d6de92b6fe5d04111c3e9cd4a0c269c840ce59b6 Mon Sep 17 00:00:00 2001 From: schu Date: Wed, 11 May 2011 12:40:04 +0200 Subject: [PATCH 19/34] Move tree.c to the new error handling Signed-off-by: schu --- src/tree.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/tree.c b/src/tree.c index b7daf39c4..6f9fc8607 100644 --- a/src/tree.c +++ b/src/tree.c @@ -151,15 +151,15 @@ static int tree_parse_buffer(git_tree *tree, const char *buffer, const char *buf return GIT_ENOMEM; if (git__strtol32((long *)&entry->attr, buffer, &buffer, 8) < GIT_SUCCESS) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tree. Can't parse attributes"); if (*buffer++ != ' ') { - error = GIT_EOBJCORRUPTED; + error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse tree. Object it corrupted"); break; } if (memchr(buffer, 0, buffer_end - buffer) == NULL) { - error = GIT_EOBJCORRUPTED; + error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse tree. Object it corrupted"); break; } @@ -274,7 +274,7 @@ int git_tree_create_fromindex(git_oid *oid, git_index *index) int error; if (index->repository == NULL) - return GIT_EBAREINDEX; + return git__throw(GIT_EBAREINDEX, "Failed to create tree. The index file is not backed up by an existing repository"); error = write_index(oid, index, "", 0, 0, git_index_entrycount(index)); return (error < GIT_SUCCESS) ? error : GIT_SUCCESS; @@ -343,7 +343,7 @@ int git_treebuilder_insert(git_tree_entry **entry_out, git_treebuilder *bld, con assert(bld && id && filename); if (!valid_attributes(attributes)) - return GIT_ERROR; + return git__throw(GIT_ERROR, "Failed to insert entry. Invalid atrributes"); if ((pos = git_vector_bsearch2(&bld->entries, entry_search_cmp, filename)) != GIT_ENOTFOUND) { entry = git_vector_get(&bld->entries, pos); @@ -400,7 +400,7 @@ int git_treebuilder_remove(git_treebuilder *bld, const char *filename) git_tree_entry *remove_ptr = (git_tree_entry *)git_treebuilder_get(bld, filename); if (remove_ptr == NULL || remove_ptr->removed) - return GIT_ENOTFOUND; + return git__throw(GIT_ENOTFOUND, "Failed to remove entry. File isn't in the tree"); remove_ptr->removed = 1; bld->entry_count--; @@ -431,7 +431,7 @@ int git_treebuilder_write(git_oid *oid, git_repository *repo, git_treebuilder *b } if ((error = git_odb_open_wstream(&stream, git_repository_database(repo), size, GIT_OBJ_TREE)) < GIT_SUCCESS) - return error; + return git__rethrow(error, "Failed to write tree. Can't open write stream"); for (i = 0; i < bld->entries.length; ++i) { git_tree_entry *entry = bld->entries.contents[i]; From 86f5fa7810c5675d5c789450c9c140330432bbbb Mon Sep 17 00:00:00 2001 From: schu Date: Wed, 11 May 2011 14:00:38 +0200 Subject: [PATCH 20/34] Move vector.c to the new error handling Remove "redundant" check for v->_cmp in wrapper function git_vector_bsearch(). Signed-off-by: schu --- src/vector.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/vector.c b/src/vector.c index d0b0c5c56..1ddc26e3e 100644 --- a/src/vector.c +++ b/src/vector.c @@ -106,7 +106,7 @@ int git_vector_bsearch2(git_vector *v, git_vector_cmp key_lookup, const void *ke /* need comparison function to sort the vector */ if (v->_cmp == NULL) - return GIT_ENOTFOUND; + return git__throw(GIT_ENOTFOUND, "Can't sort vector. No comparison function set"); git_vector_sort(v); @@ -114,7 +114,7 @@ int git_vector_bsearch2(git_vector *v, git_vector_cmp key_lookup, const void *ke if (find != NULL) return (int)(find - v->contents); - return GIT_ENOTFOUND; + return git__throw(GIT_ENOTFOUND, "Can't find element"); } int git_vector_search2(git_vector *v, git_vector_cmp key_lookup, const void *key) @@ -128,7 +128,7 @@ int git_vector_search2(git_vector *v, git_vector_cmp key_lookup, const void *key return i; } - return GIT_ENOTFOUND; + return git__throw(GIT_ENOTFOUND, "Can't find element"); } static int strict_comparison(const void *a, const void *b) @@ -143,9 +143,6 @@ int git_vector_search(git_vector *v, const void *entry) int git_vector_bsearch(git_vector *v, const void *key) { - if (v->_cmp == NULL) - return GIT_ENOTFOUND; - return git_vector_bsearch2(v, v->_cmp, key); } @@ -156,7 +153,7 @@ int git_vector_remove(git_vector *v, unsigned int idx) assert(v); if (idx >= v->length || v->length == 0) - return GIT_ENOTFOUND; + return git__throw(GIT_ENOTFOUND, "Can't remove element. Index out of bounds"); for (i = idx; i < v->length - 1; ++i) v->contents[i] = v->contents[i + 1]; From b51c92693d07dab0cb31311b9ad6b96aac127779 Mon Sep 17 00:00:00 2001 From: schu Date: Wed, 11 May 2011 14:44:44 +0200 Subject: [PATCH 21/34] Move revwalk.c to the new error handling Signed-off-by: schu --- src/revwalk.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index 78798480f..a32a16e2a 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -210,7 +210,7 @@ static int commit_quick_parse(git_revwalk *walk, commit_object *commit, git_rawo git_oid oid; if (git_oid_mkstr(&oid, (char *)buffer + STRLEN("parent ")) < GIT_SUCCESS) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse commit. Parent object is corrupted"); commit->parents[i] = commit_lookup(walk, &oid); if (commit->parents[i] == NULL) @@ -222,14 +222,14 @@ static int commit_quick_parse(git_revwalk *walk, commit_object *commit, git_rawo commit->out_degree = (unsigned short)parents; if ((buffer = memchr(buffer, '\n', buffer_end - buffer)) == NULL) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse commit. Object is corrupted"); buffer = memchr(buffer, '>', buffer_end - buffer); if (buffer == NULL) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse commit. Can't find author"); if (git__strtol32(&commit_time, (char *)buffer + 2, NULL, 10) < GIT_SUCCESS) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to parse commit. Can't parse commit time"); commit->time = (time_t)commit_time; commit->parsed = 1; @@ -245,11 +245,11 @@ static int commit_parse(git_revwalk *walk, commit_object *commit) return GIT_SUCCESS; if ((error = git_odb_read(&obj, walk->repo->db, &commit->oid)) < GIT_SUCCESS) - return error; + return git__rethrow(error, "Failed to parse commit. Can't read object"); if (obj->raw.type != GIT_OBJ_COMMIT) { git_odb_object_close(obj); - return GIT_EOBJTYPE; + return git__throw(GIT_EOBJTYPE, "Failed to parse commit. Object is no commit object"); } error = commit_quick_parse(walk, commit, &obj->raw); @@ -305,7 +305,7 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting) commit = commit_lookup(walk, oid); if (commit == NULL) - return GIT_ENOTFOUND; + return git__throw(GIT_ENOTFOUND, "Failed to push commit. Object not found"); commit->uninteresting = uninteresting; @@ -349,7 +349,7 @@ static int revwalk_next_timesort(commit_object **object_out, git_revwalk *walk) } } - return GIT_EREVWALKOVER; + return git__throw(GIT_EREVWALKOVER, "No more commits left to iterate"); } static int revwalk_next_unsorted(commit_object **object_out, git_revwalk *walk) @@ -367,7 +367,7 @@ static int revwalk_next_unsorted(commit_object **object_out, git_revwalk *walk) } } - return GIT_EREVWALKOVER; + return git__throw(GIT_EREVWALKOVER, "No more commits left to iterate"); } static int revwalk_next_toposort(commit_object **object_out, git_revwalk *walk) @@ -378,7 +378,7 @@ static int revwalk_next_toposort(commit_object **object_out, git_revwalk *walk) for (;;) { next = commit_list_pop(&walk->iterator_topo); if (next == NULL) - return GIT_EREVWALKOVER; + return git__throw(GIT_EREVWALKOVER, "No more commits left to iterate"); if (next->in_degree > 0) { next->topo_delay = 1; From f31bd03f7eed1bcabbd9561b444661e19b0f2567 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 13 May 2011 04:15:46 +0300 Subject: [PATCH 22/34] Include "common.h" in "errors.h" --- include/git2/errors.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/git2/errors.h b/include/git2/errors.h index dbe565aab..60af7b3d0 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -25,6 +25,8 @@ #ifndef INCLUDE_git_errors_h__ #define INCLUDE_git_errors_h__ +#include "common.h" + /** * @file git2/errors.h * @brief Git error handling routines and variables From 098173c52adabf313e99c48475f51cd0d1e942a5 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 13 May 2011 04:17:24 +0300 Subject: [PATCH 23/34] Check Redis replies for NULL --- src/backends/hiredis.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/backends/hiredis.c b/src/backends/hiredis.c index 707412bf6..2533a16da 100644 --- a/src/backends/hiredis.c +++ b/src/backends/hiredis.c @@ -53,7 +53,7 @@ int hiredis_backend__read_header(size_t *len_p, git_otype *type_p, git_odb_backe reply = redisCommand(backend->db, "HMGET %b %s %s", oid->id, GIT_OID_RAWSZ, "type", "size"); - if (reply->type == REDIS_REPLY_ARRAY) { + if (reply && reply->type == REDIS_REPLY_ARRAY) { if (reply->element[0]->type != REDIS_REPLY_NIL && reply->element[0]->type != REDIS_REPLY_NIL) { *type_p = (git_otype) atoi(reply->element[0]->str); @@ -83,7 +83,7 @@ int hiredis_backend__read(void **data_p, size_t *len_p, git_otype *type_p, git_o reply = redisCommand(backend->db, "HMGET %b %s %s %s", oid->id, GIT_OID_RAWSZ, "type", "size", "data"); - if (reply->type == REDIS_REPLY_ARRAY) { + if (reply && reply->type == REDIS_REPLY_ARRAY) { if (reply->element[0]->type != REDIS_REPLY_NIL && reply->element[1]->type != REDIS_REPLY_NIL && reply->element[2]->type != REDIS_REPLY_NIL) { @@ -118,7 +118,7 @@ int hiredis_backend__exists(git_odb_backend *_backend, const git_oid *oid) { found = 0; reply = redisCommand(backend->db, "exists %b", oid->id, GIT_OID_RAWSZ); - if (reply->type != REDIS_REPLY_NIL && reply->type != REDIS_REPLY_ERROR) + if (reply && reply->type != REDIS_REPLY_NIL && reply->type != REDIS_REPLY_ERROR) found = 1; @@ -144,7 +144,8 @@ int hiredis_backend__write(git_oid *id, git_odb_backend *_backend, const void *d "size %d " "data %b ", id->id, GIT_OID_RAWSZ, (int) type, len, data, len); - error = reply->type == REDIS_REPLY_ERROR ? GIT_ERROR : GIT_SUCCESS; + + error = (reply == NULL || reply->type == REDIS_REPLY_ERROR) ? GIT_ERROR : GIT_SUCCESS; freeReplyObject(reply); return error; From 77c3999ca928539c6ba9a29910bbe99d2b705efd Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sat, 14 May 2011 14:40:56 +0200 Subject: [PATCH 24/34] Move fileops.c to the new error handling --- src/fileops.c | 61 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/src/fileops.c b/src/fileops.c index 5dd4a3806..45dd8f444 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -10,7 +10,7 @@ int gitfo_mkdir_2file(const char *file_path) error = git__dirname_r(target_folder_path, sizeof(target_folder_path), file_path); if (error < GIT_SUCCESS) - return error; + return git__throw(GIT_EINVALIDPATH, "Failed to recursively build `%s` tree structure. Unable to parse parent folder name", file_path); /* Does the containing folder exist? */ if (gitfo_isdir(target_folder_path)) { @@ -19,7 +19,7 @@ int gitfo_mkdir_2file(const char *file_path) /* Let's create the tree structure */ error = gitfo_mkdir_recurs(target_folder_path, mode); if (error < GIT_SUCCESS) - return error; + return error; /* The callee already takes care of setting the correct error message. */ } return GIT_SUCCESS; @@ -164,19 +164,19 @@ int gitfo_read_file(gitfo_buf *obj, const char *path) if (((size = gitfo_size(fd)) < 0) || !git__is_sizet(size+1)) { gitfo_close(fd); - return GIT_ERROR; + return git__throw(GIT_ERROR, "Failed to read file `%s`. Either an error occured while calculating its size or the file is too large", path); } len = (size_t) size; if ((buff = git__malloc(len + 1)) == NULL) { gitfo_close(fd); - return GIT_ERROR; + return GIT_ENOMEM; } if (gitfo_read(fd, buff, len) < 0) { gitfo_close(fd); free(buff); - return GIT_ERROR; + return git__throw(GIT_ERROR, "Failed to read file `%s`", path); } buff[len] = '\0'; @@ -197,13 +197,15 @@ void gitfo_free_buf(gitfo_buf *obj) int gitfo_mv(const char *from, const char *to) { + int error; + #ifdef GIT_WIN32 /* * Win32 POSIX compilance my ass. If the destination * file exists, the `rename` call fails. This is as * close as it gets with the Win32 API. */ - return MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED) ? GIT_SUCCESS : GIT_EOSERR; + error = MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED) ? GIT_SUCCESS : GIT_EOSERR; #else /* Don't even try this on Win32 */ if (!link(from, to)) { @@ -214,16 +216,21 @@ int gitfo_mv(const char *from, const char *to) if (!rename(from, to)) return GIT_SUCCESS; - return GIT_EOSERR; + error = GIT_EOSERR; #endif + + if (error < GIT_SUCCESS) + return git__throw(error, "Failed to move file from `%s`to `%s`", from, to); + + return GIT_SUCCESS; } int gitfo_mv_force(const char *from, const char *to) { if (gitfo_mkdir_2file(to) < GIT_SUCCESS) - return GIT_EOSERR; + return GIT_EOSERR; /* The callee already takes care of setting the correct error message. */ - return gitfo_mv(from, to); + return gitfo_mv(from, to); /* The callee already takes care of setting the correct error message. */ } int gitfo_map_ro(git_map *out, git_file fd, git_off_t begin, size_t len) @@ -338,7 +345,7 @@ int gitfo_dirent( struct dirent *de; if (!wd_len || path_sz < wd_len + 2) - return GIT_ERROR; + return git__throw(GIT_EINVALIDARGS, "Failed to process `%s` tree structure. Path is either empty or buffer size is too short", path); while (path[wd_len - 1] == '/') wd_len--; @@ -347,7 +354,7 @@ int gitfo_dirent( dir = opendir(path); if (!dir) - return GIT_EOSERR; + return git__throw(GIT_EOSERR, "Failed to process `%s` tree structure. An error occured while opening the directory", path); while ((de = readdir(dir)) != NULL) { size_t de_len; @@ -364,14 +371,14 @@ int gitfo_dirent( de_len = strlen(de->d_name); if (path_sz < wd_len + de_len + 1) { closedir(dir); - return GIT_ERROR; + return git__throw(GIT_ERROR, "Failed to process `%s` tree structure. Buffer size is too short", path); } strcpy(path + wd_len, de->d_name); result = fn(arg, path); if (result < GIT_SUCCESS) { closedir(dir); - return result; + return result; /* The callee is reponsible for setting the correct error message */ } if (result > 0) { closedir(dir); @@ -399,7 +406,7 @@ int retrieve_path_root_offset(const char *path) if (*(path + offset) == '/') return offset; - return GIT_ERROR; + return -1; /* Not a real error. Rather a signal than the path is not rooted */ } @@ -438,7 +445,11 @@ int gitfo_mkdir_recurs(const char *path, int mode) error = gitfo_mkdir(path, mode); free(path_copy); - return error; + + if (error < GIT_SUCCESS) + return git__throw(error, "Failed to recursively create `%s` tree structure", path); + + return GIT_SUCCESS; } static int retrieve_previous_path_component_start(const char *path) @@ -484,7 +495,7 @@ int gitfo_prettify_dir_path(char *buffer_out, size_t size, const char *path) if (root_path_offset < 0) { error = gitfo_getcwd(buffer_out, size); if (error < GIT_SUCCESS) - return error; + return error; /* The callee already takes care of setting the correct error message. */ len = strlen(buffer_out); buffer_out += len; @@ -529,7 +540,7 @@ int gitfo_prettify_dir_path(char *buffer_out, size_t size, const char *path) /* Are we escaping out of the root dir? */ if (len < 0) - return GIT_EINVALIDPATH; + return git__throw(GIT_EINVALIDPATH, "Failed to normalize path `%s`. The path escapes out of the root directory", path); buffer_out = (char *)buffer_out_start + len; continue; @@ -537,7 +548,7 @@ int gitfo_prettify_dir_path(char *buffer_out, size_t size, const char *path) /* Guard against potential multiple dot path traversal (cf http://cwe.mitre.org/data/definitions/33.html) */ if (only_dots && segment_len > 0) - return GIT_EINVALIDPATH; + return git__throw(GIT_EINVALIDPATH, "Failed to normalize path `%s`. The path contains a segment with three `.` or more", path); *buffer_out++ = '/'; len++; @@ -557,21 +568,21 @@ int gitfo_prettify_file_path(char *buffer_out, size_t size, const char *path) /* Let's make sure the filename isn't empty nor a dot */ if (path_len == 0 || (path_len == 1 && *path == '.')) - return GIT_EINVALIDPATH; + return git__throw(GIT_EINVALIDPATH, "Failed to normalize file path `%s`. The path is either empty or equals `.`", path); /* Let's make sure the filename doesn't end with "/", "/." or "/.." */ for (i = 1; path_len > i && i < 4; i++) { if (!strncmp(path + path_len - i, pattern, i)) - return GIT_EINVALIDPATH; + return git__throw(GIT_EINVALIDPATH, "Failed to normalize file path `%s`. The path points to a folder", path); } error = gitfo_prettify_dir_path(buffer_out, size, path); if (error < GIT_SUCCESS) - return error; + return error; /* The callee already takes care of setting the correct error message. */ path_len = strlen(buffer_out); - if (path_len < 2) - return GIT_EINVALIDPATH; + if (path_len < 2) /* TODO: Fixme. We should also take of detecting Windows rooted path (probably through usage of retrieve_path_root_offset) */ + return git__throw(GIT_EINVALIDPATH, "Failed to normalize file path `%s`. The path points to a folder", path); /* Remove the trailing slash */ buffer_out[path_len - 1] = '\0'; @@ -616,11 +627,11 @@ int gitfo_getcwd(char *buffer_out, size_t size) #ifdef GIT_WIN32 cwd_buffer = _getcwd(buffer_out, size); #else - cwd_buffer = getcwd(buffer_out, size); //TODO: Fixme. Ensure the required headers are correctly included + cwd_buffer = getcwd(buffer_out, size); #endif if (cwd_buffer == NULL) - return GIT_EOSERR; + return git__throw(GIT_EOSERR, "Failed to retrieve current working directory"); posixify_path(buffer_out); From 3abe3bba5ab234a8fcbf4cc7a50adc86323d7287 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sat, 14 May 2011 16:05:33 +0200 Subject: [PATCH 25/34] Move repository.c to the new error handling --- src/cache.c | 2 +- src/repository.c | 51 ++++++++++++++++++++++++++---------------------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/cache.c b/src/cache.c index fd42e2c5b..a372df160 100644 --- a/src/cache.c +++ b/src/cache.c @@ -51,7 +51,7 @@ void git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_p cache->lru_count = 0; cache->free_obj = free_ptr; - cache->nodes = git__malloc((size + 1) * sizeof(cache_node)); + cache->nodes = git__malloc((size + 1) * sizeof(cache_node)); //TODO: How should we deal with GIT_ENOMEM? for (i = 0; i < (size + 1); ++i) { git_mutex_init(&cache->nodes[i].lock); diff --git a/src/repository.c b/src/repository.c index 8cc2644ca..8a5934f59 100644 --- a/src/repository.c +++ b/src/repository.c @@ -124,16 +124,16 @@ static int check_repository_dirs(git_repository *repo) char path_aux[GIT_PATH_MAX]; if (gitfo_isdir(repo->path_repository) < GIT_SUCCESS) - return GIT_ENOTAREPO; + return git__throw(GIT_ENOTAREPO, "`%s` is not a folder", repo->path_repository); /* Ensure GIT_OBJECT_DIRECTORY exists */ if (gitfo_isdir(repo->path_odb) < GIT_SUCCESS) - return GIT_ENOTAREPO; + return git__throw(GIT_ENOTAREPO, "`%s` does not exist", repo->path_odb); /* Ensure HEAD file exists */ git__joinpath(path_aux, repo->path_repository, GIT_HEAD_FILE); if (gitfo_exists(path_aux) < 0) - return GIT_ENOTAREPO; + return git__throw(GIT_ENOTAREPO, "HEAD file is missing"); return GIT_SUCCESS; } @@ -145,12 +145,12 @@ static int guess_repository_dirs(git_repository *repo, const char *repository_pa /* Git directory name */ if (git__basename_r(buffer, sizeof(buffer), repository_path) < 0) - return GIT_EINVALIDPATH; + return git__throw(GIT_EINVALIDPATH, "Unable to parse folder name from `%s`", repository_path); if (strcmp(buffer, DOT_GIT) == 0) { /* Path to working dir */ if (git__dirname_r(buffer, sizeof(buffer), repository_path) < 0) - return GIT_EINVALIDPATH; + return git__throw(GIT_EINVALIDPATH, "Unable to parse parent folder name from `%s`", repository_path); path_work_tree = buffer; } @@ -177,7 +177,7 @@ static git_repository *repository_alloc() static int init_odb(git_repository *repo) { - return git_odb_open(&repo->db, repo->path_odb); + return git_odb_open(&repo->db, repo->path_odb); /* TODO: Move odb.c to new error handling */ } int git_repository_open3(git_repository **repo_out, @@ -192,7 +192,7 @@ int git_repository_open3(git_repository **repo_out, assert(repo_out); if (object_database == NULL) - return GIT_ERROR; + return git__throw(GIT_EINVALIDARGS, "Failed to open repository. `object_database` can't be null"); repo = repository_alloc(); if (repo == NULL) @@ -218,7 +218,7 @@ int git_repository_open3(git_repository **repo_out, cleanup: git_repository_free(repo); - return error; + return git__rethrow(error, "Failed to open repository"); } @@ -259,7 +259,7 @@ int git_repository_open2(git_repository **repo_out, cleanup: git_repository_free(repo); - return error; + return git__rethrow(error, "Failed to open repository"); } int git_repository_open(git_repository **repo_out, const char *path) @@ -290,7 +290,7 @@ int git_repository_open(git_repository **repo_out, const char *path) cleanup: git_repository_free(repo); - return error; + return git__rethrow(error, "Failed to open repository"); } void git_repository_free(git_repository *repo) @@ -322,7 +322,7 @@ int git_repository_index(git_index **index_out, git_repository *repo) assert(index_out && repo); if (repo->index == NULL) { - error = git_index_open_inrepo(&repo->index, repo); + error = git_index_open_inrepo(&repo->index, repo); /* TODO: move index.c to new error handling */ if (error < GIT_SUCCESS) return error; @@ -349,7 +349,7 @@ static int repo_init_reinit(repo_init *results) static int repo_init_createhead(git_repository *repo) { git_reference *head_reference; - return git_reference_create_symbolic(&head_reference, repo, GIT_HEAD_FILE, GIT_REFS_HEADS_MASTER_FILE); + return git_reference_create_symbolic(&head_reference, repo, GIT_HEAD_FILE, GIT_REFS_HEADS_MASTER_FILE); /* TODO: finalize moving refs.c to new error handling */ } static int repo_init_check_head_existence(char * repository_path) @@ -363,6 +363,7 @@ static int repo_init_check_head_existence(char * repository_path) static int repo_init_structure(repo_init *results) { const int mode = 0755; /* or 0777 ? */ + int error; char temp_path[GIT_PATH_MAX]; char *git_dir = results->path_repository; @@ -372,23 +373,27 @@ static int repo_init_structure(repo_init *results) /* Creates the '/objects/info/' directory */ git__joinpath(temp_path, git_dir, GIT_OBJECTS_INFO_DIR); - if (gitfo_mkdir_recurs(temp_path, mode) < GIT_SUCCESS) - return GIT_ERROR; + error = gitfo_mkdir_recurs(temp_path, mode); + if (error < GIT_SUCCESS) + return error; /* Creates the '/objects/pack/' directory */ git__joinpath(temp_path, git_dir, GIT_OBJECTS_PACK_DIR); - if (gitfo_mkdir(temp_path, mode)) - return GIT_ERROR; + error = gitfo_mkdir(temp_path, mode); + if (error < GIT_SUCCESS) + return git__throw(error, "Unable to create `%s` folder", temp_path); /* Creates the '/refs/heads/' directory */ git__joinpath(temp_path, git_dir, GIT_REFS_HEADS_DIR); - if (gitfo_mkdir_recurs(temp_path, mode)) - return GIT_ERROR; + error = gitfo_mkdir_recurs(temp_path, mode); + if (error < GIT_SUCCESS) + return error; /* Creates the '/refs/tags/' directory */ git__joinpath(temp_path, git_dir, GIT_REFS_TAGS_DIR); - if (gitfo_mkdir(temp_path, mode)) - return GIT_ERROR; + error = gitfo_mkdir(temp_path, mode); + if (error < GIT_SUCCESS) + return git__throw(error, "Unable to create `%s` folder", temp_path); /* TODO: what's left? templates? */ @@ -467,7 +472,7 @@ int git_repository_init(git_repository **repo_out, const char *path, unsigned is cleanup: free(results.path_repository); git_repository_free(repo); - return error; + return git__rethrow(error, "Failed to (re)init the repository `%s`", path); } int git_repository_is_empty(git_repository *repo) @@ -477,10 +482,10 @@ int git_repository_is_empty(git_repository *repo) error = git_reference_lookup(&head, repo, "HEAD"); if (error < GIT_SUCCESS) - return error; + return git__throw(error, "Failed to determine the emptiness of the repository. An error occured while retrieving the HEAD reference"); if (git_reference_type(head) != GIT_REF_SYMBOLIC) - return GIT_EOBJCORRUPTED; + return git__throw(GIT_EOBJCORRUPTED, "Failed to determine the emptiness of the repository. HEAD is probably in detached state"); return git_reference_resolve(&branch, head) == GIT_SUCCESS ? 0 : 1; } From 81201a4c4d1e8704260ab4572eb6c480d576fe83 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sun, 15 May 2011 06:57:34 +0200 Subject: [PATCH 26/34] Move cache.c to the new error handling --- src/cache.c | 6 ++++-- src/cache.h | 2 +- src/odb.c | 6 +++++- src/repository.c | 8 +++++++- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/cache.c b/src/cache.c index a372df160..b0a093100 100644 --- a/src/cache.c +++ b/src/cache.c @@ -32,7 +32,7 @@ #define GIT_CACHE_OPENADR 3 -void git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_ptr) +int git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_ptr) { size_t i; @@ -51,7 +51,9 @@ void git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_p cache->lru_count = 0; cache->free_obj = free_ptr; - cache->nodes = git__malloc((size + 1) * sizeof(cache_node)); //TODO: How should we deal with GIT_ENOMEM? + cache->nodes = git__malloc((size + 1) * sizeof(cache_node)); + if (cache->nodes == NULL) + return GIT_ENOMEM; for (i = 0; i < (size + 1); ++i) { git_mutex_init(&cache->nodes[i].lock); diff --git a/src/cache.h b/src/cache.h index 975aaff7e..2d9bb51eb 100644 --- a/src/cache.h +++ b/src/cache.h @@ -31,7 +31,7 @@ typedef struct { } git_cache; -void git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_ptr); +int git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_ptr); void git_cache_free(git_cache *cache); void *git_cache_try_store(git_cache *cache, void *entry); diff --git a/src/odb.c b/src/odb.c index e9e495eb1..025b591c5 100644 --- a/src/odb.c +++ b/src/odb.c @@ -234,13 +234,17 @@ static int backend_sort_cmp(const void *a, const void *b) int git_odb_new(git_odb **out) { + int error; + git_odb *db = git__calloc(1, sizeof(*db)); if (!db) return GIT_ENOMEM; git_cache_init(&db->cache, GIT_DEFAULT_CACHE_SIZE, &free_odb_object); + if (error < GIT_SUCCESS) + return error; - if (git_vector_init(&db->backends, 4, backend_sort_cmp) < 0) { + if (git_vector_init(&db->backends, 4, backend_sort_cmp) < GIT_SUCCESS) { free(db); return GIT_ENOMEM; } diff --git a/src/repository.c b/src/repository.c index 8a5934f59..1072b22a1 100644 --- a/src/repository.c +++ b/src/repository.c @@ -159,13 +159,19 @@ static int guess_repository_dirs(git_repository *repo, const char *repository_pa static git_repository *repository_alloc() { + int error; + git_repository *repo = git__malloc(sizeof(git_repository)); if (!repo) return NULL; memset(repo, 0x0, sizeof(git_repository)); - git_cache_init(&repo->objects, GIT_DEFAULT_CACHE_SIZE, &git_object__free); + error = git_cache_init(&repo->objects, GIT_DEFAULT_CACHE_SIZE, &git_object__free); + if (error < GIT_SUCCESS) { + free(repo); + return NULL; + } if (git_repository__refcache_init(&repo->references) < GIT_SUCCESS) { free(repo); From 71747bcae034d2b0230b5f38c80eb3d185a63b42 Mon Sep 17 00:00:00 2001 From: Shuhei Tanuma Date: Sun, 15 May 2011 20:07:54 +0900 Subject: [PATCH 27/34] fix git_otype typo when calling `git_odb_read_header`. --- src/odb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/odb.c b/src/odb.c index e9e495eb1..3dca97764 100644 --- a/src/odb.c +++ b/src/odb.c @@ -444,7 +444,7 @@ int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git return error; *len_p = object->raw.len; - *type_p = object->raw.len; + *type_p = object->raw.type; git_odb_object_close(object); } From 4edf3e099aafbdaea2e4e128fc3230b36af38f41 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Sun, 15 May 2011 23:45:24 +0300 Subject: [PATCH 28/34] Return success code on `git_cache_init` --- src/cache.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cache.c b/src/cache.c index b0a093100..0509afd46 100644 --- a/src/cache.c +++ b/src/cache.c @@ -60,6 +60,8 @@ int git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_pt cache->nodes[i].ptr = NULL; cache->nodes[i].lru = 0; } + + return GIT_SUCCESS; } void git_cache_free(git_cache *cache) From 7cadd1f6a788dbb997997481f09a2e430cc94958 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Sun, 15 May 2011 23:46:22 +0300 Subject: [PATCH 29/34] Check error code from `git_cache_init` --- src/odb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/odb.c b/src/odb.c index 8637534c1..f09b6d7e9 100644 --- a/src/odb.c +++ b/src/odb.c @@ -240,13 +240,13 @@ int git_odb_new(git_odb **out) if (!db) return GIT_ENOMEM; - git_cache_init(&db->cache, GIT_DEFAULT_CACHE_SIZE, &free_odb_object); + error = git_cache_init(&db->cache, GIT_DEFAULT_CACHE_SIZE, &free_odb_object); if (error < GIT_SUCCESS) return error; - if (git_vector_init(&db->backends, 4, backend_sort_cmp) < GIT_SUCCESS) { + if ((error = git_vector_init(&db->backends, 4, backend_sort_cmp)) < GIT_SUCCESS) { free(db); - return GIT_ENOMEM; + return error; } *out = db; From f87d9beb8d9e3ed2b6baedd9ca15464f7da93f25 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Sun, 15 May 2011 23:46:39 +0300 Subject: [PATCH 30/34] Change error codes from DEFINEs to an enum --- include/git2/errors.h | 122 ++++++++++++++++++++---------------------- 1 file changed, 58 insertions(+), 64 deletions(-) diff --git a/include/git2/errors.h b/include/git2/errors.h index 60af7b3d0..7e957b803 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -35,97 +35,91 @@ */ GIT_BEGIN_DECL -/** Operation completed successfully. */ -#define GIT_SUCCESS 0 +typedef enum { + GIT_SUCCESS = 0, + GIT_ERROR = -1, -/** - * Operation failed, with unspecified reason. - * This value also serves as the base error code; all other - * error codes are subtracted from it such that all errors - * are < 0, in typical POSIX C tradition. - */ -#define GIT_ERROR -1 + /** Input was not a properly formatted Git object id. */ + GIT_ENOTOID = -2, -/** Input was not a properly formatted Git object id. */ -#define GIT_ENOTOID (GIT_ERROR - 1) + /** Input does not exist in the scope searched. */ + GIT_ENOTFOUND = -3, -/** Input does not exist in the scope searched. */ -#define GIT_ENOTFOUND (GIT_ERROR - 2) + /** Not enough space available. */ + GIT_ENOMEM = -4, -/** Not enough space available. */ -#define GIT_ENOMEM (GIT_ERROR - 3) + /** Consult the OS error information. */ + GIT_EOSERR = -5, -/** Consult the OS error information. */ -#define GIT_EOSERR (GIT_ERROR - 4) + /** The specified object is of invalid type */ + GIT_EOBJTYPE = -6, -/** The specified object is of invalid type */ -#define GIT_EOBJTYPE (GIT_ERROR - 5) + /** The specified repository is invalid */ + GIT_ENOTAREPO = -7, -/** The specified object has its data corrupted */ -#define GIT_EOBJCORRUPTED (GIT_ERROR - 6) + /** The object type is invalid or doesn't match */ + GIT_EINVALIDTYPE = -8, -/** The specified repository is invalid */ -#define GIT_ENOTAREPO (GIT_ERROR - 7) + /** The object cannot be written because it's missing internal data */ + GIT_EMISSINGOBJDATA = -9, -/** The object type is invalid or doesn't match */ -#define GIT_EINVALIDTYPE (GIT_ERROR - 8) + /** The packfile for the ODB is corrupted */ + GIT_EPACKCORRUPTED = -10, -/** The object cannot be written because it's missing internal data */ -#define GIT_EMISSINGOBJDATA (GIT_ERROR - 9) + /** Failed to acquire or release a file lock */ + GIT_EFLOCKFAIL = -11, -/** The packfile for the ODB is corrupted */ -#define GIT_EPACKCORRUPTED (GIT_ERROR - 10) + /** The Z library failed to inflate/deflate an object's data */ + GIT_EZLIB = -12, -/** Failed to acquire or release a file lock */ -#define GIT_EFLOCKFAIL (GIT_ERROR - 11) + /** The queried object is currently busy */ + GIT_EBUSY = -13, -/** The Z library failed to inflate/deflate an object's data */ -#define GIT_EZLIB (GIT_ERROR - 12) + /** The index file is not backed up by an existing repository */ + GIT_EBAREINDEX = -14, -/** The queried object is currently busy */ -#define GIT_EBUSY (GIT_ERROR - 13) + /** The name of the reference is not valid */ + GIT_EINVALIDREFNAME = -15, -/** The index file is not backed up by an existing repository */ -#define GIT_EBAREINDEX (GIT_ERROR - 14) + /** The specified reference has its data corrupted */ + GIT_EREFCORRUPTED = -16, -/** The name of the reference is not valid */ -#define GIT_EINVALIDREFNAME (GIT_ERROR - 15) + /** The specified symbolic reference is too deeply nested */ + GIT_ETOONESTEDSYMREF = -17, -/** The specified reference has its data corrupted */ -#define GIT_EREFCORRUPTED (GIT_ERROR - 16) + /** The pack-refs file is either corrupted or its format is not currently supported */ + GIT_EPACKEDREFSCORRUPTED = -18, -/** The specified symbolic reference is too deeply nested */ -#define GIT_ETOONESTEDSYMREF (GIT_ERROR - 17) + /** The path is invalid */ + GIT_EINVALIDPATH = -19, -/** The pack-refs file is either corrupted or its format is not currently supported */ -#define GIT_EPACKEDREFSCORRUPTED (GIT_ERROR - 18) + /** The revision walker is empty; there are no more commits left to iterate */ + GIT_EREVWALKOVER = -20, -/** The path is invalid */ -#define GIT_EINVALIDPATH (GIT_ERROR - 19) + /** The state of the reference is not valid */ + GIT_EINVALIDREFSTATE = -21, -/** The revision walker is empty; there are no more commits left to iterate */ -#define GIT_EREVWALKOVER (GIT_ERROR - 20) + /** This feature has not been implemented yet */ + GIT_ENOTIMPLEMENTED = -22, -/** The state of the reference is not valid */ -#define GIT_EINVALIDREFSTATE (GIT_ERROR - 21) + /** A reference with this name already exists */ + GIT_EEXISTS = -23, -/** This feature has not been implemented yet */ -#define GIT_ENOTIMPLEMENTED (GIT_ERROR - 22) + /** The given integer literal is too large to be parsed */ + GIT_EOVERFLOW = -24, -/** A reference with this name already exists */ -#define GIT_EEXISTS (GIT_ERROR - 23) + /** The given literal is not a valid number */ + GIT_ENOTNUM = -25, -/** The given integer literal is too large to be parsed */ -#define GIT_EOVERFLOW (GIT_ERROR - 24) + /** Streaming error */ + GIT_ESTREAM = -26, -/** The given literal is not a valid number */ -#define GIT_ENOTNUM (GIT_ERROR - 25) + /** invalid arguments to function */ + GIT_EINVALIDARGS = -27, -/** Streaming error */ -#define GIT_ESTREAM (GIT_ERROR - 26) - -/** invalid arguments to function */ -#define GIT_EINVALIDARGS (GIT_ERROR - 27) + /** The specified object has its data corrupted */ + GIT_EOBJCORRUPTED = -28, +} git_error; /** * Return a detailed error string with the latest error From 5ca2f58057dfd5e0cf03a176782e4daa925a7f0b Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Sun, 15 May 2011 23:48:05 +0300 Subject: [PATCH 31/34] Do not set error message on `GIT_EREVWALKOVER` This is not really an error, just a special return code to mark the end of an iteration. --- src/revwalk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/revwalk.c b/src/revwalk.c index a32a16e2a..8163851a3 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -349,7 +349,7 @@ static int revwalk_next_timesort(commit_object **object_out, git_revwalk *walk) } } - return git__throw(GIT_EREVWALKOVER, "No more commits left to iterate"); + return GIT_EREVWALKOVER; } static int revwalk_next_unsorted(commit_object **object_out, git_revwalk *walk) @@ -367,7 +367,7 @@ static int revwalk_next_unsorted(commit_object **object_out, git_revwalk *walk) } } - return git__throw(GIT_EREVWALKOVER, "No more commits left to iterate"); + return GIT_EREVWALKOVER; } static int revwalk_next_toposort(commit_object **object_out, git_revwalk *walk) @@ -378,7 +378,7 @@ static int revwalk_next_toposort(commit_object **object_out, git_revwalk *walk) for (;;) { next = commit_list_pop(&walk->iterator_topo); if (next == NULL) - return git__throw(GIT_EREVWALKOVER, "No more commits left to iterate"); + return GIT_EREVWALKOVER; if (next->in_degree > 0) { next->topo_delay = 1; From cce225bfaceed90c027d44d46680a3dbe182c93b Mon Sep 17 00:00:00 2001 From: Alexei Sholik Date: Mon, 16 May 2011 01:56:20 -0700 Subject: [PATCH 32/34] Fix a typo in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b6779bd84..7cfde59f8 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ Under Unix-like systems, like Linux, *BSD and Mac OS X, libgit2 expects `pthread they should be installed by default on all systems. Under Windows, libgit2 uses the native Windows API for threading. -Additionally, he following libraries may be used as replacement for built-in functionality: +Additionally, the following libraries may be used as replacement for built-in functionality: * LibSSL **(optional)** From 3de79280e3aa79eb9bc95fa1e5a69499ba448bd9 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Tue, 17 May 2011 00:51:52 +0300 Subject: [PATCH 33/34] cache: Fix deadlock Do not try to adquire the same node lock twice when the cuckoo hashing resolves to the same node. --- src/cache.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/cache.c b/src/cache.c index 0509afd46..6e4ad4c9e 100644 --- a/src/cache.c +++ b/src/cache.c @@ -110,7 +110,7 @@ void *git_cache_try_store(git_cache *cache, void *entry) cache_node *nodes[GIT_CACHE_OPENADR], *lru_node; const uint32_t *hash; const git_oid *oid; - size_t i; + size_t i, j, node_count; oid = &((git_cached_obj*)entry)->oid; hash = (const uint32_t *)oid->id; @@ -119,16 +119,24 @@ void *git_cache_try_store(git_cache *cache, void *entry) * the cache now owns it */ git_cached_obj_incref(entry); + node_count = 0; for (i = 0; i < GIT_CACHE_OPENADR; ++i) { size_t pos = hash[i] & cache->size_mask; + cache_node *node = &cache->nodes[pos]; - nodes[i] = &cache->nodes[pos]; - git_mutex_lock(&nodes[i]->lock); + for (j = 0; j < node_count; ++j) + if (nodes[j] == node) + break; + + if (j == node_count) { + nodes[node_count++] = node; + git_mutex_lock(node); + } } lru_node = nodes[0]; - for (i = 0; i < GIT_CACHE_OPENADR; ++i) { + for (i = 0; i < node_count; ++i) { if (nodes[i]->ptr == NULL) { nodes[i]->ptr = entry; @@ -145,7 +153,7 @@ void *git_cache_try_store(git_cache *cache, void *entry) lru_node = nodes[i]; } - if (i == GIT_CACHE_OPENADR) { + if (i == node_count) { void *old_entry = lru_node->ptr; assert(old_entry); @@ -158,7 +166,7 @@ void *git_cache_try_store(git_cache *cache, void *entry) * returning it to the user */ git_cached_obj_incref(entry); - for (i = 0; i < GIT_CACHE_OPENADR; ++i) + for (i = 0; i < node_count; ++i) git_mutex_unlock(&nodes[i]->lock); return entry; From 335d6c998041f7c44dce48d3bb087c52136d970f Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Tue, 17 May 2011 01:46:07 +0300 Subject: [PATCH 34/34] cache: Drop cuckoo hashing Now we use a simple closed-addressing cache. Cuckoo hashing was creating too many issues with race conditions. Fuck that. Let's see what happens performance wise, we may have to roll back or come up with another way to implement an efficient multi-threaded cache. --- src/cache.c | 86 +++++++++++++---------------------------------------- src/cache.h | 1 - 2 files changed, 21 insertions(+), 66 deletions(-) diff --git a/src/cache.c b/src/cache.c index 6e4ad4c9e..433fc3d9c 100644 --- a/src/cache.c +++ b/src/cache.c @@ -29,9 +29,6 @@ #include "thread-utils.h" #include "cache.h" -#define GIT_CACHE_OPENADR 3 - - int git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_ptr) { size_t i; @@ -58,7 +55,6 @@ int git_cache_init(git_cache *cache, size_t size, git_cached_obj_freeptr free_pt for (i = 0; i < (size + 1); ++i) { git_mutex_init(&cache->nodes[i].lock); cache->nodes[i].ptr = NULL; - cache->nodes[i].lru = 0; } return GIT_SUCCESS; @@ -81,93 +77,53 @@ void git_cache_free(git_cache *cache) void *git_cache_get(git_cache *cache, const git_oid *oid) { const uint32_t *hash; - size_t i, pos, found = 0; cache_node *node = NULL; + void *result = NULL; hash = (const uint32_t *)oid->id; + node = &cache->nodes[hash[0] & cache->size_mask]; - for (i = 0; !found && i < GIT_CACHE_OPENADR; ++i) { - pos = hash[i] & cache->size_mask; - node = &cache->nodes[pos]; - - git_mutex_lock(&node->lock); - { - if (node->ptr && git_cached_obj_compare(node->ptr, oid) == 0) { - git_cached_obj_incref(node->ptr); - node->lru = ++cache->lru_count; - found = 1; - } + git_mutex_lock(&node->lock); + { + if (node->ptr && git_cached_obj_compare(node->ptr, oid) == 0) { + git_cached_obj_incref(node->ptr); + result = node->ptr; } - git_mutex_unlock(&node->lock); } + git_mutex_unlock(&node->lock); - - return found ? node->ptr : NULL; + return result; } void *git_cache_try_store(git_cache *cache, void *entry) { - cache_node *nodes[GIT_CACHE_OPENADR], *lru_node; const uint32_t *hash; const git_oid *oid; - size_t i, j, node_count; + cache_node *node = NULL; oid = &((git_cached_obj*)entry)->oid; hash = (const uint32_t *)oid->id; + node = &cache->nodes[hash[0] & cache->size_mask]; /* increase the refcount on this object, because * the cache now owns it */ git_cached_obj_incref(entry); + git_mutex_lock(&node->lock); - node_count = 0; - for (i = 0; i < GIT_CACHE_OPENADR; ++i) { - size_t pos = hash[i] & cache->size_mask; - cache_node *node = &cache->nodes[pos]; - - for (j = 0; j < node_count; ++j) - if (nodes[j] == node) - break; - - if (j == node_count) { - nodes[node_count++] = node; - git_mutex_lock(node); - } - } - - lru_node = nodes[0]; - - for (i = 0; i < node_count; ++i) { - - if (nodes[i]->ptr == NULL) { - nodes[i]->ptr = entry; - nodes[i]->lru = ++cache->lru_count; - break; - } else if (git_cached_obj_compare(nodes[i]->ptr, oid) == 0) { - git_cached_obj_decref(entry, cache->free_obj); - entry = nodes[i]->ptr; - nodes[i]->lru = ++cache->lru_count; - break; - } - - if (nodes[i]->lru < lru_node->lru) - lru_node = nodes[i]; - } - - if (i == node_count) { - void *old_entry = lru_node->ptr; - assert(old_entry); - - git_cached_obj_decref(old_entry, cache->free_obj); - lru_node->ptr = entry; - lru_node->lru = ++cache->lru_count; + if (node->ptr == NULL) { + node->ptr = entry; + } else if (git_cached_obj_compare(node->ptr, oid) == 0) { + git_cached_obj_decref(entry, cache->free_obj); + entry = node->ptr; + } else { + git_cached_obj_decref(node->ptr, cache->free_obj); + node->ptr = entry; } /* increase the refcount again, because we are * returning it to the user */ git_cached_obj_incref(entry); - - for (i = 0; i < node_count; ++i) - git_mutex_unlock(&nodes[i]->lock); + git_mutex_unlock(&node->lock); return entry; } diff --git a/src/cache.h b/src/cache.h index 2d9bb51eb..4794dea3a 100644 --- a/src/cache.h +++ b/src/cache.h @@ -19,7 +19,6 @@ typedef struct { typedef struct { git_cached_obj *ptr; git_mutex lock; - unsigned int lru; } cache_node; typedef struct {