From 73a5faf0176548640784090190bb2448cf1708aa Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Apr 2019 09:48:14 -0400 Subject: test-prio-queue: use xmalloc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test-prio-queue.c doesn't check the return value of malloc, and could segfault. It's unlikely for this to matter in practice; it's a small allocation, and this code isn't even installed alongside the rest of Git. But let's use xmalloc(), which makes auditing for other accidental uses of bare malloc() easier. Reported-by: 王健强 Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/t/helper/test-prio-queue.c b/t/helper/test-prio-queue.c index 5bc9c46..f402844 100644 --- a/t/helper/test-prio-queue.c +++ b/t/helper/test-prio-queue.c @@ -40,7 +40,7 @@ int cmd__prio_queue(int argc, const char **argv) } else if (!strcmp(*argv, "stack")) { pq.compare = NULL; } else { - int *v = malloc(sizeof(*v)); + int *v = xmalloc(sizeof(*v)); *v = atoi(*argv); prio_queue_put(&pq, v); } -- cgit v0.10.2-6-g49f6 From b46054b3746271d23feab001b49357983fda42f1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Apr 2019 09:48:33 -0400 Subject: xdiff: use git-compat-util Since the xdiff library was not originally part of Git, it does its own system includes. Let's instead use git-compat-util, which has two benefits: 1. It adjusts for any system-specific quirks in how or what we should include (though xdiff's needs are light enough that this hasn't been a problem in the past). 2. It lets us use wrapper functions like xmalloc(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/xdiff/xinclude.h b/xdiff/xinclude.h index f35c448..a4285ac 100644 --- a/xdiff/xinclude.h +++ b/xdiff/xinclude.h @@ -23,13 +23,7 @@ #if !defined(XINCLUDE_H) #define XINCLUDE_H -#include -#include -#include -#include -#include -#include - +#include "git-compat-util.h" #include "xmacros.h" #include "xdiff.h" #include "xtypes.h" -- cgit v0.10.2-6-g49f6 From 36c831972497c1141a704fb5ad179675111ee321 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Apr 2019 09:49:25 -0400 Subject: xdiff: use xmalloc/xrealloc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most of xdiff uses a bare malloc() to allocate memory, and returns an error when we get NULL. However, there are a few spots which don't check the return value and may segfault, including at least xdl_merge() and xpatience.c's find_longest_common_sequence(). Let's use xmalloc() everywhere instead, so that we get a graceful die() for these cases, without having to do further auditing. This does mean the existing cases which check errors will now die() instead of returning an error up the stack. But: - that's how the rest of Git behaves already for malloc errors - all of the callers of xdi_diff(), etc, die upon seeing an error So while we might one day want to fully lib-ify the diff code and make it possible to use as part of a long-running process, we're not close to that now. And because we're just tweaking the xdl_malloc() macro here, we're not really moving ourselves any further away from that. We could, for example, simplify some of the functions which handle malloc() errors which can no longer occur. But that would probably be taking us in the wrong direction. This also makes our malloc handling more consistent with the rest of Git, including enforcing GIT_ALLOC_LIMIT and trying to reclaim pack memory when needed. Reported-by: 王健强 Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index b158369..032e3a9 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -113,9 +113,9 @@ typedef struct s_bdiffparam { } bdiffparam_t; -#define xdl_malloc(x) malloc(x) +#define xdl_malloc(x) xmalloc(x) #define xdl_free(ptr) free(ptr) -#define xdl_realloc(ptr,x) realloc(ptr,x) +#define xdl_realloc(ptr,x) xrealloc(ptr,x) void *xdl_mmfile_first(mmfile_t *mmf, long *size); long xdl_mmfile_size(mmfile_t *mmf); -- cgit v0.10.2-6-g49f6 From 999b951b285233d96904b1aad5e0dea22bed55c7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Apr 2019 09:49:57 -0400 Subject: progress: use xmalloc/xcalloc Since the early days of Git, the progress code allocates its struct with a bare malloc(), not xmalloc(). If the allocation fails, we just avoid showing progress at all. While perhaps a noble goal not to fail the whole operation because of optional progress, in practice: 1. Any failure to allocate a few dozen bytes here means critical path allocations are likely to fail, too. 2. These days we use a strbuf for throughput progress (and there's a patch under discussion to do the same for non-throughput cases, too). And that uses xmalloc() under the hood, which means we'd still die on some allocation failures. Let's switch to xmalloc(). That makes us consistent with the rest of Git and makes it easier to audit for other (less careful) bare mallocs. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/progress.c b/progress.c index 5a99c9f..699ac33 100644 --- a/progress.c +++ b/progress.c @@ -139,12 +139,10 @@ void display_throughput(struct progress *progress, uint64_t total) now_ns = getnanotime(); if (!tp) { - progress->throughput = tp = calloc(1, sizeof(*tp)); - if (tp) { - tp->prev_total = tp->curr_total = total; - tp->prev_ns = now_ns; - strbuf_init(&tp->display, 0); - } + progress->throughput = tp = xcalloc(1, sizeof(*tp)); + tp->prev_total = tp->curr_total = total; + tp->prev_ns = now_ns; + strbuf_init(&tp->display, 0); return; } tp->curr_total = total; @@ -196,13 +194,7 @@ int display_progress(struct progress *progress, uint64_t n) static struct progress *start_progress_delay(const char *title, uint64_t total, unsigned delay) { - struct progress *progress = malloc(sizeof(*progress)); - if (!progress) { - /* unlikely, but here's a good fallback */ - fprintf(stderr, "%s...\n", title); - fflush(stderr); - return NULL; - } + struct progress *progress = xmalloc(sizeof(*progress)); progress->title = title; progress->total = total; progress->last_value = -1; -- cgit v0.10.2-6-g49f6