Re: [patch] ATTR_* -> ATTRIBUTE_* unification [Re: `sentinel' gcc-3.x/OpenBSD compat.]

[Pedro Alves <pedro@codesourcery.com>]

On Sunday 02 May 2010 13:04:19, Jan Kratochvil wrote:

> Attached, OK to check-in?  It is just mechanical except of one new such
> attribute for find_complaint.  Compiled on x86_64-fedora12-linux-gnu.

Thanks!

> As the new definiton assumes ATTRIBUTE_NONNULL I have checked that none of the
> ATTR_FORMAT uses has NULL as a meaningful format string.  Some empty functions
> have been also 'upgraded' to ATTRIBUTE_NONNULL-using ATTRIBUTE_PRINTF.

The GDB part looks fine to me.  Want to commit it separatelly to the
GDBserver bit?

> gdbserver/
> 2010-05-02  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* server.h (ATTRIBUTE_NONNULL, ATTRIBUTE_PRINTF): New.
> 	(ATTR_FORMAT): Remove.
> 	(buffer_xml_printf, error, fatal, internal_error, warning): Change
> 	ATTR_FORMAT to ATTRIBUTE_PRINTF.
> 	* tracepoint.c (trace_debug_1): Change ATTR_FORMAT to ATTRIBUTE_PRINTF.

> --- a/gdb/gdbserver/server.h
> +++ b/gdb/gdbserver/server.h
> @@ -62,13 +62,22 @@ extern void *memmem (const void *, size_t , const void *, size_t);
>  #endif
>  #endif
>  
> -#ifndef ATTR_FORMAT
> -#if defined(__GNUC__) && (__GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 4))
> -#define ATTR_FORMAT(type, x, y) __attribute__ ((format(type, x, y)))
> -#else
> -#define ATTR_FORMAT(type, x, y) /* nothing */
> -#endif
> -#endif
> +/* Attribute `nonnull' was valid as of gcc 3.3.  */
> +#ifndef ATTRIBUTE_NONNULL
> +# if (GCC_VERSION >= 3003)
> +#  define ATTRIBUTE_NONNULL(m) __attribute__ ((__nonnull__ (m)))
> +# else
> +#  define ATTRIBUTE_NONNULL(m)
> +# endif /* GNUC >= 3.3 */
> +#endif /* ATTRIBUTE_NONNULL */
> +
> +/* Use ATTRIBUTE_PRINTF when the format specifier must not be NULL.
> +   This was the case for the `printf' format attribute by itself
> +   before GCC 3.3, but as of 3.3 we need to add the `nonnull'
> +   attribute to retain this behavior.  */
> +#ifndef ATTRIBUTE_PRINTF
> +#define ATTRIBUTE_PRINTF(m, n) __attribute__ ((__format__ (__printf__, m, n))) ATTRIBUTE_NONNULL(m)
> +#endif /* ATTRIBUTE_PRINTF */
>  

I think this misses defining __attribute__ to empty on non-gcc.  It's
this bit in ansidecl.h that handles it:

#if (GCC_VERSION < 2007)
# define __attribute__(x)
#endif

I also see that nowhere is GCC_VERSION defined in server.h.  My fault, I missed
it when importing gdb_assert to gdbserver.  :-/  We're currently always picking
the else branch in:

if (GCC_VERSION >= 2004)
#define ASSERT_FUNCTION		__PRETTY_FUNCTION__
#else
#if defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
#define ASSERT_FUNCTION		__func__
#endif
#endif

Your patch adds more uses.  If you don't feel like fixing this,
I'll fix it after your patch is in.

-- 
Pedro Alves