This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch 2/3] ARM 64 bit atomic operations
On Fri, 1 Jul 2011, Dr. David Alan Gilbert wrote:
> +/* For write */
> +#include <unistd.h>
> +/* For abort */
> +#include <stdlib.h>
Please don't include system headers in libgcc without appropriate
inhibit_libc checks for bootstrap purposes. In this case, it would seem
better just to declare the functions you need.
> +/* Check that the kernel has a new enough version at load */
> +void __check_for_sync8_kernelhelper (void)
Shouldn't this function be static?
> +{
> + if (__kernel_helper_version < 5)
> + {
> + const char err[] = "A newer kernel is required to run this binary. (__kernel_cmpxchg64 helper)\n";
> + /* At this point we need a way to crash with some information
> + for the user - I'm not sure I can rely on much else being
> + available at this point, so do the same as generic-morestack.c
> + write() and abort(). */
> + write (2 /* stderr */, err, sizeof(err));
"write" is in the user's namespace in ISO C so it's not ideal to have a
call to it. If there isn't a reserved-namespace version, using the
syscall directly (hardcoding the syscall number) might be better.
> +void (*__sync8_kernelhelper_inithook[]) (void) __attribute__ ((section (".init_array"))) = {
> + &__check_for_sync8_kernelhelper
> +};
Shouldn't this also be static (marked "used" if needed)? Though I'd have
thought simply marking the function as a constructor would be simpler and
better....
--
Joseph S. Myers
joseph@codesourcery.com