This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Simplify perturb_byte logic.
- From: Will Newton <will dot newton at linaro dot org>
- To: Ondřej Bílka <neleai at seznam dot cz>
- Cc: libc-alpha <libc-alpha at sourceware dot org>
- Date: Mon, 9 Dec 2013 15:51:34 +0000
- Subject: Re: [PATCH] Simplify perturb_byte logic.
- Authentication-results: sourceware.org; auth=none
- References: <20131209141613 dot GA15247 at domone dot podge>
On 9 December 2013 14:16, OndÅej BÃlka <neleai@seznam.cz> wrote:
> Hi,
>
> We in malloc used pattern
>
> if (perturb_byte != 0)
> alloc_perturb (p, n);
>
> We could simplify that by moving that check into
> alloc_perturb/free_perturb.
>
> OK to commit?
>
> * malloc/malloc.c (alloc_perturb, free_perturb): Add
> perturb_byte == 0 optimization.
> (_int_malloc, _int_free): Remove perturb_byte == 0 conditions.
This looks ok in general and is a nice cleanup. Not sure if it is an
optimization though (as the generated code should be the sam), so the
ChangeLog entry might be better as something like:
* malloc/malloc.c (alloc_perturb, free_perturb): Convert from
macro to a function. Check for zero perturb_byte.
(_int_malloc, _int_free): Remove zero perturb_byte checks.
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 4821deb..acfa9a2 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -1870,8 +1870,20 @@ static int check_action = DEFAULT_CHECK_ACTION;
>
> static int perturb_byte;
>
> -#define alloc_perturb(p, n) memset (p, (perturb_byte ^ 0xff) & 0xff, n)
> -#define free_perturb(p, n) memset (p, perturb_byte & 0xff, n)
> +static inline void
> +alloc_perturb (char *p, size_t n)
> +{
> + if (__glibc_unlikely (perturb_byte))
> + memset (p, (perturb_byte ^ 0xff) & 0xff, n);
> +}
> +
> +static inline void
> +free_perturb (char *p, size_t n)
> +{
> + if (__glibc_unlikely (perturb_byte))
> + memset (p, perturb_byte & 0xff, n);
> +}
Do these checks count as boolean coercion? i.e. should they be
"perturb_byte != 0"?
Also a bit odd that "perturb_byte" is actually an int. :-/
--
Will Newton
Toolchain Working Group, Linaro