Re: [Patch] Warning fixes for glib

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj gtk org> writes:

> On 29 Oct 2001, Daniel Egger wrote:
> 
> > Hija,
> > 
> > while compiling on ppc-linux with gcc-3.1 (CVS) I fixed some warnings.
> > 
> > OK to commit?
> > 
> > ===================================================================
> > 2001-10-29  Daniel Egger  <degger fhm edu>
> > 
> >        * glib/gbacktrace.h: Fix non-Intel/-Alpha version of the 
> >        G_BREAKPOINT macro to include <signal.h> and use SIGTRAP.
> 
> ===================================================================
> RCS file: /cvs/gnome/glib/glib/gbacktrace.h,v
> retrieving revision 1.3
> diff -u -r1.3 gbacktrace.h
> --- glib/gbacktrace.h   2001/06/26 16:01:14     1.3
> +++ glib/gbacktrace.h   2001/10/29 14:15:33
> @@ -53,7 +53,8 @@
>  #elif defined (__alpha__) && defined (__GNUC__) && __GNUC__ >= 2
>  #  define G_BREAKPOINT()       G_STMT_START{ __asm__ __volatile__ ("bpt"); }G_STMT_END
>  #else  /* !__i386__ && !__alpha__ */
> -#  define G_BREAKPOINT()       G_STMT_START{ raise (5 /* SIGTRAP */); }G_STMT_END
> +#include <signal.h> /* For prototype and signal name definition */ 
> +#  define G_BREAKPOINT()       G_STMT_START{ raise (SIGTRAP); }G_STMT_END
>  #endif /* __i386__ */
>  
>  G_END_DECLS
> 
> 
> the point in writing raise (5 /* SIGTRAP */) was exactly to _not_ include signal.h,
> so please do not commit this. the right way to get rid of the signal is to
> add more asm statements for not-yet-supported platforms.

This really makes no sense at all to me. 

 - We can't just use functions without their prototypes. 
 - Assuming sigtrap is going to be '5' seems very dubious
 - Add more assembly? There is no guarantee that you can _implement_
   raise() in assembly.

If we aren't going to include signal.h, the only possible solution
is to remove the third branch of the #if and say "G_BREAKPOINT()
is implemented on virtually no platforms, don't use in code you
expect users to be able to compile."

Regards,
                                        Owen