Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 19 Jan 2020 12:31:34 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: Minor style patch to exit.c

* Markus Wichmann <nullplan@....net> [2020-01-19 12:07:43 +0100]:
> The previous version did have a maze of parentheses here. But the actual
> logic of the function is not to iterate over some numbers that happen to
> be convertible to pointers, but rather to iterate over an array of
> function pointers. So let us use pointer arithmetic correctly.
> ---
>  src/exit/exit.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/exit/exit.c b/src/exit/exit.c
> index a6869b37..e02ba2be 100644
> --- a/src/exit/exit.c
> +++ b/src/exit/exit.c
> @@ -16,9 +16,9 @@ extern weak hidden void (*const __fini_array_start)(void), (*const __fini_array_
> 
>  static void libc_exit_fini(void)
>  {
> -	uintptr_t a = (uintptr_t)&__fini_array_end;
> -	for (; a>(uintptr_t)&__fini_array_start; a-=sizeof(void(*)()))
> -		(*(void (**)())(a-sizeof(void(*)())))();
> +	void (*const *a)() = &__fini_array_end;
> +	while (a > &__fini_array_start)
> +		(*--a)();

this has undefined behaviour.

the original code was carefully written to avoid that.
you need to keep the uintptr_t cast since -- and > are
undefined for pointers that go out of bound or don't
point to the same object (and the _start, _end symbols
don't represent the same c language object, they are
independent).

>  	_fini();
>  }
> 
> --
> 2.17.1
> 

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.