git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/7] xdiff: use G[C]ALLOC_ARRAY(), not XDL_CALLOC_ARRAY()

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jul 11, 2022, 10:10 UTC
Message-ID
<37667911-3049-4e78-e67d-9fcb7389dfc0@gmail.com>
In-Reply-To
<patch-4.7-19567077b84-20220708T140354Z-avarab@gmail.com>
Hi Ævar
On 08/07/2022 15:20, Ævar Arnfjörð Bjarmason wrote:
Show 11 quoted lines
> Use the newly created GCALLOC_ARRAY() helpers rather than the recently
> introduced XDL_[C]ALLOC_ARRAY().
> 
> As shown in this diff the calling convention differs, we cannot use
> GCALLOC_ARRAY() as an expression, but that's an advantage in not
> having to relay the "sizeof()" down via a wrapper function.
> 
> This also:
> 
>   * Fixes long-standing potential overflow issues, as we're using
>     st_mult() in the underlying G_[C]ALLOC().  Note that the
What issues is this fixing? XDL_ALLOC_ARRAY() already checks for overflow.
>   * Slightly optimizes the "XDL_CALLOC_ARRAY", as we'll now use
>     calloc() rather than malloc() + memset() (although smart compilers
>     will probably do the same for both...).

That's addressed in V2 of my series, unfortunately I sent it just after you'd sent this series.

>   * Changes the "XDL_CALLOC_ARRAY" behavior where we'd shortcut if the
>     size was already large enough, but this behavior was changed when
>     XDL_ALLOC_ARRAY() was introduced, so this is safe.
I'm not sure what you mean here - how did we shortcut before?
Show 35 quoted lines
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   xdiff/xdiffi.c     |  3 ++-
>   xdiff/xhistogram.c |  9 ++++++---
>   xdiff/xmacros.h    | 12 ------------
>   xdiff/xpatience.c  |  6 ++++--
>   xdiff/xprepare.c   | 24 ++++++++++++++++--------
>   5 files changed, 28 insertions(+), 26 deletions(-)
> 
> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
> index 6fded43e87d..077cc456087 100644
> --- a/xdiff/xdiffi.c
> +++ b/xdiff/xdiffi.c
> @@ -333,7 +333,8 @@ 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 (!XDL_ALLOC_ARRAY(kvd, 2 * ndiags + 2))
> +	GALLOC_ARRAY(kvd, 2 * ndiags + 2);
> +	if (!kvd)
>   		return -1;
>   	kvdf = kvd;
>   	kvdb = kvdf + ndiags;
> diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c
> index df909004c10..f20592bfbdd 100644
> --- a/xdiff/xhistogram.c
> +++ b/xdiff/xhistogram.c
> @@ -266,14 +266,17 @@ 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 (!XDL_CALLOC_ARRAY(index.records, index.records_size))
> +	GCALLOC_ARRAY(index.records, index.records_size);
> +	if (!index.records)

I don't think that having GALLOC_ARRAY() as a statement is an improvement here.

Best Wishes
Phillip
Show 133 quoted lines
>   		goto cleanup;
>   
>   	index.line_map_size = count1;
> -	if (!XDL_CALLOC_ARRAY(index.line_map, index.line_map_size))
> +	GCALLOC_ARRAY(index.line_map, index.line_map_size);
> +	if (!index.line_map)
>   		goto cleanup;
>   
> -	if (!XDL_CALLOC_ARRAY(index.next_ptrs, index.line_map_size))
> +	GCALLOC_ARRAY(index.next_ptrs, index.line_map_size);
> +	if (!index.next_ptrs)
>   		goto cleanup;
>   
>   	/* lines / 4 + 1 comes from xprepare.c:xdl_prepare_ctx() */
> diff --git a/xdiff/xmacros.h b/xdiff/xmacros.h
> index d13a6724629..75506bdf17e 100644
> --- a/xdiff/xmacros.h
> +++ b/xdiff/xmacros.h
> @@ -49,18 +49,6 @@ 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)
> -
> -/* Allocate an array of nr zeroed out elements, returns NULL on failure */
> -#define XDL_CALLOC_ARRAY(p, nr)				\
> -	(XDL_ALLOC_ARRAY((p), (nr))			\
> -		? memset((p), 0, (nr) * sizeof(*(p)))	\
> -		: NULL)
> -
>   /*
>    * Ensure array p can accommodate at least nr elements, growing the
>    * array and updating alloc (which is the number of allocated
> diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c
> index fe39c2978cb..bb328d9f852 100644
> --- a/xdiff/xpatience.c
> +++ b/xdiff/xpatience.c
> @@ -151,7 +151,8 @@ static int fill_hashmap(mmfile_t *file1, mmfile_t *file2,
>   
>   	/* We know exactly how large we want the hash map */
>   	result->alloc = count1 * 2;
> -	if (!XDL_CALLOC_ARRAY(result->entries, result->alloc))
> +	GCALLOC_ARRAY(result->entries, result->alloc);
> +	if (!result->entries)
>   		return -1;
>   
>   	/* First, fill with entries from the first file */
> @@ -208,7 +209,8 @@ static int find_longest_common_sequence(struct hashmap *map, struct entry **res)
>   	 */
>   	int anchor_i = -1;
>   
> -	if (!XDL_ALLOC_ARRAY(sequence, map->nr))
> +	GALLOC_ARRAY(sequence, map->nr);
> +	if (!sequence)
>   		return -1;
>   
>   	for (entry = map->first; entry; entry = entry->next) {
> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> index c84549f6c50..d6cbee32a2a 100644
> --- a/xdiff/xprepare.c
> +++ b/xdiff/xprepare.c
> @@ -78,15 +78,17 @@ static int xdl_init_classifier(xdlclassifier_t *cf, long size, long flags) {
>   
>   		return -1;
>   	}
> -	if (!XDL_CALLOC_ARRAY(cf->rchash, cf->hsize)) {
> +	GCALLOC_ARRAY(cf->rchash, cf->hsize);
> +	if (!cf->rchash) {
>   
>   		xdl_cha_free(&cf->ncha);
>   		return -1;
>   	}
>   
>   	cf->alloc = size;
> -	if (!XDL_ALLOC_ARRAY(cf->rcrecs, cf->alloc)) {
>   
> +	GALLOC_ARRAY(cf->rcrecs, cf->alloc);
> +	if (!cf->rcrecs) {
>   		xdl_free(cf->rchash);
>   		xdl_cha_free(&cf->ncha);
>   		return -1;
> @@ -170,12 +172,14 @@ 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 (!XDL_ALLOC_ARRAY(recs, narec))
> +	GALLOC_ARRAY(recs, narec);
> +	if (!recs)
>   		goto abort;
>   
>   	hbits = xdl_hashbits((unsigned int) narec);
>   	hsize = 1 << hbits;
> -	if (!XDL_CALLOC_ARRAY(rhash, hsize))
> +	GCALLOC_ARRAY(rhash, hsize);
> +	if (!rhash)
>   		goto abort;
>   
>   	nrec = 0;
> @@ -196,14 +200,17 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
>   		}
>   	}
>   
> -	if (!XDL_CALLOC_ARRAY(rchg, nrec + 2))
> +	GCALLOC_ARRAY(rchg, nrec + 2);
> +	if (!rchg)
>   		goto abort;
>   
>   	if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) &&
>   	    (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) {
> -		if (!XDL_ALLOC_ARRAY(rindex, nrec + 1))
> +		GALLOC_ARRAY(rindex, nrec + 1);
> +		if (!rindex)
>   			goto abort;
> -		if (!XDL_ALLOC_ARRAY(ha, nrec + 1))
> +		GALLOC_ARRAY(ha, nrec + 1);
> +		if (!ha)
>   			goto abort;
>   	}
>   
> @@ -369,7 +376,8 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
>   	xdlclass_t *rcrec;
>   	char *dis, *dis1, *dis2;
>   
> -	if (!XDL_CALLOC_ARRAY(dis, xdf1->nrec + xdf2->nrec + 2))
> +	GCALLOC_ARRAY(dis, xdf1->nrec + xdf2->nrec + 2);
> +	if (!dis)
>   		return -1;
>   	dis1 = dis;
>   	dis2 = dis1 + xdf1->nrec + 1;
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 26 of 52 in “xdiff: introduce memory allocation macros”
  1. 0/3 xdiff: introduce memory allocation macrosPhillip Wood via GitGitGadget, Jun 29, 2022
  2. 1/3 xdiff: introduce XDL_ALLOC_ARRAY()Phillip Wood via GitGitGadget, Jun 29, 2022
  3. 2/3 xdiff: introduce XDL_CALLOC_ARRAY()Phillip Wood via GitGitGadget, Jun 29, 2022
  4. Junio C HamanoJun 30, 2022
  5. Phillip WoodJul 6, 2022
  6. 3/3 xdiff: introduce XDL_ALLOC_GROW()Phillip Wood via GitGitGadget, Jun 29, 2022
  7. Ævar Arnfjörð BjarmasonJun 30, 2022
  8. Phillip WoodJun 30, 2022
  9. Phillip WoodJun 30, 2022
  10. Ævar Arnfjörð BjarmasonJun 30, 2022
  11. Phillip WoodJul 6, 2022
  12. Ævar Arnfjörð BjarmasonJul 7, 2022
  13. Phillip WoodJul 8, 2022
  14. 0/7 xdiff: use standard alloc macros, share them via git-shared-util.hÆvar Arnfjörð Bjarmason, Jul 8, 2022
  15. 1/7 xdiff: simplify freeing patterns around xdl_free_env()Ævar Arnfjörð Bjarmason, Jul 8, 2022
  16. 2/7 git-shared-util.h: move "shared" allocation utilities hereÆvar Arnfjörð Bjarmason, Jul 8, 2022
  17. 3/7 git-shared-util.h: add G*() versions of *ALLOC_*()Ævar Arnfjörð Bjarmason, Jul 8, 2022
  18. Phillip WoodJul 11, 2022
  19. 5/7 xdiff: use GALLOC_GROW(), not XDL_ALLOC_GROW()Ævar Arnfjörð Bjarmason, Jul 8, 2022
  20. Phillip WoodJul 11, 2022
  21. Ævar Arnfjörð BjarmasonJul 11, 2022
  22. Phillip WoodJul 13, 2022
  23. Ævar Arnfjörð BjarmasonJul 13, 2022
  24. Phillip WoodJul 13, 2022
  25. 4/7 xdiff: use G[C]ALLOC_ARRAY(), not XDL_CALLOC_ARRAY()Ævar Arnfjörð Bjarmason, Jul 8, 2022
  26. Phillip WoodJul 11, 2022
  27. 6/7 xdiff: remove xdl_malloc() wrapper, use malloc(), not xmalloc()Ævar Arnfjörð Bjarmason, Jul 8, 2022
  28. Phillip WoodJul 8, 2022
  29. Ævar Arnfjörð BjarmasonJul 8, 2022
  30. Jeff KingJul 8, 2022
  31. Ævar Arnfjörð BjarmasonJul 8, 2022
  32. Jeff KingJul 11, 2022
  33. 7/7 xdiff: remove xdl_free(), use free() insteadÆvar Arnfjörð Bjarmason, Jul 8, 2022
  34. Phillip WoodJul 8, 2022
  35. Ævar Arnfjörð BjarmasonJul 8, 2022
  36. Phillip WoodJul 11, 2022
  37. Phillip WoodJul 11, 2022
  38. Ævar Arnfjörð BjarmasonJul 11, 2022
  39. Phillip WoodJul 13, 2022
  40. Ævar Arnfjörð BjarmasonJul 13, 2022
  41. Junio C HamanoJun 30, 2022
  42. Phillip WoodJul 6, 2022
  43. Ævar Arnfjörð BjarmasonJun 30, 2022
  44. 0/4 xdiff: introduce memory allocation macrosPhillip Wood via GitGitGadget, Jul 8, 2022
  45. 2/4 xdiff: introduce xdl_callocPhillip Wood via GitGitGadget, Jul 8, 2022
  46. 1/4 xdiff: introduce XDL_ALLOC_ARRAY()Phillip Wood via GitGitGadget, Jul 8, 2022
  47. 4/4 xdiff: introduce XDL_ALLOC_GROW()Phillip Wood via GitGitGadget, Jul 8, 2022
  48. Ævar Arnfjörð BjarmasonJul 8, 2022
  49. Phillip WoodJul 11, 2022
  50. Jeff KingJul 12, 2022
  51. Phillip WoodJul 13, 2022
  52. 3/4 xdiff: introduce XDL_CALLOC_ARRAY()Phillip Wood via GitGitGadget, Jul 8, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.