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

Pedro Alves
2010-05-02T15:33:29Z

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