Re: [PATCH 1/6] malloc: Use memsize instead of CHUNK_AVAILABLE_SIZE

[DJ Delorie <dj@redhat.com>]

Szabolcs Nagy <szabolcs.nagy@arm.com> writes:
> This is a pure refactoring change that does not affect behaviour.

You hope :-)

LGTM.
Reviewed-by: DJ Delorie <dj@redhat.com>

> -  for (size = CHUNK_AVAILABLE_SIZE (p) - 1;
> +  for (size = CHUNK_HDR_SZ + memsize (p) - 1;

Ok.

> -  max_sz = CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ;
> +  max_sz = memsize (p);

Ok.

> -      for (sz = CHUNK_AVAILABLE_SIZE (p) - 1;
> +      for (sz = CHUNK_HDR_SZ + memsize (p) - 1;

Ok.  Odd that it's not chunksize() but that would affect behavior ;-)

> -      for (sz = CHUNK_AVAILABLE_SIZE (p) - 1;
> +      for (sz = CHUNK_HDR_SZ + memsize (p) - 1;

Ok.

> -      (void)tag_region (chunk2rawmem (p), CHUNK_AVAILABLE_SIZE (p)
> -                                         - CHUNK_HDR_SZ);
> +      (void)tag_region (chunk2rawmem (p), memsize (p));

Ok.

> -/* Available size of chunk.  This is the size of the real usable data
> -   in the chunk, plus the chunk header.  Note: If memory tagging is
> -   enabled the layout changes to accomodate the granule size, this is
> -   wasteful for small allocations so not done by default.  The logic
> -   does not work if chunk headers are not granule aligned.  */
> -_Static_assert (__MTAG_GRANULE_SIZE <= CHUNK_HDR_SZ,
> -		"memory tagging is not supported with large granule.");
> -#define CHUNK_AVAILABLE_SIZE(p)                                       \
> -  (__MTAG_GRANULE_SIZE > SIZE_SZ && __glibc_unlikely (mtag_enabled) ? \
> -    chunksize (p) :                                                   \
> -    chunksize (p) + (chunk_is_mmapped (p) ? 0 : SIZE_SZ))
> -

Dropped, ok.

> +/* This is the size of the real usable data in the chunk.  Not valid for
> +   dumped heap chunks.  */
> +#define memsize(p)                                                    \
> +  (__MTAG_GRANULE_SIZE > SIZE_SZ && __glibc_unlikely (mtag_enabled) ? \
> +    chunksize (p) - CHUNK_HDR_SZ :                                    \
> +    chunksize (p) - CHUNK_HDR_SZ + (chunk_is_mmapped (p) ? 0 : SIZE_SZ))
> +
> +/* If memory tagging is enabled the layout changes to accomodate the granule
> +   size, this is wasteful for small allocations so not done by default.
> +   Both the chunk header and user data has to be granule aligned.  */
> +_Static_assert (__MTAG_GRANULE_SIZE <= CHUNK_HDR_SZ,
> +		"memory tagging is not supported with large granule.");
> +

New, ok.

> -      ptr = __libc_mtag_tag_region (__libc_mtag_new_tag (ptr),
> -				    CHUNK_AVAILABLE_SIZE (cp) - CHUNK_HDR_SZ);
> +      ptr = __libc_mtag_tag_region (__libc_mtag_new_tag (ptr), memsize (cp));

Ok.

> -      (void)tag_region (chunk2rawmem (p),
> -			CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ);
> +      (void)tag_region (chunk2rawmem (p), memsize (p));

Ok.

> -	  size_t sz = CHUNK_AVAILABLE_SIZE (oldp) - CHUNK_HDR_SZ;
> +	  size_t sz = memsize (oldp);

Ok.

> -    return tag_new_zero_region (mem, CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ);
> +    return tag_new_zero_region (mem, memsize (p));

Ok.

> -	      size_t sz = CHUNK_AVAILABLE_SIZE (oldp) - CHUNK_HDR_SZ;
> +	      size_t sz = memsize (oldp);

Ok.

> -	result = CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ;
> +	result = memsize (p);

Ok.