From abf04bdaa845a6650d1f44b9f5914df442559d75 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 8 Jul 2022 16:25:16 +0000 Subject: [PATCH 1/4] xdiff: introduce XDL_ALLOC_ARRAY() Add a helper to allocate an array that automatically calculates the allocation size. This is analogous to ALLOC_ARRAY() in the rest of the codebase but returns NULL if the allocation fails to accommodate other users of libxdiff such as libgit2. The helper will also return NULL if the multiplication in the allocation calculation overflows. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 2 +- xdiff/xmacros.h | 5 +++++ xdiff/xpatience.c | 4 ++-- xdiff/xprepare.c | 8 ++++---- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index 758410c11a..53e803e6bc 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -337,7 +337,7 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, * One is to store the forward path and one to store the backward path. */ ndiags = xe->xdf1.nreff + xe->xdf2.nreff + 3; - if (!(kvd = (long *) xdl_malloc((2 * ndiags + 2) * sizeof(long)))) { + if (!XDL_ALLOC_ARRAY(kvd, 2 * ndiags + 2)) { xdl_free_env(xe); return -1; diff --git a/xdiff/xmacros.h b/xdiff/xmacros.h index ae4636c247..9fd3c5da91 100644 --- a/xdiff/xmacros.h +++ b/xdiff/xmacros.h @@ -49,5 +49,10 @@ do { \ ((unsigned long) __p[2]) << 16 | ((unsigned long) __p[3]) << 24; \ } while (0) +/* Allocate an array of nr elements, returns NULL on failure */ +#define XDL_ALLOC_ARRAY(p, nr) \ + ((p) = SIZE_MAX / sizeof(*(p)) >= (size_t)(nr) \ + ? xdl_malloc((nr) * sizeof(*(p))) \ + : NULL) #endif /* #if !defined(XMACROS_H) */ diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index 1a21c6a74b..ce87b9084c 100644 --- a/xdiff/xpatience.c +++ b/xdiff/xpatience.c @@ -200,7 +200,7 @@ static int binary_search(struct entry **sequence, int longest, */ static int find_longest_common_sequence(struct hashmap *map, struct entry **res) { - struct entry **sequence = xdl_malloc(map->nr * sizeof(struct entry *)); + struct entry **sequence; int longest = 0, i; struct entry *entry; @@ -211,7 +211,7 @@ static int find_longest_common_sequence(struct hashmap *map, struct entry **res) */ int anchor_i = -1; - if (!sequence) + if (!XDL_ALLOC_ARRAY(sequence, map->nr)) return -1; for (entry = map->first; entry; entry = entry->next) { diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 105752758f..25866a1667 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -86,7 +86,7 @@ static int xdl_init_classifier(xdlclassifier_t *cf, long size, long flags) { memset(cf->rchash, 0, cf->hsize * sizeof(xdlclass_t *)); cf->alloc = size; - if (!(cf->rcrecs = (xdlclass_t **) xdl_malloc(cf->alloc * sizeof(xdlclass_t *)))) { + if (!XDL_ALLOC_ARRAY(cf->rcrecs, cf->alloc)) { xdl_free(cf->rchash); xdl_cha_free(&cf->ncha); @@ -178,7 +178,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0) goto abort; - if (!(recs = (xrecord_t **) xdl_malloc(narec * sizeof(xrecord_t *)))) + if (!XDL_ALLOC_ARRAY(recs, narec)) goto abort; hbits = xdl_hashbits((unsigned int) narec); @@ -215,9 +215,9 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) { - if (!(rindex = xdl_malloc((nrec + 1) * sizeof(*rindex)))) + if (!XDL_ALLOC_ARRAY(rindex, nrec + 1)) goto abort; - if (!(ha = xdl_malloc((nrec + 1) * sizeof(*ha)))) + if (!XDL_ALLOC_ARRAY(ha, nrec + 1)) goto abort; } From 18aae7e21e1a88c75adb38aa2555e0b1928b137d Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 8 Jul 2022 16:25:17 +0000 Subject: [PATCH 2/4] xdiff: introduce xdl_calloc In preparation for introducing XDL_CALLOC_ARRAY() use calloc() to obtain zeroed out memory rather than malloc() followed by memset(). To try and keep the lines a reasonable length this commit also stops casting the pointer returned by calloc() as this is unnecessary. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- xdiff/xdiff.h | 1 + xdiff/xhistogram.c | 22 +++++++++------------- xdiff/xpatience.c | 4 +--- xdiff/xprepare.c | 12 ++++-------- 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index 72e25a9ffa..bb56b23f34 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -120,6 +120,7 @@ typedef struct s_bdiffparam { #define xdl_malloc(x) xmalloc(x) +#define xdl_calloc(n, sz) xcalloc(n, sz) #define xdl_free(ptr) free(ptr) #define xdl_realloc(ptr,x) xrealloc(ptr,x) diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c index 01decffc33..c97edc1e29 100644 --- a/xdiff/xhistogram.c +++ b/xdiff/xhistogram.c @@ -251,7 +251,7 @@ static int find_lcs(xpparam_t const *xpp, xdfenv_t *env, int line1, int count1, int line2, int count2) { int b_ptr; - int sz, ret = -1; + int ret = -1; struct histindex index; memset(&index, 0, sizeof(index)); @@ -265,23 +265,19 @@ static int find_lcs(xpparam_t const *xpp, xdfenv_t *env, index.rcha.head = NULL; index.table_bits = xdl_hashbits(count1); - sz = index.records_size = 1 << index.table_bits; - sz *= sizeof(struct record *); - if (!(index.records = (struct record **) xdl_malloc(sz))) + index.records_size = 1 << index.table_bits; + if (!(index.records = xdl_calloc(index.records_size, + sizeof(*index.records)))) goto cleanup; - memset(index.records, 0, sz); - sz = index.line_map_size = count1; - sz *= sizeof(struct record *); - if (!(index.line_map = (struct record **) xdl_malloc(sz))) + index.line_map_size = count1; + if (!(index.line_map = xdl_calloc(index.line_map_size, + sizeof(*index.line_map)))) goto cleanup; - memset(index.line_map, 0, sz); - sz = index.line_map_size; - sz *= sizeof(unsigned int); - if (!(index.next_ptrs = (unsigned int *) xdl_malloc(sz))) + if (!(index.next_ptrs = xdl_calloc(index.line_map_size, + sizeof(*index.next_ptrs)))) goto cleanup; - memset(index.next_ptrs, 0, sz); /* lines / 4 + 1 comes from xprepare.c:xdl_prepare_ctx() */ if (xdl_cha_init(&index.rcha, sizeof(struct record), count1 / 4 + 1) < 0) diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index ce87b9084c..94f8886dd1 100644 --- a/xdiff/xpatience.c +++ b/xdiff/xpatience.c @@ -151,11 +151,9 @@ static int fill_hashmap(mmfile_t *file1, mmfile_t *file2, /* We know exactly how large we want the hash map */ result->alloc = count1 * 2; - result->entries = (struct entry *) - xdl_malloc(result->alloc * sizeof(struct entry)); + result->entries = xdl_calloc(result->alloc, sizeof(*result->entries)); if (!result->entries) return -1; - memset(result->entries, 0, result->alloc * sizeof(struct entry)); /* First, fill with entries from the first file */ while (count1--) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 25866a1667..45190e59fc 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -78,12 +78,11 @@ static int xdl_init_classifier(xdlclassifier_t *cf, long size, long flags) { return -1; } - if (!(cf->rchash = (xdlclass_t **) xdl_malloc(cf->hsize * sizeof(xdlclass_t *)))) { + if (!(cf->rchash = xdl_calloc(cf->hsize, sizeof(*cf->rchash)))) { xdl_cha_free(&cf->ncha); return -1; } - memset(cf->rchash, 0, cf->hsize * sizeof(xdlclass_t *)); cf->alloc = size; if (!XDL_ALLOC_ARRAY(cf->rcrecs, cf->alloc)) { @@ -183,9 +182,8 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ hbits = xdl_hashbits((unsigned int) narec); hsize = 1 << hbits; - if (!(rhash = (xrecord_t **) xdl_malloc(hsize * sizeof(xrecord_t *)))) + if (!(rhash = xdl_calloc(hsize, sizeof(*rhash)))) goto abort; - memset(rhash, 0, hsize * sizeof(xrecord_t *)); nrec = 0; if ((cur = blk = xdl_mmfile_first(mf, &bsize))) { @@ -209,9 +207,8 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ } } - if (!(rchg = (char *) xdl_malloc((nrec + 2) * sizeof(char)))) + if (!(rchg = xdl_calloc((nrec + 2), sizeof(*rchg)))) goto abort; - memset(rchg, 0, (nrec + 2) * sizeof(char)); if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) { @@ -383,11 +380,10 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd xdlclass_t *rcrec; char *dis, *dis1, *dis2; - if (!(dis = (char *) xdl_malloc(xdf1->nrec + xdf2->nrec + 2))) { + if (!(dis = xdl_calloc(xdf1->nrec + xdf2->nrec + 2, sizeof(*dis)))) { return -1; } - memset(dis, 0, xdf1->nrec + xdf2->nrec + 2); dis1 = dis; dis2 = dis1 + xdf1->nrec + 1; From 848fd5ae5b6506df50ae872302d94b794dc57d51 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 8 Jul 2022 16:25:18 +0000 Subject: [PATCH 3/4] xdiff: introduce XDL_CALLOC_ARRAY() Add a helper for allocating an array and initialize the elements to zero. This is analogous to CALLOC_ARRAY() in the rest of the codebase but it returns NULL on allocation failures rather than dying to accommodate other users of libxdiff such as libgit2. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- xdiff/xhistogram.c | 9 +++------ xdiff/xmacros.h | 3 +++ xdiff/xpatience.c | 3 +-- xdiff/xprepare.c | 10 ++++------ 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c index c97edc1e29..df909004c1 100644 --- a/xdiff/xhistogram.c +++ b/xdiff/xhistogram.c @@ -266,17 +266,14 @@ static int find_lcs(xpparam_t const *xpp, xdfenv_t *env, index.table_bits = xdl_hashbits(count1); index.records_size = 1 << index.table_bits; - if (!(index.records = xdl_calloc(index.records_size, - sizeof(*index.records)))) + if (!XDL_CALLOC_ARRAY(index.records, index.records_size)) goto cleanup; index.line_map_size = count1; - if (!(index.line_map = xdl_calloc(index.line_map_size, - sizeof(*index.line_map)))) + if (!XDL_CALLOC_ARRAY(index.line_map, index.line_map_size)) goto cleanup; - if (!(index.next_ptrs = xdl_calloc(index.line_map_size, - sizeof(*index.next_ptrs)))) + if (!XDL_CALLOC_ARRAY(index.next_ptrs, index.line_map_size)) goto cleanup; /* lines / 4 + 1 comes from xprepare.c:xdl_prepare_ctx() */ diff --git a/xdiff/xmacros.h b/xdiff/xmacros.h index 9fd3c5da91..0977d1615a 100644 --- a/xdiff/xmacros.h +++ b/xdiff/xmacros.h @@ -55,4 +55,7 @@ do { \ ? xdl_malloc((nr) * sizeof(*(p))) \ : NULL) +/* Allocate an array of nr zeroed out elements, returns NULL on failure */ +#define XDL_CALLOC_ARRAY(p, nr) ((p) = xdl_calloc(nr, sizeof(*(p)))) + #endif /* #if !defined(XMACROS_H) */ diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index 94f8886dd1..fe39c2978c 100644 --- a/xdiff/xpatience.c +++ b/xdiff/xpatience.c @@ -151,8 +151,7 @@ static int fill_hashmap(mmfile_t *file1, mmfile_t *file2, /* We know exactly how large we want the hash map */ result->alloc = count1 * 2; - result->entries = xdl_calloc(result->alloc, sizeof(*result->entries)); - if (!result->entries) + if (!XDL_CALLOC_ARRAY(result->entries, result->alloc)) return -1; /* First, fill with entries from the first file */ diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 45190e59fc..b016570c48 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -78,7 +78,7 @@ static int xdl_init_classifier(xdlclassifier_t *cf, long size, long flags) { return -1; } - if (!(cf->rchash = xdl_calloc(cf->hsize, sizeof(*cf->rchash)))) { + if (!XDL_CALLOC_ARRAY(cf->rchash, cf->hsize)) { xdl_cha_free(&cf->ncha); return -1; @@ -182,7 +182,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ hbits = xdl_hashbits((unsigned int) narec); hsize = 1 << hbits; - if (!(rhash = xdl_calloc(hsize, sizeof(*rhash)))) + if (!XDL_CALLOC_ARRAY(rhash, hsize)) goto abort; nrec = 0; @@ -207,7 +207,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ } } - if (!(rchg = xdl_calloc((nrec + 2), sizeof(*rchg)))) + if (!XDL_CALLOC_ARRAY(rchg, nrec + 2)) goto abort; if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && @@ -380,10 +380,8 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd xdlclass_t *rcrec; char *dis, *dis1, *dis2; - if (!(dis = xdl_calloc(xdf1->nrec + xdf2->nrec + 2, sizeof(*dis)))) { - + if (!XDL_CALLOC_ARRAY(dis, xdf1->nrec + xdf2->nrec + 2)) return -1; - } dis1 = dis; dis2 = dis1 + xdf1->nrec + 1; From f7b587bf656574103a0a2ad9c2337b0d15c0e92d Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 8 Jul 2022 16:25:19 +0000 Subject: [PATCH 4/4] xdiff: introduce XDL_ALLOC_GROW() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a helper to grow an array. This is analogous to ALLOC_GROW() in the rest of the codebase but returns −1 on allocation failure to accommodate other users of libxdiff such as libgit2. It will also return a error if the multiplication overflows while calculating the new allocation size. Note that this keeps doubling on reallocation like the code it is replacing rather than increasing the existing size by half like ALLOC_GROW(). It does however copy ALLOC_GROW()'s trick of adding a small amount to the new allocation to avoid a lot of reallocations at small sizes. Note that xdl_alloc_grow_helper() uses long rather than size_t for `nr` and `alloc` to match the existing code. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- xdiff/xmacros.h | 10 ++++++++++ xdiff/xprepare.c | 19 ++++--------------- xdiff/xutils.c | 17 +++++++++++++++++ xdiff/xutils.h | 3 ++- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/xdiff/xmacros.h b/xdiff/xmacros.h index 0977d1615a..8487bb396f 100644 --- a/xdiff/xmacros.h +++ b/xdiff/xmacros.h @@ -58,4 +58,14 @@ do { \ /* Allocate an array of nr zeroed out elements, returns NULL on failure */ #define XDL_CALLOC_ARRAY(p, nr) ((p) = xdl_calloc(nr, sizeof(*(p)))) +/* + * Ensure array p can accommodate at least nr elements, growing the + * array and updating alloc (which is the number of allocated + * elements) as necessary. Frees p and returns -1 on failure, returns + * 0 on success + */ +#define XDL_ALLOC_GROW(p, nr, alloc) \ + (-!((nr) <= (alloc) || \ + ((p) = xdl_alloc_grow_helper((p), (nr), &(alloc), sizeof(*(p)))))) + #endif /* #if !defined(XMACROS_H) */ diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index b016570c48..c84549f6c5 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -111,7 +111,6 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t long hi; char const *line; xdlclass_t *rcrec; - xdlclass_t **rcrecs; line = rec->ptr; hi = (long) XDL_HASHLONG(rec->ha, cf->hbits); @@ -127,14 +126,8 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t return -1; } rcrec->idx = cf->count++; - if (cf->count > cf->alloc) { - cf->alloc *= 2; - if (!(rcrecs = (xdlclass_t **) xdl_realloc(cf->rcrecs, cf->alloc * sizeof(xdlclass_t *)))) { - + if (XDL_ALLOC_GROW(cf->rcrecs, cf->count, cf->alloc)) return -1; - } - cf->rcrecs = rcrecs; - } cf->rcrecs[rcrec->idx] = rcrec; rcrec->line = line; rcrec->size = rec->size; @@ -163,7 +156,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ unsigned long hav; char const *blk, *cur, *top, *prev; xrecord_t *crec; - xrecord_t **recs, **rrecs; + xrecord_t **recs; xrecord_t **rhash; unsigned long *ha; char *rchg; @@ -190,12 +183,8 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ for (top = blk + bsize; cur < top; ) { prev = cur; hav = xdl_hash_record(&cur, top, xpp->flags); - if (nrec >= narec) { - narec *= 2; - if (!(rrecs = (xrecord_t **) xdl_realloc(recs, narec * sizeof(xrecord_t *)))) - goto abort; - recs = rrecs; - } + if (XDL_ALLOC_GROW(recs, nrec + 1, narec)) + goto abort; if (!(crec = xdl_cha_alloc(&xdf->rcha))) goto abort; crec->ptr = prev; diff --git a/xdiff/xutils.c b/xdiff/xutils.c index 115b2b1640..9e36f24875 100644 --- a/xdiff/xutils.c +++ b/xdiff/xutils.c @@ -432,3 +432,20 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp, return 0; } + +void* xdl_alloc_grow_helper(void *p, long nr, long *alloc, size_t size) +{ + void *tmp = NULL; + size_t n = ((LONG_MAX - 16) / 2 >= *alloc) ? 2 * *alloc + 16 : LONG_MAX; + if (nr > n) + n = nr; + if (SIZE_MAX / size >= n) + tmp = xdl_realloc(p, n * size); + if (tmp) { + *alloc = n; + } else { + xdl_free(p); + *alloc = 0; + } + return tmp; +} diff --git a/xdiff/xutils.h b/xdiff/xutils.h index fba7bae03c..fd0bba94e8 100644 --- a/xdiff/xutils.h +++ b/xdiff/xutils.h @@ -42,6 +42,7 @@ int xdl_emit_hunk_hdr(long s1, long c1, long s2, long c2, int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp, int line1, int count1, int line2, int count2); - +/* Do not call this function, use XDL_ALLOC_GROW instead */ +void* xdl_alloc_grow_helper(void* p, long nr, long* alloc, size_t size); #endif /* #if !defined(XUTILS_H) */