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
jos...@codesourcery.com

Reply via email to