From f85fc367e01eb0f5918263b1fcdfc8b7d16065c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 26 Jul 2015 21:12:00 +0200 Subject: error: store the error messages in a reusable buffer Instead of allocating a brand new buffer for each error string we want to store, we can use a per-thread buffer to store the error string and re-use the underlying storage. We already use the buffer to format the string, so this mostly makes that more direct. --- src/errors.c | 50 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 17 deletions(-) (limited to 'src/errors.c') diff --git a/src/errors.c b/src/errors.c index 7a2600586..56fad009f 100644 --- a/src/errors.c +++ b/src/errors.c @@ -18,19 +18,30 @@ static git_error g_git_oom_error = { GITERR_NOMEMORY }; -static void set_error(int error_class, char *string) +static void set_error_from_buffer(int error_class) { git_error *error = &GIT_GLOBAL->error_t; + git_buf *buf = &GIT_GLOBAL->error_buf; - if (error->message != string) - git__free(error->message); - - error->message = string; + error->message = buf->ptr; error->klass = error_class; GIT_GLOBAL->last_error = error; } +static void set_error(int error_class, char *string) +{ + git_buf *buf = &GIT_GLOBAL->error_buf; + + git_buf_clear(buf); + if (string) { + git_buf_puts(buf, string); + git__free(string); + } + + set_error_from_buffer(error_class); +} + void giterr_set_oom(void) { GIT_GLOBAL->last_error = &g_git_oom_error; @@ -38,27 +49,28 @@ void giterr_set_oom(void) void giterr_set(int error_class, const char *string, ...) { - git_buf buf = GIT_BUF_INIT; va_list arglist; #ifdef GIT_WIN32 DWORD win32_error_code = (error_class == GITERR_OS) ? GetLastError() : 0; #endif int error_code = (error_class == GITERR_OS) ? errno : 0; + git_buf *buf = &GIT_GLOBAL->error_buf; + git_buf_clear(buf); if (string) { va_start(arglist, string); - git_buf_vprintf(&buf, string, arglist); + git_buf_vprintf(buf, string, arglist); va_end(arglist); if (error_class == GITERR_OS) - git_buf_PUTS(&buf, ": "); + git_buf_PUTS(buf, ": "); } if (error_class == GITERR_OS) { #ifdef GIT_WIN32 char * win32_error = git_win32_get_error_message(win32_error_code); if (win32_error) { - git_buf_puts(&buf, win32_error); + git_buf_puts(buf, win32_error); git__free(win32_error); SetLastError(0); @@ -66,26 +78,29 @@ void giterr_set(int error_class, const char *string, ...) else #endif if (error_code) - git_buf_puts(&buf, strerror(error_code)); + git_buf_puts(buf, strerror(error_code)); if (error_code) errno = 0; } - if (!git_buf_oom(&buf)) - set_error(error_class, git_buf_detach(&buf)); + if (!git_buf_oom(buf)) + set_error_from_buffer(error_class); } void giterr_set_str(int error_class, const char *string) { - char *message; + git_buf *buf = &GIT_GLOBAL->error_buf; assert(string); - message = git__strdup(string); + if (!string) + return; - if (message) - set_error(error_class, message); + git_buf_clear(buf); + git_buf_puts(buf, string); + if (!git_buf_oom(buf)) + set_error_from_buffer(error_class); } int giterr_set_regex(const regex_t *regex, int error_code) @@ -119,13 +134,14 @@ void giterr_clear(void) int giterr_detach(git_error *cpy) { git_error *error = GIT_GLOBAL->last_error; + git_buf *buf = &GIT_GLOBAL->error_buf; assert(cpy); if (!error) return -1; - cpy->message = error->message; + cpy->message = git_buf_detach(buf); cpy->klass = error->klass; error->message = NULL; -- cgit v1.2.1 From c2f17bda074b2e5718456aed75fedd2196c8dc94 Mon Sep 17 00:00:00 2001 From: Michael Procter Date: Thu, 23 Jul 2015 13:17:08 +0100 Subject: Ensure static oom error message not detached Error messages that are detached are assumed to be dynamically allocated. Passing a pointer to the static oom error message can cause an attempt to free the static buffer later. This change checks if the oom error message is about to be detached and detaches a copy instead. --- src/errors.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src/errors.c') diff --git a/src/errors.c b/src/errors.c index 7a2600586..979602a2a 100644 --- a/src/errors.c +++ b/src/errors.c @@ -125,10 +125,14 @@ int giterr_detach(git_error *cpy) if (!error) return -1; - cpy->message = error->message; + if (error == &g_git_oom_error) { + cpy->message = git__strdup(error->message); + } else { + cpy->message = error->message; + error->message = NULL; + } cpy->klass = error->klass; - error->message = NULL; giterr_clear(); return 0; -- cgit v1.2.1 From 25dbcf34993cad3cdc3981f1ed394d3374fb640f Mon Sep 17 00:00:00 2001 From: Michael Procter Date: Mon, 27 Jul 2015 09:59:07 +0100 Subject: Make giterr_detach no longer public --- src/errors.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/errors.c') diff --git a/src/errors.c b/src/errors.c index 979602a2a..95c62176c 100644 --- a/src/errors.c +++ b/src/errors.c @@ -116,7 +116,7 @@ void giterr_clear(void) #endif } -int giterr_detach(git_error *cpy) +static int giterr_detach(git_error *cpy) { git_error *error = GIT_GLOBAL->last_error; -- cgit v1.2.1 From 0fcfb60dc4f5e6cfd91c902d844f5d8665a5c1a7 Mon Sep 17 00:00:00 2001 From: Michael Procter Date: Mon, 27 Jul 2015 10:10:18 +0100 Subject: Make giterr_restore aware of g_git_oom_error Allow restoring a previously captured oom error, by detecting when the captured message pointer points to the static oom error message. This means there is no need to strdup the message in giterr_detach. --- src/errors.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'src/errors.c') diff --git a/src/errors.c b/src/errors.c index 95c62176c..f10430c10 100644 --- a/src/errors.c +++ b/src/errors.c @@ -125,14 +125,12 @@ static int giterr_detach(git_error *cpy) if (!error) return -1; - if (error == &g_git_oom_error) { - cpy->message = git__strdup(error->message); - } else { - cpy->message = error->message; - error->message = NULL; - } + cpy->message = error->message; cpy->klass = error->klass; + if (error != &g_git_oom_error) { + error->message = NULL; + } giterr_clear(); return 0; @@ -153,8 +151,13 @@ int giterr_capture(git_error_state *state, int error_code) int giterr_restore(git_error_state *state) { - if (state && state->error_code && state->error_msg.message) - set_error(state->error_msg.klass, state->error_msg.message); + if (state && state->error_code && state->error_msg.message) { + if (state->error_msg.message == g_git_oom_error.message) { + giterr_set_oom(); + } else { + set_error(state->error_msg.klass, state->error_msg.message); + } + } else giterr_clear(); -- cgit v1.2.1 From ef4857c2b3d4a61fd1d840199afc92eaf2e15345 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 3 Aug 2015 16:50:27 -0500 Subject: errors: tighten up git_error_state OOMs a bit more When an error state is an OOM, make sure that we treat is specially and do not try to free it. --- src/errors.c | 72 +++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 30 deletions(-) (limited to 'src/errors.c') diff --git a/src/errors.c b/src/errors.c index 973326c00..91acc3541 100644 --- a/src/errors.c +++ b/src/errors.c @@ -131,53 +131,65 @@ void giterr_clear(void) #endif } -static int giterr_detach(git_error *cpy) +const git_error *giterr_last(void) +{ + return GIT_GLOBAL->last_error; +} + +int giterr_state_capture(git_error_state *state, int error_code) { git_error *error = GIT_GLOBAL->last_error; - git_buf *buf = &GIT_GLOBAL->error_buf; + git_buf *error_buf = &GIT_GLOBAL->error_buf; - assert(cpy); + memset(state, 0, sizeof(git_error_state)); - if (!error) - return -1; + if (!error_code) + return 0; + + state->error_code = error_code; + state->oom = (error == &g_git_oom_error); - cpy->message = git_buf_detach(buf); - cpy->klass = error->klass; + if (error) { + state->error_msg.klass = error->klass; - if (error != &g_git_oom_error) { - error->message = NULL; + if (state->oom) + state->error_msg.message = g_git_oom_error.message; + else + state->error_msg.message = git_buf_detach(error_buf); } - giterr_clear(); - return 0; + giterr_clear(); + return error_code; } -const git_error *giterr_last(void) +int giterr_state_restore(git_error_state *state) { - return GIT_GLOBAL->last_error; -} + int ret = 0; -int giterr_capture(git_error_state *state, int error_code) -{ - state->error_code = error_code; - if (error_code) - giterr_detach(&state->error_msg); - return error_code; -} + giterr_clear(); -int giterr_restore(git_error_state *state) -{ - if (state && state->error_code && state->error_msg.message) { - if (state->error_msg.message == g_git_oom_error.message) { + if (state && state->error_msg.message) { + if (state->oom) giterr_set_oom(); - } else { + else set_error(state->error_msg.klass, state->error_msg.message); - } + + ret = state->error_code; + memset(state, 0, sizeof(git_error_state)); } - else - giterr_clear(); - return state ? state->error_code : 0; + return ret; +} + +void giterr_state_free(git_error_state *state) +{ + if (!state) + return; + + if (!state->oom) + git__free(state->error_msg.message); + + memset(state, 0, sizeof(git_error_state)); } int giterr_system_last(void) -- cgit v1.2.1