From: Phillip Wood Date: Mon, 11 Jul 2022 10:13:06 GMT Subject: Re: [PATCH 5/7] xdiff: use GALLOC_GROW(), not XDL_ALLOC_GROW() Message-ID: In-Reply-To: Hi Ævar On 08/07/2022 15:20, Ævar Arnfjörð Bjarmason wrote: > Replace the recently introduced XDL_ALLOC_GROW() with invocations of > the GALLOC_GROW() from git-shared-util.h. > > As this change shows the macro + function indirection of > XDL_ALLOC_GROW() is something we needed only because the two callsites > we used it in wanted to use it as an expression, and we thus had to > pass the "sizeof" down. > > Let's just check the value afterwards instead, which allows us to use > the shared macro, we can also remove xdl_reallo(), this was its last > user. I don't think this expression->statement change is an improvement. This change also removes the overflow checks that are present in XDL_ALLOC_GROW() and fails to free the old allocation when realloc() fails. It is not a like for like replacement. Best Wishes Phillip > > Signed-off-by: Ævar Arnfjörð Bjarmason > --- > xdiff/xdiff.h | 1 - > xdiff/xmacros.h | 11 ----------- > xdiff/xprepare.c | 8 +++++--- > xdiff/xutils.c | 17 ----------------- > xdiff/xutils.h | 2 -- > 5 files changed, 5 insertions(+), 34 deletions(-) > > diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h > index 72e25a9ffa5..832cf9d977e 100644 > --- a/xdiff/xdiff.h > +++ b/xdiff/xdiff.h > @@ -121,7 +121,6 @@ typedef struct s_bdiffparam { > > #define xdl_malloc(x) xmalloc(x) > #define xdl_free(ptr) free(ptr) > -#define xdl_realloc(ptr,x) xrealloc(ptr,x) > > void *xdl_mmfile_first(mmfile_t *mmf, long *size); > long xdl_mmfile_size(mmfile_t *mmf); > diff --git a/xdiff/xmacros.h b/xdiff/xmacros.h > index 75506bdf17e..6a6b3057375 100644 > --- a/xdiff/xmacros.h > +++ b/xdiff/xmacros.h > @@ -48,15 +48,4 @@ do { \ > (v) = (unsigned long) __p[0] | ((unsigned long) __p[1]) << 8 | \ > ((unsigned long) __p[2]) << 16 | ((unsigned long) __p[3]) << 24; \ > } while (0) > - > -/* > - * 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 d6cbee32a2a..4182d9e1c0a 100644 > --- a/xdiff/xprepare.c > +++ b/xdiff/xprepare.c > @@ -128,8 +128,9 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t > return -1; > } > rcrec->idx = cf->count++; > - if (XDL_ALLOC_GROW(cf->rcrecs, cf->count, cf->alloc)) > - return -1; > + GALLOC_GROW(cf->rcrecs, cf->count, cf->alloc); > + if (!cf->rcrecs) > + return -1; > cf->rcrecs[rcrec->idx] = rcrec; > rcrec->line = line; > rcrec->size = rec->size; > @@ -187,7 +188,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 (XDL_ALLOC_GROW(recs, nrec + 1, narec)) > + GALLOC_GROW(recs, nrec + 1, narec); > + if (!recs) > goto abort; > if (!(crec = xdl_cha_alloc(&xdf->rcha))) > goto abort; > diff --git a/xdiff/xutils.c b/xdiff/xutils.c > index a6f10353cff..c0cd5338c4e 100644 > --- a/xdiff/xutils.c > +++ b/xdiff/xutils.c > @@ -436,20 +436,3 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp, > > return status; > } > - > -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 fd0bba94e8b..7ae3f897bef 100644 > --- a/xdiff/xutils.h > +++ b/xdiff/xutils.h > @@ -42,7 +42,5 @@ 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) */