diff options
author | Russell Belfer <arrbee@arrbee.com> | 2012-03-14 01:23:24 +0400 |
---|---|---|
committer | Russell Belfer <arrbee@arrbee.com> | 2012-03-14 01:23:24 +0400 |
commit | e3c475107045cb89c53c114716bafebc7538433f (patch) | |
tree | eb6df56dc41633a30a935058477e45b8e39ef77a | |
parent | 1736799d2a15d912cfc46b7089c2bff02a1cbd0e (diff) |
Resolve comments from pull request
This converts the map validation function into a macro, tweaks
the GITERR_OS system error automatic appending, and adds a
tentative new error access API and some quick unit tests for
both the old and new error APIs.
-rw-r--r-- | include/git2/errors.h | 16 | ||||
-rw-r--r-- | src/errors.c | 33 | ||||
-rw-r--r-- | src/map.c | 36 | ||||
-rw-r--r-- | src/map.h | 5 | ||||
-rw-r--r-- | src/unix/map.c | 3 | ||||
-rw-r--r-- | src/win32/map.c | 3 | ||||
-rw-r--r-- | tests-clar/core/errors.c | 74 |
7 files changed, 122 insertions, 48 deletions
diff --git a/include/git2/errors.h b/include/git2/errors.h index cd9dc08e7..5a4e540e1 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -134,6 +134,7 @@ typedef enum { /** * Return a detailed error string with the latest error * that occurred in the library. + * @deprecated This will be replaced in the new error handling * @return a string explaining the error */ GIT_EXTERN(const char *) git_lasterror(void); @@ -145,6 +146,7 @@ GIT_EXTERN(const char *) git_lasterror(void); * NOTE: This method will be eventually deprecated in favor * of the new `git_lasterror`. * + * @deprecated This will be replaced in the new error handling * @param num The error code to explain * @return a string explaining the error code */ @@ -152,9 +154,23 @@ GIT_EXTERN(const char *) git_strerror(int num); /** * Clear the latest library error + * @deprecated This will be replaced in the new error handling */ GIT_EXTERN(void) git_clearerror(void); +/** + * Return the last `git_error` object that was generated for the + * current thread or NULL if no error has occurred. + * + * @return A git_error object. + */ +GIT_EXTERN(const git_error *) git_error_last(void); + +/** + * Clear the last library error that occurred for this thread. + */ +GIT_EXTERN(void) git_error_clear(void); + /** @} */ GIT_END_DECL #endif diff --git a/src/errors.c b/src/errors.c index 19bc7b77b..70aa641c4 100644 --- a/src/errors.c +++ b/src/errors.c @@ -123,33 +123,41 @@ void giterr_set(int error_class, const char *string, ...) char error_str[1024]; va_list arglist; + /* Grab errno before calling vsnprintf() so it won't be overwritten */ + const char *os_error_msg = + (error_class == GITERR_OS && errno != 0) ? strerror(errno) : NULL; +#ifdef GIT_WIN32 + DWORD dwLastError = GetLastError(); +#endif + va_start(arglist, string); p_vsnprintf(error_str, sizeof(error_str), string, arglist); va_end(arglist); /* automatically suffix strerror(errno) for GITERR_OS errors */ if (error_class == GITERR_OS) { - if (errno != 0) { + if (os_error_msg != NULL) { strncat(error_str, ": ", sizeof(error_str)); - strncat(error_str, strerror(errno), sizeof(error_str)); - errno = 0; + strncat(error_str, os_error_msg, sizeof(error_str)); + errno = 0; /* reset so same error won't be reported twice */ } #ifdef GIT_WIN32 - else { - LPVOID lpMsgBuf; - DWORD dw = GetLastError(); + else if (dwLastError != 0) { + LPVOID lpMsgBuf = NULL; FormatMessage( FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, - NULL, dw, 0, (LPTSTR) &lpMsgBuf, 0, NULL); + NULL, dwLastError, 0, (LPTSTR) &lpMsgBuf, 0, NULL); if (lpMsgBuf) { strncat(error_str, ": ", sizeof(error_str)); strncat(error_str, (const char *)lpMsgBuf, sizeof(error_str)); LocalFree(lpMsgBuf); } + + SetLastError(0); } #endif } @@ -185,3 +193,14 @@ void giterr_clear(void) { GIT_GLOBAL->last_error = NULL; } + +const git_error *git_error_last(void) +{ + return GIT_GLOBAL->last_error; +} + +void git_error_clear(void) +{ + giterr_clear(); +} + diff --git a/src/map.c b/src/map.c deleted file mode 100644 index 56a37f3f6..000000000 --- a/src/map.c +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright (C) 2009-2012 the libgit2 contributors - * - * This file is part of libgit2, distributed under the GNU GPL v2 with - * a Linking Exception. For full terms see the included COPYING file. - */ -#include <git2/common.h> -#include "map.h" - -int validate_map_args( - git_map *out, size_t len, int prot, int flags, int fd, git_off_t offset) -{ - GIT_UNUSED(fd); - GIT_UNUSED(offset); - - if (out == NULL || len == 0) { - errno = EINVAL; - giterr_set(GITERR_OS, "Failed to mmap. No map or zero length"); - return -1; - } - - if (!(prot & GIT_PROT_WRITE) && !(prot & GIT_PROT_READ)) { - errno = EINVAL; - giterr_set(GITERR_OS, "Failed to mmap. Invalid protection parameters"); - return -1; - } - - if (flags & GIT_MAP_FIXED) { - errno = EINVAL; - giterr_set(GITERR_OS, "Failed to mmap. FIXED not set"); - return -1; - } - - return 0; -} - @@ -31,7 +31,10 @@ typedef struct { /* memory mapped buffer */ #endif } git_map; -extern int validate_map_args(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offset); +#define GIT_MMAP_VALIDATE(out, len, prot, flags) do { \ + assert(out != NULL && len > 0); \ + assert((prot & GIT_PROT_WRITE) || (prot & GIT_PROT_READ)); \ + assert((flags & GIT_MAP_FIXED) == 0); } while (0) extern int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offset); extern int p_munmap(git_map *map); diff --git a/src/unix/map.c b/src/unix/map.c index 1e2389ec2..772f4e247 100644 --- a/src/unix/map.c +++ b/src/unix/map.c @@ -17,8 +17,7 @@ int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offs int mprot = 0; int mflag = 0; - if (validate_map_args(out, len, prot, flags, fd, offset) < 0) - return -1; + GIT_MMAP_VALIDATE(out, len, prot, flags); out->data = NULL; out->len = 0; diff --git a/src/win32/map.c b/src/win32/map.c index de996e0d1..f730120cc 100644 --- a/src/win32/map.c +++ b/src/win32/map.c @@ -33,8 +33,7 @@ int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offs git_off_t page_start; git_off_t page_offset; - if (validate_map_args(out, len, prot, flags, fd, offset) < 0) - return -1; + GIT_MMAP_VALIDATE(out, len, prot, flags); out->data = NULL; out->len = 0; diff --git a/tests-clar/core/errors.c b/tests-clar/core/errors.c new file mode 100644 index 000000000..52b2652c8 --- /dev/null +++ b/tests-clar/core/errors.c @@ -0,0 +1,74 @@ +#include "clar_libgit2.h" +#include "common.h" +#include "util.h" +#include "posix.h" + +#ifdef git__throw +void test_core_errors__old_school(void) +{ + git_clearerror(); + cl_assert(git_lasterror() == NULL); + + cl_assert(git_strerror(GIT_ENOTFOUND) != NULL); + + git__throw(GIT_ENOTFOUND, "My Message"); + cl_assert(git_lasterror() != NULL); + cl_assert(git__prefixcmp(git_lasterror(), "My Message") == 0); + git_clearerror(); +} +#endif + +#ifdef GITERR_CHECK_ALLOC +void test_core_errors__new_school(void) +{ + char *str_in_error; + + git_error_clear(); + cl_assert(git_error_last() == NULL); + + giterr_set_oom(); /* internal fn */ + + cl_assert(git_error_last() != NULL); + cl_assert(git_error_last()->klass == GITERR_NOMEMORY); + str_in_error = strstr(git_error_last()->message, "memory"); + cl_assert(str_in_error != NULL); + + git_error_clear(); + + giterr_set(GITERR_REPOSITORY, "This is a test"); /* internal fn */ + + cl_assert(git_error_last() != NULL); + str_in_error = strstr(git_error_last()->message, "This is a test"); + cl_assert(str_in_error != NULL); + + git_error_clear(); + + { + struct stat st; + assert(p_lstat("this_file_does_not_exist", &st) < 0); + } + giterr_set(GITERR_OS, "stat failed"); /* internal fn */ + + cl_assert(git_error_last() != NULL); + str_in_error = strstr(git_error_last()->message, "stat failed"); + cl_assert(str_in_error != NULL); + cl_assert(git__prefixcmp(str_in_error, "stat failed: ") == 0); + cl_assert(strlen(str_in_error) > strlen("stat failed: ")); + +#ifdef GIT_WIN32 + git_error_clear(); + + /* The MSDN docs use this to generate a sample error */ + cl_assert(GetProcessId(NULL) == 0); + giterr_set(GITERR_OS, "GetProcessId failed"); /* internal fn */ + + cl_assert(git_error_last() != NULL); + str_in_error = strstr(git_error_last()->message, "GetProcessId failed"); + cl_assert(str_in_error != NULL); + cl_assert(git__prefixcmp(str_in_error, "GetProcessId failed: ") == 0); + cl_assert(strlen(str_in_error) > strlen("GetProcessId failed: ")); +#endif + + git_error_clear(); +} +#endif |