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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278
diff --git a/include/git2/errors.h b/include/git2/errors.h
index cd9dc08..5a4e540 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 19bc7b7..70aa641 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 56a37f3..0000000
--- 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;
-}
-
diff --git a/src/map.h b/src/map.h
index d0ca1ee..96d8795 100644
--- a/src/map.h
+++ b/src/map.h
@@ -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 1e2389e..772f4e2 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 de996e0..f730120 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 0000000..52b2652
--- /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