[Mono-devel-list] Re: Patch: 100% working mono under FreeBSD (small nit)

[Paolo Molaro]

On 04/11/05 Bill Middleton wrote:
> Index: mono/mini/exceptions-x86.c
> ===================================================================
> --- mono/mini/exceptions-x86.c	(revision 42774)
> +++ mono/mini/exceptions-x86.c	(working copy)
> @@ -25,6 +25,10 @@
>  #include "mini.h"
>  #include "mini-x86.h"
>  
> +#if defined(__FreeBSD__)
> +#include <ucontext.h>
> +#endif 

Maybe it's better to check if the header is present in configure
and use HAVE_UCONTEXT_H here? Same with checking for the gregs field
in ctx->uc_mcontext.
In any case it may be better to define the macros to access the registers
in the context and use those in the file:

#if defined(__FreeBSD__)
#define CTX_REG_EAX(ctx) ((ctx)uc_mcontext.mc_eax)
...
#else
#define CTX_REG_EAX(ctx) ((ctx)uc_mcontext.gregs [REG_EAX])
...
#endif

This way the #ifdefs are moved to the header file and not in the middle
of the code.

> Index: mono/mini/mini-x86.c
> ===================================================================
> --- mono/mini/mini-x86.c	(revision 42774)
> +++ mono/mini/mini-x86.c	(working copy)
> @@ -4691,6 +4691,9 @@
>  	pthread_getattr_np( self, &attr );
>  #else
>  #ifdef HAVE_PTHREAD_ATTR_GET_NP
> +#if defined(__FreeBSD__) 
> +	pthread_attr_init( &attr );
> +#endif

This is likely a good idea on any platform, right? So no need for #ifdefs.

> Index: mono/mini/mini-x86.h
> ===================================================================
> --- mono/mini/mini-x86.h	(revision 42774)
> +++ mono/mini/mini-x86.h	(working copy)
> @@ -77,17 +77,21 @@
>  #ifndef PLATFORM_WIN32
>  
>  #ifdef HAVE_WORKING_SIGALTSTACK
> -
>  #define MONO_ARCH_SIGSEGV_ON_ALTSTACK
>  #define MONO_ARCH_USE_SIGACTION
>  
> -/* NetBSD doesn't define SA_STACK */
> -#ifndef SA_STACK
> -#define SA_STACK SA_ONSTACK
> -#endif
> -#endif
> +/* FreeBSD and NetBSD need SA_STACK and MAP_ANON re-definitions */
> +#	if defined(__FreeBSD__) || defined(__NetBSD__) 
> +#		ifndef SA_STACK
> +#			define SA_STACK SA_ONSTACK
> +#		endif
> +#		ifndef MAP_ANONYMOUS
> +#			define MAP_ANONYMOUS MAP_ANON
> +#		endif
> +#	endif /* BSDs */

This stuff doesn't belong to mini-x86.h, though mono-compiler.h is not a 
good place either. In any case, I don't see any need to make this conditional
on __FreeBSD__/__NetBSD__, so remove at least that.

Thanks!
lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better