[patch 1/3] Make obconcat use stdarg

[patch 1/3] Make obconcat use stdarg

[Jan Kratochvil]

Hi,

__attribute__ ((sentinel)) availability for gcc >= 4.0 I have copied from
<glib-2.0/glib/gmacros.h>.  It roughly matches the GCC ChangeLog dates.

Using it in patch 3/3 only in typename_concat for 4 strings; it has supported
now only 3 strings.  This patch is very unimportand and it can be dropped with
some simple-but-ugly hack just in typename_concat of the patch 3/3.

Still I find this patch as a code cleanup.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu for the whole
patchset.


Thanks,
Jan


2010-04-30  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* defs.h (ATTR_SENTINEL): New.
	* dwarf2read.c (typename_concat): Use NULL-terminated stdarg list for
	the obconcat call.
	* mdebugread.c (parse_symbol): Likewise.
	* stabsread.c (define_symbol, read_member_functions, read_cpp_abbrev):
	Likewise.
	* symfile.c (obconcat): Replace the s1, s2 and s3 parameters by `...'.
	New variables ap, dest, rename val to retval.
	* symfile.h (obconcat): Likewise for the prototype.

--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -304,6 +304,14 @@ struct cleanup
 #endif
 #endif
 
+#ifndef ATTR_SENTINEL
+#if defined(__GNUC__) && __GNUC__ >= 4
+#define ATTR_SENTINEL __attribute__ ((sentinel))
+#else
+#define ATTR_SENTINEL		/* nothing */
+#endif
+#endif
+
 /* Be conservative and use enum bitfields only with GCC.
    This is copied from gcc 3.3.1, system.h.  */
 
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9223,7 +9223,7 @@ typename_concat (struct obstack *obs, const char *prefix, const char *suffix,
   else
     {
       /* We have an obstack.  */
-      return obconcat (obs, prefix, sep, suffix);
+      return obconcat (obs, prefix, sep, suffix, NULL);
     }
 }
 
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -998,8 +998,8 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
 	if (sh->iss == 0 || name[0] == '.' || name[0] == '\0')
 	  TYPE_TAG_NAME (t) = NULL;
 	else
-	  TYPE_TAG_NAME (t) = obconcat (&current_objfile->objfile_obstack,
-					"", "", name);
+	  TYPE_TAG_NAME (t) = obconcat (&current_objfile->objfile_obstack, name,
+					NULL);
 
 	TYPE_CODE (t) = type_code;
 	TYPE_LENGTH (t) = sh->value;
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -1280,9 +1280,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
           SYMBOL_VALUE (struct_sym) = valu;
           SYMBOL_DOMAIN (struct_sym) = STRUCT_DOMAIN;
           if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
-            TYPE_NAME (SYMBOL_TYPE (sym))
-              = obconcat (&objfile->objfile_obstack, "", "",
-                          SYMBOL_LINKAGE_NAME (sym));
+            TYPE_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      NULL);
           add_symbol_to_list (struct_sym, &file_symbols);
         }
       
@@ -1307,9 +1307,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
       SYMBOL_VALUE (sym) = valu;
       SYMBOL_DOMAIN (sym) = STRUCT_DOMAIN;
       if (TYPE_TAG_NAME (SYMBOL_TYPE (sym)) == 0)
-	TYPE_TAG_NAME (SYMBOL_TYPE (sym))
-	  = obconcat (&objfile->objfile_obstack, "", "",
-		      SYMBOL_LINKAGE_NAME (sym));
+	TYPE_TAG_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      NULL);
       add_symbol_to_list (sym, &file_symbols);
 
       if (synonym)
@@ -1322,9 +1322,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
 	  SYMBOL_VALUE (typedef_sym) = valu;
 	  SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 	  if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
-	    TYPE_NAME (SYMBOL_TYPE (sym))
-	      = obconcat (&objfile->objfile_obstack, "", "",
-			  SYMBOL_LINKAGE_NAME (sym));
+	    TYPE_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      NULL);
 	  add_symbol_to_list (typedef_sym, &file_symbols);
 	}
       break;
@@ -2610,8 +2610,8 @@ read_member_functions (struct field_info *fip, char **pp, struct type *type,
 	      make_cleanup (xfree, destr_fnlist);
 	      memset (destr_fnlist, 0, sizeof (struct next_fnfieldlist));
 	      destr_fnlist->fn_fieldlist.name
-		= obconcat (&objfile->objfile_obstack, "", "~",
-			    new_fnlist->fn_fieldlist.name);
+		= obconcat (&objfile->objfile_obstack, "~",
+			    new_fnlist->fn_fieldlist.name, NULL);
 
 	      destr_fnlist->fn_fieldlist.fn_fields = (struct fn_field *)
 		obstack_alloc (&objfile->objfile_obstack,
@@ -2748,8 +2748,8 @@ read_cpp_abbrev (struct field_info *fip, char **pp, struct type *type,
 	  {
 		  name = "";
 	  }
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack, vptr_name, name, "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack,
+					    vptr_name, name, NULL);
 	  break;
 
 	case 'b':		/* $vb -- a virtual bsomethingorother */
@@ -2761,15 +2761,14 @@ read_cpp_abbrev (struct field_info *fip, char **pp, struct type *type,
 			 symnum);
 	      name = "FOO";
 	    }
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack, vb_name, name, "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack, vb_name,
+					    name, NULL);
 	  break;
 
 	default:
 	  invalid_cpp_abbrev_complaint (*pp);
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack,
-		      "INVALID_CPLUSPLUS_ABBREV", "", "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack,
+					    "INVALID_CPLUSPLUS_ABBREV", NULL);
 	  break;
 	}
 
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -212,19 +212,46 @@ obsavestring (const char *ptr, int size, struct obstack *obstackp)
   return p;
 }
 
-/* Concatenate strings S1, S2 and S3; return the new string.  Space is found
-   in the obstack pointed to by OBSTACKP.  */
+/* Concatenate NULL terminated variable argument list of `const char *' strings;
+   return the new string.  Space is found in the OBSTACKP  */
 
 char *
-obconcat (struct obstack *obstackp, const char *s1, const char *s2,
-	  const char *s3)
-{
-  int len = strlen (s1) + strlen (s2) + strlen (s3) + 1;
-  char *val = (char *) obstack_alloc (obstackp, len);
-  strcpy (val, s1);
-  strcat (val, s2);
-  strcat (val, s3);
-  return val;
+obconcat (struct obstack *obstackp, ...)
+{
+  va_list ap;
+  size_t len = 1;
+  char *retval, *dest;
+
+  va_start (ap, obstackp);
+  for (;;)
+    {
+      const char *s = va_arg (ap, const char *);
+
+      if (s == NULL)
+	break;
+      len += strlen (s);
+    }
+  va_end (ap);
+
+  retval = (char *) obstack_alloc (obstackp, len);
+  dest = retval;
+
+  va_start (ap, obstackp);
+  for (;;)
+    {
+      const char *s = va_arg (ap, const char *);
+      size_t l;
+
+      if (s == NULL)
+	break;
+      l = strlen (s);
+      memcpy (dest, s, l);
+      dest += l;
+    }
+  va_end (ap);
+  *dest = 0;
+
+  return retval;
 }
 
 /* True if we are reading a symbol table. */
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -458,11 +458,10 @@ extern struct partial_symtab *start_psymtab_common (struct objfile *,
 
 extern char *obsavestring (const char *, int, struct obstack *);
 
-/* Concatenate strings S1, S2 and S3; return the new string.  Space is
-   found in the OBSTACKP  */
+/* Concatenate NULL terminated variable argument list of `const char *' strings;
+   return the new string.  Space is found in the OBSTACKP  */
 
-extern char *obconcat (struct obstack *obstackp, const char *, const char *,
-		       const char *);
+extern char *obconcat (struct obstack *obstackp, ...) ATTR_SENTINEL;
 
 			/*   Variables   */
 

Re: [patch 1/3] Make obconcat use stdarg

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> __attribute__ ((sentinel)) availability for gcc >= 4.0 I have copied from
Jan> <glib-2.0/glib/gmacros.h>.  It roughly matches the GCC ChangeLog dates.

Jan> Still I find this patch as a code cleanup.

I agree.

Jan> +obconcat (struct obstack *obstackp, ...)
Jan> +{
Jan> +  va_list ap;
Jan> +  size_t len = 1;
Jan> +  char *retval, *dest;
Jan> +
Jan> +  va_start (ap, obstackp);
Jan> +  for (;;)
Jan> +    {
Jan> +      const char *s = va_arg (ap, const char *);
Jan> +
Jan> +      if (s == NULL)
Jan> +	break;
Jan> +      len += strlen (s);
Jan> +    }
Jan> +  va_end (ap);

I think it is better to just do the copying in a single loop.
Obstacks already handle growing objects well enough, there's no need to
work around this.

Personally I think this function would be better placed in util.c, but
that isn't a requirement for patch.

Tom

Re: [patch 1/3] Make obconcat use stdarg

[Jan Kratochvil]

On Fri, 30 Apr 2010 20:38:49 +0200, Tom Tromey wrote:
> I think it is better to just do the copying in a single loop.
> Obstacks already handle growing objects well enough, there's no need to
> work around this.

True, fixed.


Thanks,
Jan


2010-04-30  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* defs.h (ATTR_SENTINEL): New.
	* dwarf2read.c (typename_concat): Use NULL-terminated stdarg list for
	the obconcat call.
	* mdebugread.c (parse_symbol): Likewise.
	* stabsread.c (define_symbol, read_member_functions, read_cpp_abbrev):
	Likewise.
	* symfile.c (obconcat): Replace the s1, s2 and s3 parameters by `...'.
	New variable ap.  Remove variables len and val.
	* symfile.h (obconcat): Likewise for the prototype.

--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -304,6 +304,14 @@ struct cleanup
 #endif
 #endif
 
+#ifndef ATTR_SENTINEL
+#if defined(__GNUC__) && __GNUC__ >= 4
+#define ATTR_SENTINEL __attribute__ ((sentinel))
+#else
+#define ATTR_SENTINEL		/* nothing */
+#endif
+#endif
+
 /* Be conservative and use enum bitfields only with GCC.
    This is copied from gcc 3.3.1, system.h.  */
 
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9223,7 +9223,7 @@ typename_concat (struct obstack *obs, const char *prefix, const char *suffix,
   else
     {
       /* We have an obstack.  */
-      return obconcat (obs, prefix, sep, suffix);
+      return obconcat (obs, prefix, sep, suffix, NULL);
     }
 }
 
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -998,8 +998,8 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
 	if (sh->iss == 0 || name[0] == '.' || name[0] == '\0')
 	  TYPE_TAG_NAME (t) = NULL;
 	else
-	  TYPE_TAG_NAME (t) = obconcat (&current_objfile->objfile_obstack,
-					"", "", name);
+	  TYPE_TAG_NAME (t) = obconcat (&current_objfile->objfile_obstack, name,
+					NULL);
 
 	TYPE_CODE (t) = type_code;
 	TYPE_LENGTH (t) = sh->value;
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -1280,9 +1280,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
           SYMBOL_VALUE (struct_sym) = valu;
           SYMBOL_DOMAIN (struct_sym) = STRUCT_DOMAIN;
           if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
-            TYPE_NAME (SYMBOL_TYPE (sym))
-              = obconcat (&objfile->objfile_obstack, "", "",
-                          SYMBOL_LINKAGE_NAME (sym));
+            TYPE_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      NULL);
           add_symbol_to_list (struct_sym, &file_symbols);
         }
       
@@ -1307,9 +1307,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
       SYMBOL_VALUE (sym) = valu;
       SYMBOL_DOMAIN (sym) = STRUCT_DOMAIN;
       if (TYPE_TAG_NAME (SYMBOL_TYPE (sym)) == 0)
-	TYPE_TAG_NAME (SYMBOL_TYPE (sym))
-	  = obconcat (&objfile->objfile_obstack, "", "",
-		      SYMBOL_LINKAGE_NAME (sym));
+	TYPE_TAG_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      NULL);
       add_symbol_to_list (sym, &file_symbols);
 
       if (synonym)
@@ -1322,9 +1322,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
 	  SYMBOL_VALUE (typedef_sym) = valu;
 	  SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 	  if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
-	    TYPE_NAME (SYMBOL_TYPE (sym))
-	      = obconcat (&objfile->objfile_obstack, "", "",
-			  SYMBOL_LINKAGE_NAME (sym));
+	    TYPE_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      NULL);
 	  add_symbol_to_list (typedef_sym, &file_symbols);
 	}
       break;
@@ -2610,8 +2610,8 @@ read_member_functions (struct field_info *fip, char **pp, struct type *type,
 	      make_cleanup (xfree, destr_fnlist);
 	      memset (destr_fnlist, 0, sizeof (struct next_fnfieldlist));
 	      destr_fnlist->fn_fieldlist.name
-		= obconcat (&objfile->objfile_obstack, "", "~",
-			    new_fnlist->fn_fieldlist.name);
+		= obconcat (&objfile->objfile_obstack, "~",
+			    new_fnlist->fn_fieldlist.name, NULL);
 
 	      destr_fnlist->fn_fieldlist.fn_fields = (struct fn_field *)
 		obstack_alloc (&objfile->objfile_obstack,
@@ -2748,8 +2748,8 @@ read_cpp_abbrev (struct field_info *fip, char **pp, struct type *type,
 	  {
 		  name = "";
 	  }
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack, vptr_name, name, "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack,
+					    vptr_name, name, NULL);
 	  break;
 
 	case 'b':		/* $vb -- a virtual bsomethingorother */
@@ -2761,15 +2761,14 @@ read_cpp_abbrev (struct field_info *fip, char **pp, struct type *type,
 			 symnum);
 	      name = "FOO";
 	    }
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack, vb_name, name, "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack, vb_name,
+					    name, NULL);
 	  break;
 
 	default:
 	  invalid_cpp_abbrev_complaint (*pp);
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack,
-		      "INVALID_CPLUSPLUS_ABBREV", "", "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack,
+					    "INVALID_CPLUSPLUS_ABBREV", NULL);
 	  break;
 	}
 
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -212,19 +212,28 @@ obsavestring (const char *ptr, int size, struct obstack *obstackp)
   return p;
 }
 
-/* Concatenate strings S1, S2 and S3; return the new string.  Space is found
-   in the obstack pointed to by OBSTACKP.  */
+/* Concatenate NULL terminated variable argument list of `const char *' strings;
+   return the new string.  Space is found in the OBSTACKP  */
 
 char *
-obconcat (struct obstack *obstackp, const char *s1, const char *s2,
-	  const char *s3)
-{
-  int len = strlen (s1) + strlen (s2) + strlen (s3) + 1;
-  char *val = (char *) obstack_alloc (obstackp, len);
-  strcpy (val, s1);
-  strcat (val, s2);
-  strcat (val, s3);
-  return val;
+obconcat (struct obstack *obstackp, ...)
+{
+  va_list ap;
+
+  va_start (ap, obstackp);
+  for (;;)
+    {
+      const char *s = va_arg (ap, const char *);
+
+      if (s == NULL)
+	break;
+
+      obstack_grow_str (obstackp, s);
+    }
+  va_end (ap);
+  obstack_1grow (obstackp, 0);
+
+  return obstack_finish (obstackp);
 }
 
 /* True if we are reading a symbol table. */
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -458,11 +458,10 @@ extern struct partial_symtab *start_psymtab_common (struct objfile *,
 
 extern char *obsavestring (const char *, int, struct obstack *);
 
-/* Concatenate strings S1, S2 and S3; return the new string.  Space is
-   found in the OBSTACKP  */
+/* Concatenate NULL terminated variable argument list of `const char *' strings;
+   return the new string.  Space is found in the OBSTACKP  */
 
-extern char *obconcat (struct obstack *obstackp, const char *, const char *,
-		       const char *);
+extern char *obconcat (struct obstack *obstackp, ...) ATTR_SENTINEL;
 
 			/*   Variables   */
 

Re: [patch 1/3] Make obconcat use stdarg

[Mark Kettenis]

> Date: Fri, 30 Apr 2010 20:16:05 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Hi,
> 
> __attribute__ ((sentinel)) availability for gcc >= 4.0 I have copied from
> <glib-2.0/glib/gmacros.h>.  It roughly matches the GCC ChangeLog dates.

The OpenBSD system compile, which is based on GCC 3.3.5, already has
the sentinel attribute.

> No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu for the whole
> patchset.

I'm afraid you'll need an explicit cast for the NULLs used as the
sentinel value, otherwise platforms that

  #define NULL 0L

will generate warnings like:

  sentinel.c: In function 'foo':
  sentinel.c:8: warning: missing sentinel in function call

and with -Werror, that's not a good :(.

`sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Jan Kratochvil]

On Fri, 30 Apr 2010 21:08:48 +0200, Mark Kettenis wrote:
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > 
> > __attribute__ ((sentinel)) availability for gcc >= 4.0 I have copied from
> > <glib-2.0/glib/gmacros.h>.  It roughly matches the GCC ChangeLog dates.
> 
> The OpenBSD system compile, which is based on GCC 3.3.5, already has
> the sentinel attribute.

FSF GCC 3.3.5 does not support this attribute:
	void f (int i, ...) __attribute__ ((__sentinel__));
	sentinel.c:1: warning: `__sentinel__' attribute directive ignored
	(+ there are no traces of any `sentinel' in the gcc-3.3.5 sources)

OpenBSD-4.6-amd64 from md5sum
	28edaa65b16bd5381ebf645d27039a94  openbsd-install46.iso
does not boot for me on Fedora 12 x86_64 KVM using
	kernel-2.6.32.11-99.fc12.x86_64
	qemu-system-x86-0.11.0-13.fc12.x86_64
as it hangs on "setting tty flags"; I have not found any OpenBSD Live image.

Moreover I do not consider relevant to support this attribute on so obsolete
compiler.  Not using the attribute has no effect on users building GDB.
This attribute is useful only for the GDB developers.


> I'm afraid you'll need an explicit cast for the NULLs used as the
> sentinel value, otherwise platforms that
> 
>   #define NULL 0L
> 
> will generate warnings like:
> 
>   sentinel.c: In function 'foo':
>   sentinel.c:8: warning: missing sentinel in function call
> 
> and with -Werror, that's not a good :(.

Which platforms?  Could you provide a login there?  As NULL is commonly
defined using (such as in <dbus-1.0/dbus/dbus-macros.h>):

#ifndef NULL
#  ifdef __cplusplus
#    define NULL        (0L)
#  else /* !__cplusplus */
#    define NULL        ((void*) 0)
#  endif /* !__cplusplus */
#endif

forcing NULL to be ((void *) 0) may make GDB C++ compatibility problematic
(I do not understand reasons for the conditional above, though).

OTOH a requirement of some special sentinel expression instead of simple `NULL'
for obconcat would nullify its stdarg convenience.

Therefore something could be placed to configure.ac for NULL redefinition
depending on the test on the specific build system.


Thanks,
Jan

Re: `sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Pedro Alves]

First off, to be clear, I like the idea of the patch.  I was going
to mention the need for the cast as well, so, excuse me jumping in.

On Friday 30 April 2010 23:18:39, Jan Kratochvil wrote:
> FSF GCC 3.3.5 does not support this attribute:
>         void f (int i, ...) __attribute__ ((__sentinel__));
>         sentinel.c:1: warning: `__sentinel__' attribute directive ignored
>         (+ there are no traces of any `sentinel' in the gcc-3.3.5 sources)

I sure have seen this with OpenBSD's 3.x.x compiler.

You must be aware of libiberty's `concat', but have you seen
src/include/ansidecl.h's ATTRIBUTE_SENTINEL?

I don't understand why we reinvent these macros for this, given
that one of the first things defs.h does is include ansidecl.h.
E.g, defs.h has both ATTR_FORMAT which looks like could be
replaced by ATTRIBUTE_PRINTF, and also has ATTR_NORETURN, which
looks like could be replaced by ATTRIBUTE_NORETURN.  We already
use ATTRIBUTE_UNUSED in a lot of places.

> OTOH a requirement of some special sentinel expression instead of simple `NULL'
> for obconcat would nullify its stdarg convenience.

I don't understand what you mean here.  Why not just do what we do
with `concat' to handle this (which is what the gcc manual says to
do), that is, just write `(char *) NULL' instead of NULL for
the sentinel, and be done with it.

-- 
Pedro Alves

Re: `sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Eli Zaretskii]

> Date: Sat, 1 May 2010 00:18:39 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> > I'm afraid you'll need an explicit cast for the NULLs used as the
> > sentinel value, otherwise platforms that
> > 
> >   #define NULL 0L
> > 
> > will generate warnings like:
> > 
> >   sentinel.c: In function 'foo':
> >   sentinel.c:8: warning: missing sentinel in function call
> > 
> > and with -Werror, that's not a good :(.
> 
> Which platforms?

DJGPP (a.k.a. GO32) is one.

Re: `sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Mark Kettenis]

> Date: Sat, 1 May 2010 00:18:39 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Fri, 30 Apr 2010 21:08:48 +0200, Mark Kettenis wrote:
> > > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > > 
> > > __attribute__ ((sentinel)) availability for gcc >= 4.0 I have copied from
> > > <glib-2.0/glib/gmacros.h>.  It roughly matches the GCC ChangeLog dates.
> > 
> > The OpenBSD system compile, which is based on GCC 3.3.5, already has
> > the sentinel attribute.
> 
> FSF GCC 3.3.5 does not support this attribute:
> 	void f (int i, ...) __attribute__ ((__sentinel__));
> 	sentinel.c:1: warning: `__sentinel__' attribute directive ignored
> 	(+ there are no traces of any `sentinel' in the gcc-3.3.5 sources)
> 
> OpenBSD-4.6-amd64 from md5sum
> 	28edaa65b16bd5381ebf645d27039a94  openbsd-install46.iso
> does not boot for me on Fedora 12 x86_64 KVM using
> 	kernel-2.6.32.11-99.fc12.x86_64
> 	qemu-system-x86-0.11.0-13.fc12.x86_64
> as it hangs on "setting tty flags"; I have not found any OpenBSD Live image.

OpenBSD runs on real hardware so we blame this one on KVM ;)

> Moreover I do not consider relevant to support this attribute on so obsolete
> compiler.  Not using the attribute has no effect on users building GDB.
> This attribute is useful only for the GDB developers.

Fair enough; this feature is nice to have but in no way essential.

> > I'm afraid you'll need an explicit cast for the NULLs used as the
> > sentinel value, otherwise platforms that
> > 
> >   #define NULL 0L
> > 
> > will generate warnings like:
> > 
> >   sentinel.c: In function 'foo':
> >   sentinel.c:8: warning: missing sentinel in function call
> > 
> > and with -Werror, that's not a good :(.
> 
> Which platforms?  Could you provide a login there?  As NULL is commonly
> defined using (such as in <dbus-1.0/dbus/dbus-macros.h>):
> 
> #ifndef NULL
> #  ifdef __cplusplus
> #    define NULL        (0L)
> #  else /* !__cplusplus */
> #    define NULL        ((void*) 0)
> #  endif /* !__cplusplus */
> #endif
> 
> forcing NULL to be ((void *) 0) may make GDB C++ compatibility problematic
> (I do not understand reasons for the conditional above, though).

Beats me why the dbus developers think they have to provide their own
definition of NULL when one is readily available by including <stddef.h>, even on freestanding environments.

I don't see why using (char *)NULL as sentinel would cause problems in
C++.  Anyway, GDB is written in ISO C90, not C++.

> OTOH a requirement of some special sentinel expression instead of
> simple `NULL' for obconcat would nullify its stdarg convenience.

Dunno, I don't think using (char *)NULL is much more complicated.  And
ultimately, given the way a null pointer constant is defined in ISO
C99, the cast is simply required for varargs functions.

> Therefore something could be placed to configure.ac for NULL
> redefinition depending on the test on the specific build system.

I think that would be a bad idea.

Re: `sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Jan Kratochvil]

On Sat, 01 May 2010 08:06:47 +0200, Eli Zaretskii wrote:
> > Date: Sat, 1 May 2010 00:18:39 +0200
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > Cc: gdb-patches@sourceware.org
> > 
> > > I'm afraid you'll need an explicit cast for the NULLs used as the
> > > sentinel value, otherwise platforms that
> > > 
> > >   #define NULL 0L
> > > 
> > > will generate warnings like:
> > > 
> > >   sentinel.c: In function 'foo':
> > >   sentinel.c:8: warning: missing sentinel in function call
> > > 
> > > and with -Werror, that's not a good :(.
> > 
> > Which platforms?
> 
> DJGPP (a.k.a. GO32) is one.

#include <stddef.h>
void f (int i, ...) __attribute__ ((__sentinel__));
void 
f (int i, ...)
{
}
void
g (void)
{
  f (1, NULL);
}

On gcc442b.zip it works correctly.

no warning:
  f (1, NULL);
  f (1, (void *)0);
  f (1, ((void *)0));	/* output by gcc -E */
warning:
  f (1, 0);
  f (1, 0L);

Any other platform?


Thanks,
Jan

Re: [patch 1/3] Make obconcat use stdarg

[Jan Kratochvil]

On Fri, 30 Apr 2010 21:08:48 +0200, Mark Kettenis wrote:
> I'm afraid you'll need an explicit cast for the NULLs used as the
> sentinel value

There is also needed a fix for cases already protected by ATTRIBUTE_SENTINEL
and already using just the bare "NULL" sentinel:

concat reconcat concat_length concat_copy concat_copy2

fbsd-nat.c by Mark Kettenis 50e330a0 2004-09-19
(reconcat has ATTRIBUTE_SENTINEL since bf7628f0 2004-09-05)
        psargs = reconcat (psargs, psargs, " ", get_inferior_args (), NULL);
remote.c [HAVE_LIBEXPAT]
    remote_support_xml = concat ("xmlRegisters=", xml, NULL);
      p = concat (remote_support_xml, ",", xml, NULL);
remote.c d5ea7042 2010-03-30
      char *p = concat (msg, ";", append, NULL);
          char *p = concat ("qSupported:", q, NULL);
      name = concat ("$", tsv->name, NULL);
utils.c 8b58bcf2 2009-01-26
                  concat ("maintenance set ", problem->name, " ", NULL),
                  concat ("maintenance show ", problem->name, " ", NULL),

Nobody has noticed this problem since 2004.


> , otherwise platforms that
> 
>   #define NULL 0L

So DJGPP is not such platform, which other platforms?  FreeBSD apparently also
does not have this problem.


Thanks,
Jan

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

[Jan Kratochvil]

> On Friday 30 April 2010 23:18:39, Jan Kratochvil wrote:
> > FSF GCC 3.3.5 does not support this attribute:
> >         sentinel.c:1: warning: `__sentinel__' attribute directive ignored
> 
On Fri, 30 Apr 2010 21:08:48 +0200, Mark Kettenis wrote:
# The OpenBSD system compile, which is based on GCC 3.3.5, already has
On Sat, 01 May 2010 02:10:37 +0200, Pedro Alves wrote:
> I sure have seen this with OpenBSD's 3.x.x compiler.
include/ansidecl.h:
/* Attribute `sentinel' was valid as of gcc 3.5.  */

Isn't it more true for OpenBSD's 3.5.x - and not 3.3.5 - one?


> You must be aware of libiberty's `concat', but have you seen
> src/include/ansidecl.h's ATTRIBUTE_SENTINEL?

Neither of them.


> E.g, defs.h has both ATTR_FORMAT which looks like could be
> replaced by ATTRIBUTE_PRINTF,

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.

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.


> and also has ATTR_NORETURN, which looks like could be replaced by
> ATTRIBUTE_NORETURN.

That would come next after a check-in.


Thanks,
Jan


gdb/
2010-05-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* ada-lang.c (lim_warning): Change ATTR_FORMAT to ATTRIBUTE_PRINTF.
	* amd64-tdep.c (amd64_insn_length_fprintf): Likewise.
	* cli-out.c (cli_field_fmt, cli_message, out_field_fmt): Likewise.
	* complaints.c (find_complaint): New ATTRIBUTE_PRINTF.
	(vcomplaint): Change ATTR_FORMAT to ATTRIBUTE_PRINTF.
	* complaints.h (complaint, internal_complaint): Likewise.
	* defs.h: Change ATTR_FORMAT to ATTRIBUTE_PRINTF in the top comment.
	(ATTR_FORMAT): Remove.
	(query, nquery, yquery, vprintf_filtered, vfprintf_filtered)
	(fprintf_filtered, fprintfi_filtered, printf_filtered, printfi_filtered)
	(vprintf_unfiltered, vfprintf_unfiltered, fprintf_unfiltered)
	(printf_unfiltered, xasprintf, xvasprintf, xstrprintf, xstrvprintf)
	(xsnprintf, verror, error, vfatal, fatal, internal_verror)
	(internal_error, internal_vwarning, internal_warning, warning)
	(vwarning): Change ATTR_FORMAT to ATTRIBUTE_PRINTF.
	* disasm.c (fprintf_disasm): Likewise.
	* exceptions.c (throw_it): Likewise.
	* exceptions.h (exception_fprintf, throw_verror, throw_vfatal)
	(throw_error): Likewise.
	* language.h (type_error, range_error): Likewise.
	* linespec.c (cplusplus_error): Likewise.
	* mi/mi-interp.c (mi_interp_query_hook): Likewise.
	* mi/mi-out.c (mi_field_fmt, mi_message): Likewise.
	* monitor.c (monitor_debug): Likewise.
	* parser-defs.h (parser_fprintf): Likewise.
	* serial.h (serial_printf): Likewise.
	* tui/tui-hooks.c (tui_query_hook): Likewise.
	* ui-out.c (default_field_fmt, default_message, uo_field_fmt)
	(uo_message): Likewise.
	* ui-out.h (ui_out_field_fmt, ui_out_message): Likewise.
	* utils.c (vfprintf_maybe_filtered, internal_vproblem, defaulted_query):
	Likewise.
	* xml-support.h (gdb_xml_debug, gdb_xml_error): Likewise.

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/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -501,7 +501,7 @@ cond_offset_target (CORE_ADDR address, long offset)
 
 /* FIXME: cagney/2004-10-10: This function is mimicking the behavior
    provided by "complaint".  */
-static void lim_warning (const char *format, ...) ATTR_FORMAT (printf, 1, 2);
+static void lim_warning (const char *format, ...) ATTRIBUTE_PRINTF (1, 2);
 
 static void
 lim_warning (const char *format, ...)
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1026,7 +1026,7 @@ amd64_skip_prefixes (gdb_byte *insn)
    This function is a nop, we don't want to print anything, we just want to
    compute the length of the insn.  */
 
-static int ATTR_FORMAT (printf, 2, 3)
+static int ATTRIBUTE_PRINTF (2, 3)
 amd64_insn_length_fprintf (void *stream, const char *format, ...)
 {
   return 0;
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -51,12 +51,12 @@ static void cli_field_string (struct ui_out *uiout, int fldno, int width,
 static void cli_field_fmt (struct ui_out *uiout, int fldno,
 			   int width, enum ui_align align,
 			   const char *fldname, const char *format,
-			   va_list args) ATTR_FORMAT (printf, 6, 0);
+			   va_list args) ATTRIBUTE_PRINTF (6, 0);
 static void cli_spaces (struct ui_out *uiout, int numspaces);
 static void cli_text (struct ui_out *uiout, const char *string);
 static void cli_message (struct ui_out *uiout, int verbosity,
 			 const char *format, va_list args)
-     ATTR_FORMAT (printf, 3, 0);
+     ATTRIBUTE_PRINTF (3, 0);
 static void cli_wrap_hint (struct ui_out *uiout, char *identstring);
 static void cli_flush (struct ui_out *uiout);
 static int cli_redirect (struct ui_out *uiout, struct ui_file *outstream);
@@ -95,7 +95,7 @@ static void field_separator (void);
 
 static void out_field_fmt (struct ui_out *uiout, int fldno,
 			   const char *fldname,
-			   const char *format,...) ATTR_FORMAT (printf, 4, 5);
+			   const char *format,...) ATTRIBUTE_PRINTF (4, 5);
 
 /* Mark beginning of a table */
 
--- a/gdb/complaints.c
+++ b/gdb/complaints.c
@@ -120,7 +120,7 @@ get_complaints (struct complaints **c)
   return (*c);
 }
 
-static struct complain *
+static struct complain * ATTRIBUTE_PRINTF (4, 0)
 find_complaint (struct complaints *complaints, const char *file,
 		int line, const char *fmt)
 {
@@ -164,7 +164,7 @@ static int stop_whining = 0;
 /* Print a complaint, and link the complaint block into a chain for
    later handling.  */
 
-static void ATTR_FORMAT (printf, 4, 0)
+static void ATTRIBUTE_PRINTF (4, 0)
 vcomplaint (struct complaints **c, const char *file, int line, const char *fmt,
 	    va_list args)
 {
--- a/gdb/complaints.h
+++ b/gdb/complaints.h
@@ -31,10 +31,10 @@ extern struct complaints *symfile_complaints;
 
 /* Register a complaint.  */
 extern void complaint (struct complaints **complaints, const char *fmt,
-		       ...) ATTR_FORMAT (printf, 2, 3);
+		       ...) ATTRIBUTE_PRINTF (2, 3);
 extern void internal_complaint (struct complaints **complaints,
 				const char *file, int line, const char *fmt,
-				...) ATTR_FORMAT (printf, 4, 5);
+				...) ATTRIBUTE_PRINTF (4, 5);
 
 /* Clear out / initialize all complaint counters that have ever been
    incremented.  If LESS_VERBOSE is 1, be less verbose about
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -1,4 +1,5 @@
-/* *INDENT-OFF* */ /* ATTR_FORMAT confuses indent, avoid running it for now */
+/* *INDENT-OFF* */ /* ATTRIBUTE_PRINTF confuses indent, avoid running it
+		      for now.  */
 /* Basic, host-specific, and target-specific definitions for GDB.
    Copyright (C) 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996,
    1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009,
@@ -296,14 +297,6 @@ struct cleanup
 #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
-
 /* Be conservative and use enum bitfields only with GCC.
    This is copied from gcc 3.3.1, system.h.  */
 
@@ -404,9 +397,9 @@ extern void null_cleanup (void *);
 
 extern int myread (int, char *, int);
 
-extern int query (const char *, ...) ATTR_FORMAT (printf, 1, 2);
-extern int nquery (const char *, ...) ATTR_FORMAT (printf, 1, 2);
-extern int yquery (const char *, ...) ATTR_FORMAT (printf, 1, 2);
+extern int query (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
+extern int nquery (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
+extern int yquery (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
 extern void init_page_info (void);
 
@@ -492,25 +485,25 @@ extern void puts_filtered_tabular (char *string, int width, int right);
 
 extern void puts_debug (char *prefix, char *string, char *suffix);
 
-extern void vprintf_filtered (const char *, va_list) ATTR_FORMAT (printf, 1, 0);
+extern void vprintf_filtered (const char *, va_list) ATTRIBUTE_PRINTF (1, 0);
 
-extern void vfprintf_filtered (struct ui_file *, const char *, va_list) ATTR_FORMAT (printf, 2, 0);
+extern void vfprintf_filtered (struct ui_file *, const char *, va_list) ATTRIBUTE_PRINTF (2, 0);
 
-extern void fprintf_filtered (struct ui_file *, const char *, ...) ATTR_FORMAT (printf, 2, 3);
+extern void fprintf_filtered (struct ui_file *, const char *, ...) ATTRIBUTE_PRINTF (2, 3);
 
-extern void fprintfi_filtered (int, struct ui_file *, const char *, ...) ATTR_FORMAT (printf, 3, 4);
+extern void fprintfi_filtered (int, struct ui_file *, const char *, ...) ATTRIBUTE_PRINTF (3, 4);
 
-extern void printf_filtered (const char *, ...) ATTR_FORMAT (printf, 1, 2);
+extern void printf_filtered (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
-extern void printfi_filtered (int, const char *, ...) ATTR_FORMAT (printf, 2, 3);
+extern void printfi_filtered (int, const char *, ...) ATTRIBUTE_PRINTF (2, 3);
 
-extern void vprintf_unfiltered (const char *, va_list) ATTR_FORMAT (printf, 1, 0);
+extern void vprintf_unfiltered (const char *, va_list) ATTRIBUTE_PRINTF (1, 0);
 
-extern void vfprintf_unfiltered (struct ui_file *, const char *, va_list) ATTR_FORMAT (printf, 2, 0);
+extern void vfprintf_unfiltered (struct ui_file *, const char *, va_list) ATTRIBUTE_PRINTF (2, 0);
 
-extern void fprintf_unfiltered (struct ui_file *, const char *, ...) ATTR_FORMAT (printf, 2, 3);
+extern void fprintf_unfiltered (struct ui_file *, const char *, ...) ATTRIBUTE_PRINTF (2, 3);
 
-extern void printf_unfiltered (const char *, ...) ATTR_FORMAT (printf, 1, 2);
+extern void printf_unfiltered (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
 extern void print_spaces (int, struct ui_file *);
 
@@ -900,19 +893,19 @@ extern void *xzalloc (size_t);
 
 /* Like asprintf/vasprintf but get an internal_error if the call
    fails. */
-extern void xasprintf (char **ret, const char *format, ...) ATTR_FORMAT (printf, 2, 3);
+extern void xasprintf (char **ret, const char *format, ...) ATTRIBUTE_PRINTF (2, 3);
 extern void xvasprintf (char **ret, const char *format, va_list ap)
-     ATTR_FORMAT (printf, 2, 0);
+     ATTRIBUTE_PRINTF (2, 0);
 
 /* Like asprintf and vasprintf, but return the string, throw an error
    if no memory.  */
-extern char *xstrprintf (const char *format, ...) ATTR_FORMAT (printf, 1, 2);
+extern char *xstrprintf (const char *format, ...) ATTRIBUTE_PRINTF (1, 2);
 extern char *xstrvprintf (const char *format, va_list ap)
-     ATTR_FORMAT (printf, 1, 0);
+     ATTRIBUTE_PRINTF (1, 0);
 
 /* Like snprintf, but throw an error if the output buffer is too small.  */
 extern int xsnprintf (char *str, size_t size, const char *format, ...)
-     ATTR_FORMAT (printf, 3, 4);
+     ATTRIBUTE_PRINTF (3, 4);
 
 extern int parse_escape (struct gdbarch *, char **);
 
@@ -929,36 +922,36 @@ extern char *quit_pre_print;
 extern char *warning_pre_print;
 
 extern NORETURN void verror (const char *fmt, va_list ap)
-     ATTR_NORETURN ATTR_FORMAT (printf, 1, 0);
+     ATTR_NORETURN ATTRIBUTE_PRINTF (1, 0);
 
-extern NORETURN void error (const char *fmt, ...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
+extern NORETURN void error (const char *fmt, ...) ATTR_NORETURN ATTRIBUTE_PRINTF (1, 2);
 
 extern NORETURN void error_stream (struct ui_file *) ATTR_NORETURN;
 
 extern NORETURN void vfatal (const char *fmt, va_list ap)
-     ATTR_NORETURN ATTR_FORMAT (printf, 1, 0);
+     ATTR_NORETURN ATTRIBUTE_PRINTF (1, 0);
 
-extern NORETURN void fatal (const char *fmt, ...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
+extern NORETURN void fatal (const char *fmt, ...) ATTR_NORETURN ATTRIBUTE_PRINTF (1, 2);
 
 extern NORETURN void internal_verror (const char *file, int line,
 				      const char *, va_list ap)
-     ATTR_NORETURN ATTR_FORMAT (printf, 3, 0);
+     ATTR_NORETURN ATTRIBUTE_PRINTF (3, 0);
 
 extern NORETURN void internal_error (const char *file, int line,
-				     const char *, ...) ATTR_NORETURN ATTR_FORMAT (printf, 3, 4);
+				     const char *, ...) ATTR_NORETURN ATTRIBUTE_PRINTF (3, 4);
 
 extern void internal_vwarning (const char *file, int line,
 			       const char *, va_list ap)
-     ATTR_FORMAT (printf, 3, 0);
+     ATTRIBUTE_PRINTF (3, 0);
 
 extern void internal_warning (const char *file, int line,
-			      const char *, ...) ATTR_FORMAT (printf, 3, 4);
+			      const char *, ...) ATTRIBUTE_PRINTF (3, 4);
 
 extern NORETURN void nomem (long) ATTR_NORETURN;
 
-extern void warning (const char *, ...) ATTR_FORMAT (printf, 1, 2);
+extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
-extern void vwarning (const char *, va_list args) ATTR_FORMAT (printf, 1, 0);
+extern void vwarning (const char *, va_list args) ATTRIBUTE_PRINTF (1, 0);
 
 /* List of known OS ABIs.  If you change this, make sure to update the
    table in osabi.c.  */
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -335,7 +335,7 @@ do_assembly_only (struct gdbarch *gdbarch, struct ui_out *uiout,
 /* Initialize the disassemble info struct ready for the specified
    stream.  */
 
-static int ATTR_FORMAT (printf, 2, 3)
+static int ATTRIBUTE_PRINTF (2, 3)
 fprintf_disasm (void *stream, const char *format, ...)
 {
   va_list args;
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -374,7 +374,7 @@ print_any_exception (struct ui_file *file, const char *prefix,
     }
 }
 
-NORETURN static void ATTR_NORETURN ATTR_FORMAT (printf, 3, 0)
+NORETURN static void ATTR_NORETURN ATTRIBUTE_PRINTF (3, 0)
 throw_it (enum return_reason reason, enum errors error, const char *fmt,
 	  va_list ap)
 {
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -153,7 +153,7 @@ int exceptions_state_mc_action_iter_1 (void);
 extern void exception_print (struct ui_file *file, struct gdb_exception e);
 extern void exception_fprintf (struct ui_file *file, struct gdb_exception e,
 			       const char *prefix,
-			       ...) ATTR_FORMAT (printf, 3, 4);
+			       ...) ATTRIBUTE_PRINTF (3, 4);
 
 /* Throw an exception (as described by "struct gdb_exception").  Will
    execute a LONG JUMP to the inner most containing exception handler
@@ -168,11 +168,11 @@ extern void exception_fprintf (struct ui_file *file, struct gdb_exception e,
 
 extern NORETURN void throw_exception (struct gdb_exception exception) ATTR_NORETURN;
 extern NORETURN void throw_verror (enum errors, const char *fmt, va_list ap)
-     ATTR_NORETURN ATTR_FORMAT (printf, 2, 0);
+     ATTR_NORETURN ATTRIBUTE_PRINTF (2, 0);
 extern NORETURN void throw_vfatal (const char *fmt, va_list ap)
-     ATTR_NORETURN ATTR_FORMAT (printf, 1, 0);
+     ATTR_NORETURN ATTRIBUTE_PRINTF (1, 0);
 extern NORETURN void throw_error (enum errors error, const char *fmt,
-				  ...) ATTR_NORETURN ATTR_FORMAT (printf, 2, 3);
+				  ...) ATTR_NORETURN ATTRIBUTE_PRINTF (2, 3);
 
 /* Instead of deprecated_throw_reason, code should use catch_exception
    and throw_exception.  */
--- 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 */
 
 #ifndef ATTR_MALLOC
 #if defined(__GNUC__) && (__GNUC__ >= 3)
@@ -444,7 +453,7 @@ char* buffer_finish (struct buffer *buffer);
 /* Simple printf to BUFFER function.  Current implemented formatters:
    %s - grow an xml escaped text in OBSTACK.  */
 void buffer_xml_printf (struct buffer *buffer, const char *format, ...)
-  ATTR_FORMAT (printf, 2, 3);
+  ATTRIBUTE_PRINTF (2, 3);
 
 #define buffer_grow_str(BUFFER,STRING)         \
   buffer_grow (BUFFER, STRING, strlen (STRING))
@@ -459,11 +468,11 @@ void *xcalloc (size_t, size_t) ATTR_MALLOC;
 char *xstrdup (const char *) ATTR_MALLOC;
 void freeargv (char **argv);
 void perror_with_name (const char *string);
-void error (const char *string,...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
-void fatal (const char *string,...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
+void error (const char *string,...) ATTR_NORETURN ATTRIBUTE_PRINTF (1, 2);
+void fatal (const char *string,...) ATTR_NORETURN ATTRIBUTE_PRINTF (1, 2);
 void internal_error (const char *file, int line, const char *, ...)
-     ATTR_NORETURN ATTR_FORMAT (printf, 3, 4);
-void warning (const char *string,...) ATTR_FORMAT (printf, 1, 2);
+     ATTR_NORETURN ATTRIBUTE_PRINTF (3, 4);
+void warning (const char *string,...) ATTRIBUTE_PRINTF (1, 2);
 char *paddress (CORE_ADDR addr);
 char *pulongest (ULONGEST u);
 char *plongest (LONGEST l);
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -22,7 +22,7 @@
 #include <unistd.h>
 #include <sys/time.h>
 
-static void trace_debug_1 (const char *, ...) ATTR_FORMAT (printf, 1, 2);
+static void trace_debug_1 (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
 static void
 trace_debug_1 (const char *fmt, ...)
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -461,9 +461,9 @@ extern void binop_type_check (struct value *, struct value *, int);
 
 /* Error messages */
 
-extern void type_error (const char *, ...) ATTR_FORMAT (printf, 1, 2);
+extern void type_error (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
-extern void range_error (const char *, ...) ATTR_FORMAT (printf, 1, 2);
+extern void range_error (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
 /* Data:  Does this value represent "truth" to the current language?  */
 
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -80,7 +80,7 @@ static struct symtabs_and_lines find_method (int funfirstline,
 
 static NORETURN void cplusplus_error (const char *name,
 				      const char *fmt, ...)
-     ATTR_NORETURN ATTR_FORMAT (printf, 2, 3);
+     ATTR_NORETURN ATTRIBUTE_PRINTF (2, 3);
 
 static int total_number_of_methods (struct type *type);
 
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -44,7 +44,7 @@ static void mi_command_loop (int mi_version);
    so we can report interesting things that happened "behind the mi's
    back" in this command */
 static int mi_interp_query_hook (const char *ctlstr, va_list ap)
-     ATTR_FORMAT (printf, 1, 0);
+     ATTRIBUTE_PRINTF (1, 0);
 
 static void mi3_command_loop (void);
 static void mi2_command_loop (void);
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -55,12 +55,12 @@ static void mi_field_string (struct ui_out *uiout, int fldno, int width,
 static void mi_field_fmt (struct ui_out *uiout, int fldno,
 			  int width, enum ui_align align,
 			  const char *fldname, const char *format,
-			  va_list args) ATTR_FORMAT (printf, 6, 0);
+			  va_list args) ATTRIBUTE_PRINTF (6, 0);
 static void mi_spaces (struct ui_out *uiout, int numspaces);
 static void mi_text (struct ui_out *uiout, const char *string);
 static void mi_message (struct ui_out *uiout, int verbosity,
 			const char *format, va_list args)
-     ATTR_FORMAT (printf, 3, 0);
+     ATTRIBUTE_PRINTF (3, 0);
 static void mi_wrap_hint (struct ui_out *uiout, char *identstring);
 static void mi_flush (struct ui_out *uiout);
 
--- a/gdb/monitor.c
+++ b/gdb/monitor.c
@@ -119,7 +119,7 @@ static ptid_t monitor_ptid;
 /* Monitor specific debugging information.  Typically only useful to
    the developer of a new monitor interface. */
 
-static void monitor_debug (const char *fmt, ...) ATTR_FORMAT(printf, 1, 2);
+static void monitor_debug (const char *fmt, ...) ATTRIBUTE_PRINTF (1, 2);
 
 static int monitor_debug_p = 0;
 
--- a/gdb/parser-defs.h
+++ b/gdb/parser-defs.h
@@ -318,7 +318,7 @@ extern void print_subexp_standard (struct expression *, int *,
 /* Function used to avoid direct calls to fprintf
    in the code generated by the bison parser.  */
 
-extern void parser_fprintf (FILE *, const char *, ...) ATTR_FORMAT (printf, 2 ,3);
+extern void parser_fprintf (FILE *, const char *, ...) ATTRIBUTE_PRINTF (2, 3);
 
 extern int exp_uses_objfile (struct expression *exp, struct objfile *objfile);
 
--- a/gdb/serial.h
+++ b/gdb/serial.h
@@ -87,7 +87,7 @@ extern int serial_write (struct serial *scb, const char *str, int len);
 /* Write a printf style string onto the serial port.  */
 
 extern void serial_printf (struct serial *desc, 
-			   const char *,...) ATTR_FORMAT (printf, 2, 3);
+			   const char *,...) ATTRIBUTE_PRINTF (2, 3);
 
 /* Allow pending output to drain.  */
 
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -64,7 +64,7 @@ tui_new_objfile_hook (struct objfile* objfile)
     tui_display_main ();
 }
 
-static int ATTR_FORMAT (printf, 1, 0)
+static int ATTRIBUTE_PRINTF (1, 0)
 tui_query_hook (const char *msg, va_list argp)
 {
   int retval;
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -177,12 +177,12 @@ static void default_field_fmt (struct ui_out *uiout, int fldno,
 			       int width, enum ui_align align,
 			       const char *fldname,
 			       const char *format,
-			       va_list args) ATTR_FORMAT (printf, 6, 0);
+			       va_list args) ATTRIBUTE_PRINTF (6, 0);
 static void default_spaces (struct ui_out *uiout, int numspaces);
 static void default_text (struct ui_out *uiout, const char *string);
 static void default_message (struct ui_out *uiout, int verbosity,
 			     const char *format,
-			     va_list args) ATTR_FORMAT (printf, 3, 0);
+			     va_list args) ATTRIBUTE_PRINTF (3, 0);
 static void default_wrap_hint (struct ui_out *uiout, char *identstring);
 static void default_flush (struct ui_out *uiout);
 
@@ -245,12 +245,12 @@ static void uo_field_skip (struct ui_out *uiout, int fldno, int width,
 static void uo_field_fmt (struct ui_out *uiout, int fldno, int width,
 			  enum ui_align align, const char *fldname,
 			  const char *format, va_list args)
-     ATTR_FORMAT (printf, 6, 0);
+     ATTRIBUTE_PRINTF (6, 0);
 static void uo_spaces (struct ui_out *uiout, int numspaces);
 static void uo_text (struct ui_out *uiout, const char *string);
 static void uo_message (struct ui_out *uiout, int verbosity,
 			const char *format, va_list args)
-     ATTR_FORMAT (printf, 3, 0);
+     ATTRIBUTE_PRINTF (3, 0);
 static void uo_wrap_hint (struct ui_out *uiout, char *identstring);
 static void uo_flush (struct ui_out *uiout);
 static int uo_redirect (struct ui_out *uiout, struct ui_file *outstream);
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -124,7 +124,7 @@ extern void ui_out_field_stream (struct ui_out *uiout, const char *fldname,
 
 extern void ui_out_field_fmt (struct ui_out *uiout, const char *fldname,
 			      const char *format, ...)
-     ATTR_FORMAT (printf, 3, 4);
+     ATTRIBUTE_PRINTF (3, 4);
 
 extern void ui_out_field_skip (struct ui_out *uiout, const char *fldname);
 
@@ -134,7 +134,7 @@ extern void ui_out_text (struct ui_out *uiout, const char *string);
 
 extern void ui_out_message (struct ui_out *uiout, int verbosity,
 			    const char *format, ...)
-     ATTR_FORMAT (printf, 3, 4);
+     ATTRIBUTE_PRINTF (3, 4);
 
 extern struct ui_stream *ui_out_stream_new (struct ui_out *uiout);
 
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -92,7 +92,7 @@ void (*deprecated_error_begin_hook) (void);
 /* Prototypes for local functions */
 
 static void vfprintf_maybe_filtered (struct ui_file *, const char *,
-				     va_list, int) ATTR_FORMAT (printf, 2, 0);
+				     va_list, int) ATTRIBUTE_PRINTF (2, 0);
 
 static void fputs_maybe_filtered (const char *, struct ui_file *, int);
 
@@ -916,7 +916,7 @@ struct internal_problem
    has been reported, and assuming GDB didn't quit, the caller can
    either allow execution to resume or throw an error.  */
 
-static void ATTR_FORMAT (printf, 4, 0)
+static void ATTRIBUTE_PRINTF (4, 0)
 internal_vproblem (struct internal_problem *problem,
 		   const char *file, int line, const char *fmt, va_list ap)
 {
@@ -1452,7 +1452,7 @@ gdb_print_host_address (const void *addr, struct ui_file *stream)
    ARGS are the arguments passed along with the CTLSTR argument to
    printf.  */
 
-static int ATTR_FORMAT (printf, 1, 0)
+static int ATTRIBUTE_PRINTF (1, 0)
 defaulted_query (const char *ctlstr, const char defchar, va_list args)
 {
   int answer;
--- a/gdb/xml-support.h
+++ b/gdb/xml-support.h
@@ -199,13 +199,13 @@ int gdb_xml_parse (struct gdb_xml_parser *parser, const char *buffer);
 /* Issue a debugging message from one of PARSER's handlers.  */
 
 void gdb_xml_debug (struct gdb_xml_parser *parser, const char *format, ...)
-     ATTR_FORMAT (printf, 2, 0);
+     ATTRIBUTE_PRINTF (2, 0);
 
 /* Issue an error message from one of PARSER's handlers, and stop
    parsing.  */
 
 void gdb_xml_error (struct gdb_xml_parser *parser, const char *format, ...)
-     ATTR_NORETURN ATTR_FORMAT (printf, 2, 0);
+     ATTR_NORETURN ATTRIBUTE_PRINTF (2, 0);
 
 /* Parse an integer attribute into a ULONGEST.  */
 

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

[Pedro Alves]

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

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

[Jan Kratochvil]

On Sun, 02 May 2010 17:33:14 +0200, Pedro Alves wrote:
> The GDB part looks fine to me.  Want to commit it separatelly to the
> GDBserver bit?

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-05/msg00011.html

Reintroduced back cli-out.c:cli_field_fmt's attr-format you have dropped by
`cli-out.c cleanup' today.


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

Yes, please.  BTW I do not understand why gdb/gdbserver/ cannot use the common
infrastructure when even the doc states:
	In fact, a system that can run `gdbserver' to connect to a remote GDB
	could also run GDB locally!
I believe gdbserver/ provides less resources + features requiring code than
libiberty/ .


I will post some similar NORETURN + ATTR_NORETURN -> ATTRIBUTE_NORETURN.

Going to drop NORETURN as it is only for gcc < 2.7, ansidecl.h does not
support it and a missing noreturn flag can only affect code size, performance
and affect compilation diagnostics.


Thanks,
Jan

[patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]

[Jan Kratochvil]

On Sun, 02 May 2010 23:36:36 +0200, Jan Kratochvil wrote:
> I will post some similar NORETURN + ATTR_NORETURN -> ATTRIBUTE_NORETURN.
> 
> Going to drop NORETURN as it is only for gcc < 2.7, ansidecl.h does not
> support it and a missing noreturn flag can only affect code size, performance
> and affect compilation diagnostics.

error_no_arg was present both in command.h and cli/cli-cmds.h.  Moving it the
other way would require #include changes.

mips_error will start to be effectively declared as noreturn.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.

Similar work is needed in gdb/gdbserver/.

doc/ talks about .mh files; but when similar macros are already used from
include/ for bfd/ etc. it probably was inappropriate to do from .mh.


Thanks,
Jan


gdb/
2010-05-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* cli/cli-cmds.h (error_no_arg): Remove.  Move the comment ...
	* command.h (error_no_arg): ... here.  Remove NORETURN, change
	ATTR_NORETURN to ATTRIBUTE_NORETURN.
	* defs.h (NORETURN, ATTR_NORETURN): Remove.
	(perror_with_name, verror, error, error_stream, vfatal, fatal)
	(internal_verror, internal_error, nomem): Remove NORETURN, change
	ATTR_NORETURN to ATTRIBUTE_NORETURN.
	* exceptions.c (throw_exception, deprecated_throw_reason, throw_verror)
	(throw_vfatal, throw_error): Remove NORETURN.
	(throw_it): Remove NORETURN, change ATTR_NORETURN to ATTRIBUTE_NORETURN.
	* exceptions.h (throw_exception, throw_verror, throw_vfatal)
	(throw_error, deprecated_throw_reason): Remove NORETURN, change
	ATTR_NORETURN to ATTRIBUTE_NORETURN.
	* linespec.c (cplusplus_error): Remove NORETURN, change ATTR_NORETURN
	to ATTRIBUTE_NORETURN for prototype, for the definition only remove
	NORETURN.
	* remote-mips.c (mips_error): Change NORETURN to ATTRIBUTE_NORETURN.
	* remote-sim.c (gdb_os_error): Change ATTR_NORETURN to
	ATTRIBUTE_NORETURN.
	* target.c (tcomplain): Likewise.
	* target.h (noprocess): Remove NORETURN, change ATTR_NORETURN to
	ATTRIBUTE_NORETURN.
	* utils.c (verror, error, vfatal, fatal, error_stream, internal_verror)
	(internal_error, perror_with_name, nomem): Remove NORETURN.
	* xml-support.h (gdb_xml_error): Change ATTR_NORETURN to
	ATTRIBUTE_NORETURN.

doc/
2010-05-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdbint.texinfo (Host Definition): Remove items NORETURN and
	ATTR_NORETURN.

--- a/gdb/cli/cli-cmds.h
+++ b/gdb/cli/cli-cmds.h
@@ -128,11 +128,6 @@ extern void source_script (char *, int);
 extern int find_and_open_script (const char *file, int search_path,
 				 FILE **streamp, char **full_path);
 
-/* Used everywhere whenever at least one parameter is required and
-  none is specified. */
-
-extern NORETURN void error_no_arg (char *) ATTR_NORETURN;
-
 /* Command tracing state.  */
 
 extern int source_verbose;
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -341,7 +341,10 @@ extern void add_setshow_zuinteger_cmd (char *name,
 
 extern void cmd_show_list (struct cmd_list_element *, int, char *);
 
-extern NORETURN void error_no_arg (char *) ATTR_NORETURN;
+/* Used everywhere whenever at least one parameter is required and
+   none is specified. */
+
+extern void error_no_arg (char *) ATTRIBUTE_NORETURN;
 
 extern void dont_repeat (void);
 
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -267,36 +267,6 @@ struct cleanup
     void *arg;
   };
 
-
-/* The ability to declare that a function never returns is useful, but
-   not really required to compile GDB successfully, so the NORETURN and
-   ATTR_NORETURN macros normally expand into nothing.  */
-
-/* If compiling with older versions of GCC, a function may be declared
-   "volatile" to indicate that it does not return.  */
-
-#ifndef NORETURN
-#if defined(__GNUC__) \
-     && (__GNUC__ == 1 || (__GNUC__ == 2 && __GNUC_MINOR__ < 7))
-#define NORETURN volatile
-#else
-#define NORETURN		/* nothing */
-#endif
-#endif
-
-/* GCC 2.5 and later versions define a function attribute "noreturn",
-   which is the preferred way to declare that a function never returns.
-   However GCC 2.7 appears to be the first version in which this fully
-   works everywhere we use it. */
-
-#ifndef ATTR_NORETURN
-#if defined(__GNUC__) && (__GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 7))
-#define ATTR_NORETURN __attribute__ ((noreturn))
-#else
-#define ATTR_NORETURN		/* nothing */
-#endif
-#endif
-
 /* Be conservative and use enum bitfields only with GCC.
    This is copied from gcc 3.3.1, system.h.  */
 
@@ -552,7 +522,7 @@ extern char *hex_string_custom (LONGEST, int);
 extern void fprintf_symbol_filtered (struct ui_file *, char *,
 				     enum language, int);
 
-extern NORETURN void perror_with_name (const char *) ATTR_NORETURN;
+extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
 
 extern void print_sys_errmsg (const char *, int);
 
@@ -921,24 +891,26 @@ extern char *quit_pre_print;
 
 extern char *warning_pre_print;
 
-extern NORETURN void verror (const char *fmt, va_list ap)
-     ATTR_NORETURN ATTRIBUTE_PRINTF (1, 0);
+extern void verror (const char *fmt, va_list ap)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
 
-extern NORETURN void error (const char *fmt, ...) ATTR_NORETURN ATTRIBUTE_PRINTF (1, 2);
+extern void error (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
 
-extern NORETURN void error_stream (struct ui_file *) ATTR_NORETURN;
+extern void error_stream (struct ui_file *) ATTRIBUTE_NORETURN;
 
-extern NORETURN void vfatal (const char *fmt, va_list ap)
-     ATTR_NORETURN ATTRIBUTE_PRINTF (1, 0);
+extern void vfatal (const char *fmt, va_list ap)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
 
-extern NORETURN void fatal (const char *fmt, ...) ATTR_NORETURN ATTRIBUTE_PRINTF (1, 2);
+extern void fatal (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
 
-extern NORETURN void internal_verror (const char *file, int line,
-				      const char *, va_list ap)
-     ATTR_NORETURN ATTRIBUTE_PRINTF (3, 0);
+extern void internal_verror (const char *file, int line, const char *,
+			     va_list ap)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
 
-extern NORETURN void internal_error (const char *file, int line,
-				     const char *, ...) ATTR_NORETURN ATTRIBUTE_PRINTF (3, 4);
+extern void internal_error (const char *file, int line, const char *, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
 
 extern void internal_vwarning (const char *file, int line,
 			       const char *, va_list ap)
@@ -947,7 +919,7 @@ extern void internal_vwarning (const char *file, int line,
 extern void internal_warning (const char *file, int line,
 			      const char *, ...) ATTRIBUTE_PRINTF (3, 4);
 
-extern NORETURN void nomem (long) ATTR_NORETURN;
+extern void nomem (long) ATTRIBUTE_NORETURN;
 
 extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
--- a/gdb/doc/gdbint.texinfo
+++ b/gdb/doc/gdbint.texinfo
@@ -2784,19 +2784,6 @@ Define this if @code{lseek (n)} does not necessarily move to byte number
 @code{n} in the file.  This is only used when reading source files.  It
 is normally faster to define @code{CRLF_SOURCE_FILES} when possible.
 
-@item NORETURN
-If defined, this should be one or more tokens, such as @code{volatile},
-that can be used in both the declaration and definition of functions to
-indicate that they never return.  The default is already set correctly
-if compiling with GCC.  This will almost never need to be defined.
-
-@item ATTR_NORETURN
-If defined, this should be one or more tokens, such as
-@code{__attribute__ ((noreturn))}, that can be used in the declarations
-of functions to indicate that they never return.  The default is already
-set correctly if compiling with GCC.  This will almost never need to be
-defined.
-
 @item lint
 Define this to help placate @code{lint} in some situations.
 
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -210,7 +210,7 @@ exceptions_state_mc_action_iter_1 (void)
 
 /* Return EXCEPTION to the nearest containing catch_errors().  */
 
-NORETURN void
+void
 throw_exception (struct gdb_exception exception)
 {
   struct thread_info *tp = NULL;
@@ -239,7 +239,7 @@ throw_exception (struct gdb_exception exception)
 
 static char *last_message;
 
-NORETURN void
+void
 deprecated_throw_reason (enum return_reason reason)
 {
   struct gdb_exception exception;
@@ -374,7 +374,7 @@ print_any_exception (struct ui_file *file, const char *prefix,
     }
 }
 
-NORETURN static void ATTR_NORETURN ATTRIBUTE_PRINTF (3, 0)
+static void ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0)
 throw_it (enum return_reason reason, enum errors error, const char *fmt,
 	  va_list ap)
 {
@@ -396,19 +396,19 @@ throw_it (enum return_reason reason, enum errors error, const char *fmt,
   throw_exception (e);
 }
 
-NORETURN void
+void
 throw_verror (enum errors error, const char *fmt, va_list ap)
 {
   throw_it (RETURN_ERROR, error, fmt, ap);
 }
 
-NORETURN void
+void
 throw_vfatal (const char *fmt, va_list ap)
 {
   throw_it (RETURN_QUIT, GDB_NO_ERROR, fmt, ap);
 }
 
-NORETURN void
+void
 throw_error (enum errors error, const char *fmt, ...)
 {
   va_list args;
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -166,17 +166,18 @@ extern void exception_fprintf (struct ui_file *file, struct gdb_exception e,
    be a good thing or a dangerous thing.'' -- the Existential
    Wombat.  */
 
-extern NORETURN void throw_exception (struct gdb_exception exception) ATTR_NORETURN;
-extern NORETURN void throw_verror (enum errors, const char *fmt, va_list ap)
-     ATTR_NORETURN ATTRIBUTE_PRINTF (2, 0);
-extern NORETURN void throw_vfatal (const char *fmt, va_list ap)
-     ATTR_NORETURN ATTRIBUTE_PRINTF (1, 0);
-extern NORETURN void throw_error (enum errors error, const char *fmt,
-				  ...) ATTR_NORETURN ATTRIBUTE_PRINTF (2, 3);
+extern void throw_exception (struct gdb_exception exception) ATTRIBUTE_NORETURN;
+extern void throw_verror (enum errors, const char *fmt, va_list ap)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 0);
+extern void throw_vfatal (const char *fmt, va_list ap)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
+extern void throw_error (enum errors error, const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 3);
 
 /* Instead of deprecated_throw_reason, code should use catch_exception
    and throw_exception.  */
-extern NORETURN void deprecated_throw_reason (enum return_reason reason) ATTR_NORETURN;
+extern void deprecated_throw_reason (enum return_reason reason)
+     ATTRIBUTE_NORETURN;
 
 /* Call FUNC(UIOUT, FUNC_ARGS) but wrapped within an exception
    handler.  If an exception (enum return_reason) is thrown using
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -78,9 +78,8 @@ static struct symtabs_and_lines find_method (int funfirstline,
 					     struct symbol *sym_class,
 					     int *not_found_ptr);
 
-static NORETURN void cplusplus_error (const char *name,
-				      const char *fmt, ...)
-     ATTR_NORETURN ATTRIBUTE_PRINTF (2, 3);
+static void cplusplus_error (const char *name, const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 3);
 
 static int total_number_of_methods (struct type *type);
 
@@ -146,7 +145,7 @@ symtabs_and_lines minsym_found (int funfirstline,
    single quoted demangled C++ symbols as part of the completion
    error.  */
 
-static NORETURN void
+static void
 cplusplus_error (const char *name, const char *fmt, ...)
 {
   struct ui_file *tmp_stream;
--- a/gdb/remote-mips.c
+++ b/gdb/remote-mips.c
@@ -473,7 +473,7 @@ close_ports (void)
    all hell to break loose--the rest of GDB will tend to get left in an
    inconsistent state.  */
 
-static NORETURN void
+static void ATTRIBUTE_NORETURN
 mips_error (char *string,...)
 {
   va_list args;
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -70,7 +70,8 @@ static void gdb_os_vprintf_filtered (host_callback *, const char *, va_list);
 
 static void gdb_os_evprintf_filtered (host_callback *, const char *, va_list);
 
-static void gdb_os_error (host_callback *, const char *, ...) ATTR_NORETURN;
+static void gdb_os_error (host_callback *, const char *, ...)
+     ATTRIBUTE_NORETURN;
 
 static void gdbsim_kill (struct target_ops *);
 
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -56,7 +56,7 @@ static int default_region_ok_for_hw_watchpoint (CORE_ADDR, int);
 
 static int nosymbol (char *, CORE_ADDR *);
 
-static void tcomplain (void) ATTR_NORETURN;
+static void tcomplain (void) ATTRIBUTE_NORETURN;
 
 static int nomemory (CORE_ADDR, char *, int, int, struct target_ops *);
 
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1485,7 +1485,7 @@ extern int default_memory_insert_breakpoint (struct gdbarch *, struct bp_target_
 
 extern void initialize_targets (void);
 
-extern NORETURN void noprocess (void) ATTR_NORETURN;
+extern void noprocess (void) ATTRIBUTE_NORETURN;
 
 extern void target_require_runnable (void);
 
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -806,13 +806,13 @@ warning (const char *string, ...)
    The first argument STRING is the error message, used as a fprintf string,
    and the remaining args are passed as arguments to it.  */
 
-NORETURN void
+void
 verror (const char *string, va_list args)
 {
   throw_verror (GENERIC_ERROR, string, args);
 }
 
-NORETURN void
+void
 error (const char *string, ...)
 {
   va_list args;
@@ -825,13 +825,13 @@ error (const char *string, ...)
    The first argument STRING is the error message, used as a fprintf string,
    and the remaining args are passed as arguments to it.  */
 
-NORETURN void
+void
 vfatal (const char *string, va_list args)
 {
   throw_vfatal (string, args);
 }
 
-NORETURN void
+void
 fatal (const char *string, ...)
 {
   va_list args;
@@ -840,7 +840,7 @@ fatal (const char *string, ...)
   va_end (args);
 }
 
-NORETURN void
+void
 error_stream (struct ui_file *stream)
 {
   char *message = ui_file_xstrdup (stream, NULL);
@@ -1036,14 +1036,14 @@ static struct internal_problem internal_error_problem = {
   "internal-error", internal_problem_ask, internal_problem_ask
 };
 
-NORETURN void
+void
 internal_verror (const char *file, int line, const char *fmt, va_list ap)
 {
   internal_vproblem (&internal_error_problem, file, line, fmt, ap);
   deprecated_throw_reason (RETURN_ERROR);
 }
 
-NORETURN void
+void
 internal_error (const char *file, int line, const char *string, ...)
 {
   va_list ap;
@@ -1174,7 +1174,7 @@ Show whether GDB will create a core file of GDB when %s is detected"),
    as the file name for which the error was encountered.
    Then return to command level.  */
 
-NORETURN void
+void
 perror_with_name (const char *string)
 {
   char *err;
@@ -1240,7 +1240,7 @@ quit (void)
 /* Called when a memory allocation fails, with the number of bytes of
    memory requested in SIZE. */
 
-NORETURN void
+void
 nomem (long size)
 {
   if (size > 0)
--- a/gdb/xml-support.h
+++ b/gdb/xml-support.h
@@ -205,7 +205,7 @@ void gdb_xml_debug (struct gdb_xml_parser *parser, const char *format, ...)
    parsing.  */
 
 void gdb_xml_error (struct gdb_xml_parser *parser, const char *format, ...)
-     ATTR_NORETURN ATTRIBUTE_PRINTF (2, 0);
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 0);
 
 /* Parse an integer attribute into a ULONGEST.  */
 

Re: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]

[Pedro Alves]

On Monday 03 May 2010 00:19:41, Jan Kratochvil wrote:
> On Sun, 02 May 2010 23:36:36 +0200, Jan Kratochvil wrote:
> > I will post some similar NORETURN + ATTR_NORETURN -> ATTRIBUTE_NORETURN.
> > 
> > Going to drop NORETURN as it is only for gcc < 2.7, ansidecl.h does not
> > support it and a missing noreturn flag can only affect code size, performance
> > and affect compilation diagnostics.
> 
> error_no_arg was present both in command.h and cli/cli-cmds.h.  Moving it the
> other way would require #include changes.
> 
> mips_error will start to be effectively declared as noreturn.
> 
> No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.
> 

This is okay.  Thanks for doing this.

> Similar work is needed in gdb/gdbserver/.
> 
> doc/ talks about .mh files; but when similar macros are already used from
> include/ for bfd/ etc. it probably was inappropriate to do from .mh.

Yeah.

-- 
Pedro Alves

Re: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]

[Jan Kratochvil]

On Mon, 03 May 2010 01:41:55 +0200, Pedro Alves wrote:
> This is okay.  Thanks for doing this.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-05/msg00012.html

Great to simplify the code a bit.


Thanks,
Jan

RE: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]

[Pierre Muller]

  Should I add a rule to gdb_ari.sh
suggesting to use ATTRIBUTE_NORETURN
instead of NORETURN or ATTR_NORETURN?

  Should I do the same for ATTRIBUTE_PRINTF?

Pierre
as ARI maintainer.

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Jan Kratochvil
> Envoyé : Monday, May 03, 2010 1:53 AM
> À : Pedro Alves
> Cc : gdb-patches@sourceware.org; Mark Kettenis
> Objet : Re: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification
> [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]
> 
> On Mon, 03 May 2010 01:41:55 +0200, Pedro Alves wrote:
> > This is okay.  Thanks for doing this.
> 
> Checked-in:
> 	http://sourceware.org/ml/gdb-cvs/2010-05/msg00012.html
> 
> Great to simplify the code a bit.
> 
> 
> Thanks,
> Jan

Re: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]

[Jan Kratochvil]

On Mon, 03 May 2010 09:17:42 +0200, Pierre Muller wrote:
>   Should I add a rule to gdb_ari.sh
> suggesting to use ATTRIBUTE_NORETURN
> instead of NORETURN or ATTR_NORETURN?
> 
>   Should I do the same for ATTRIBUTE_PRINTF?

Any of an inadvertent use of NORETURN, ATTR_NORETURN or ATTR_FORMAT now causes
a compilation error as definitions of these symbols have been removed now.
Therefore I believe ARI is not needed in this case.


Thanks,
Jan

Re: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   Should I add a rule to gdb_ari.sh
Pierre> suggesting to use ATTRIBUTE_NORETURN
Pierre> instead of NORETURN or ATTR_NORETURN?

Pierre>   Should I do the same for ATTRIBUTE_PRINTF?

That is fine by me.

These do seem like identifiers we could safely poison in defs.h.

Tom

Re: [patch 1/3] Make obconcat use stdarg

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> On Fri, 30 Apr 2010 21:08:48 +0200, Mark Kettenis wrote:
>> I'm afraid you'll need an explicit cast for the NULLs used as the
>> sentinel value

Jan> There is also needed a fix for cases already protected by
Jan> ATTRIBUTE_SENTINEL and already using just the bare "NULL" sentinel:

Yes.

Jan> Nobody has noticed this problem since 2004.

Lucky, I guess.

Elsewhere gdb uses `(char *) 0':

./ser-pipe.c:      execl ("/bin/sh", "sh", "-c", name, (char *) 0);

I am ok with that, or `(char *) NULL' or even `#define SENTINEL ...'.

I realize this is ugly, but that's C for you.  This problem has really
bitten people in the past, it pays to be pedantically correct here.

Tom

Re: [patch 1/3] Make obconcat use stdarg

[Mark Kettenis]

> Date: Sun, 2 May 2010 09:52:17 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Fri, 30 Apr 2010 21:08:48 +0200, Mark Kettenis wrote:
> > I'm afraid you'll need an explicit cast for the NULLs used as the
> > sentinel value
> 
> There is also needed a fix for cases already protected by ATTRIBUTE_SENTINEL
> and already using just the bare "NULL" sentinel:
> 
> concat reconcat concat_length concat_copy concat_copy2
> 
> fbsd-nat.c by Mark Kettenis 50e330a0 2004-09-19
> (reconcat has ATTRIBUTE_SENTINEL since bf7628f0 2004-09-05)
>         psargs = reconcat (psargs, psargs, " ", get_inferior_args (), NULL);
> remote.c [HAVE_LIBEXPAT]
>     remote_support_xml = concat ("xmlRegisters=", xml, NULL);
>       p = concat (remote_support_xml, ",", xml, NULL);
> remote.c d5ea7042 2010-03-30
>       char *p = concat (msg, ";", append, NULL);
>           char *p = concat ("qSupported:", q, NULL);
>       name = concat ("$", tsv->name, NULL);
> utils.c 8b58bcf2 2009-01-26
>                   concat ("maintenance set ", problem->name, " ", NULL),
>                   concat ("maintenance show ", problem->name, " ", NULL),
> 
> Nobody has noticed this problem since 2004.

Obviously the number of platforms that have:

1. Define NULL as an integer 0.

2. Use a compiler for which we turn on the sentinel warning.

is vanishingly small.

Incidentally I'm working on switching the OpenBSD system compiler from
GCC 3.x to GCC 4.x.  And warnings about the cases you mention above do
show up.  Here's a diff to address them.

ok?


2010-05-03  Mark Kettenis  <kettenis@faure.sibelius.xs4all.nl>

	* remote.c (register_remote_support_xml)
	(remote_query_supported_append, remote_query_supported): Add cast
	to NULL used as sentinel.
	* tracepoint.c (tvariables_info_1): Likewise.
	* utils.c (add_internal_problem_command): Likewise.


Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.402
diff -u -p -r1.402 remote.c
--- remote.c	16 Apr 2010 07:49:35 -0000	1.402
+++ remote.c	3 May 2010 20:08:09 -0000
@@ -3473,7 +3473,7 @@ register_remote_support_xml (const char 
 {
 #if defined(HAVE_LIBEXPAT)
   if (remote_support_xml == NULL)
-    remote_support_xml = concat ("xmlRegisters=", xml, NULL);
+    remote_support_xml = concat ("xmlRegisters=", xml, (char *)NULL);
   else
     {
       char *copy = xstrdup (remote_support_xml + 13);
@@ -3491,7 +3491,7 @@ register_remote_support_xml (const char 
       while ((p = strtok (NULL, ",")) != NULL);
       xfree (copy);
 
-      p = concat (remote_support_xml, ",", xml, NULL);
+      p = concat (remote_support_xml, ",", xml, (char *)NULL);
       xfree (remote_support_xml);
       remote_support_xml = p;
     }
@@ -3503,7 +3503,7 @@ remote_query_supported_append (char *msg
 {
   if (msg)
     {
-      char *p = concat (msg, ";", append, NULL);
+      char *p = concat (msg, ";", append, (char *)NULL);
       xfree (msg);
       return p;
     }
@@ -3543,7 +3543,7 @@ remote_query_supported (void)
 
       if (q)
 	{
-	  char *p = concat ("qSupported:", q, NULL);
+	  char *p = concat ("qSupported:", q, (char *)NULL);
 	  xfree (q);
 	  putpkt (p);
 	  xfree (p);
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.184
diff -u -p -r1.184 tracepoint.c
--- tracepoint.c	23 Apr 2010 23:51:05 -0000	1.184
+++ tracepoint.c	3 May 2010 20:08:10 -0000
@@ -446,7 +446,7 @@ tvariables_info_1 (void)
 
       back_to2 = make_cleanup_ui_out_tuple_begin_end (uiout, "variable");
 
-      name = concat ("$", tsv->name, NULL);
+      name = concat ("$", tsv->name, (char *)NULL);
       make_cleanup (xfree, name);
       ui_out_field_string (uiout, "name", name);
       ui_out_field_string (uiout, "initial", plongest (tsv->initial_value));
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.228
diff -u -p -r1.228 utils.c
--- utils.c	8 Mar 2010 19:20:38 -0000	1.228
+++ utils.c	3 May 2010 20:08:11 -0000
@@ -1120,13 +1120,15 @@ add_internal_problem_command (struct int
   add_prefix_cmd ((char*) problem->name,
 		  class_maintenance, set_internal_problem_cmd, set_doc,
 		  set_cmd_list,
-		  concat ("maintenance set ", problem->name, " ", NULL),
+		  concat ("maintenance set ", problem->name, " ",
+			  (char *)NULL),
 		  0/*allow-unknown*/, &maintenance_set_cmdlist);
 
   add_prefix_cmd ((char*) problem->name,
 		  class_maintenance, show_internal_problem_cmd, show_doc,
 		  show_cmd_list,
-		  concat ("maintenance show ", problem->name, " ", NULL),
+		  concat ("maintenance show ", problem->name, " ",
+			  (char *)NULL),
 		  0/*allow-unknown*/, &maintenance_show_cmdlist);
 
   set_doc = xstrprintf (_("\

Re: [patch 1/3] Make obconcat use stdarg

[Pedro Alves]

On Monday 03 May 2010 21:19:05, Mark Kettenis wrote:
> ok?

Yes, of course.

> 2010-05-03  Mark Kettenis  <kettenis@faure.sibelius.xs4all.nl>
> 
>         * remote.c (register_remote_support_xml)
>         (remote_query_supported_append, remote_query_supported): Add cast
>         to NULL used as sentinel.
>         * tracepoint.c (tvariables_info_1): Likewise.
>         * utils.c (add_internal_problem_command): Likewise.


> +    remote_support_xml = concat ("xmlRegisters=", xml, (char *)NULL);

It's way more common in our code base to put a space between cast
and "castee" (would be `(char *) NULL' in this case).  Not a big
deal though.

-- 
Pedro Alves

RE: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Jan Kratochvil
> Envoyé : Monday, May 03, 2010 9:44 AM
> À : Pierre Muller
> Cc : 'Pedro Alves'; gdb-patches@sourceware.org; 'Mark Kettenis'
> Objet : Re: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification
> [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]
> 
> On Mon, 03 May 2010 09:17:42 +0200, Pierre Muller wrote:
> >   Should I add a rule to gdb_ari.sh
> > suggesting to use ATTRIBUTE_NORETURN
> > instead of NORETURN or ATTR_NORETURN?
> >
> >   Should I do the same for ATTRIBUTE_PRINTF?
> 
> Any of an inadvertent use of NORETURN, ATTR_NORETURN or ATTR_FORMAT now
> causes
> a compilation error as definitions of these symbols have been removed
> now.
> Therefore I believe ARI is not needed in this case.
  
  But they could still be defined in some 
headers of a particular system.
  If someone uses such a macro for a native file,
we will get no feedback...


  Tom, what did you mean by poison these identifiers?
Something like
#undef NORETURN
#define NORETURN "Anything that will for sure create a compilation error"

I see nothing like this in gdb/defs.h 
Is this something we should start?

  
Pierre

Re: [patch] ATTR_NORETURN -> ATTRIBUTE_NORETURN unification [Re: [patch] ATTR_* -> ATTRIBUTE_* unification]

[Joel Brobecker]

>   Tom, what did you mean by poison these identifiers?
> Something like
> #undef NORETURN
> #define NORETURN "Anything that will for sure create a compilation error"

It's a pragma. For instance:

    #pragma GCC poison calloc strdup

-- 
Joel

Re: [patch 1/3] Make obconcat use stdarg

[Mark Kettenis]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Mon, 3 May 2010 22:00:14 +0100
> 
> On Monday 03 May 2010 21:19:05, Mark Kettenis wrote:
> > ok?
> 
> Yes, of course.
> 
> > 2010-05-03  Mark Kettenis  <kettenis@faure.sibelius.xs4all.nl>
> > 
> >         * remote.c (register_remote_support_xml)
> >         (remote_query_supported_append, remote_query_supported): Add cast
> >         to NULL used as sentinel.
> >         * tracepoint.c (tvariables_info_1): Likewise.
> >         * utils.c (add_internal_problem_command): Likewise.
> 
> 
> > +    remote_support_xml = concat ("xmlRegisters=", xml, (char *)NULL);
> 
> It's way more common in our code base to put a space between cast
> and "castee" (would be `(char *) NULL' in this case).  Not a big
> deal though.

I added the missing spaces and committed the diff.  Thanks!

Re: [patch 1/3] Make obconcat use stdarg

[Jan Kratochvil]

On Tue, 04 May 2010 23:01:33 +0200, Mark Kettenis wrote:
> > On Monday 03 May 2010 21:19:05, Mark Kettenis wrote:
> > > 2010-05-03  Mark Kettenis  <kettenis@faure.sibelius.xs4all.nl>
> > > 
> > >         * remote.c (register_remote_support_xml)
> > >         (remote_query_supported_append, remote_query_supported): Add cast
> > >         to NULL used as sentinel.
> > >         * tracepoint.c (tvariables_info_1): Likewise.
> > >         * utils.c (add_internal_problem_command): Likewise.
[...]
> I added the missing spaces and committed the diff.  Thanks!

Assuming this remaining one is OK to check-in.


Regards,
Jan


gdb/
2010-05-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* fbsd-nat.c (fbsd_make_corefile_notes): Add cast to NULL used as
	sentinel.

--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -208,7 +208,8 @@ fbsd_make_corefile_notes (bfd *obfd, int *note_size)
       char *psargs = xstrdup (fname);
 
       if (get_inferior_args ())
-	psargs = reconcat (psargs, psargs, " ", get_inferior_args (), NULL);
+	psargs = reconcat (psargs, psargs, " ", get_inferior_args (),
+			   (char *) NULL);
 
       note_data = elfcore_write_prpsinfo (obfd, note_data, note_size,
 					  fname, psargs);

Re: [patch 1/3] Make obconcat use stdarg

[Pedro Alves]

On Friday 07 May 2010 15:31:03, Jan Kratochvil wrote:
> Assuming this remaining one is OK to check-in.

Okay.

-- 
Pedro Alves

Re: [patch 1/3] Make obconcat use stdarg

[Jan Kratochvil]

On Fri, 07 May 2010 16:36:06 +0200, Pedro Alves wrote:
> On Friday 07 May 2010 15:31:03, Jan Kratochvil wrote:
> > Assuming this remaining one is OK to check-in.
> 
> Okay.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-05/msg00080.html


Thanks,
Jan

Re: [patch 1/3] Make obconcat use stdarg

[Mark Kettenis]

> Date: Fri, 7 May 2010 16:31:03 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Tue, 04 May 2010 23:01:33 +0200, Mark Kettenis wrote:
> > > On Monday 03 May 2010 21:19:05, Mark Kettenis wrote:
> > > > 2010-05-03  Mark Kettenis  <kettenis@faure.sibelius.xs4all.nl>
> > > > 
> > > >         * remote.c (register_remote_support_xml)
> > > >         (remote_query_supported_append, remote_query_supported): Add cast
> > > >         to NULL used as sentinel.
> > > >         * tracepoint.c (tvariables_info_1): Likewise.
> > > >         * utils.c (add_internal_problem_command): Likewise.
> [...]
> > I added the missing spaces and committed the diff.  Thanks!
> 
> Assuming this remaining one is OK to check-in.

Sure!  Didn't hit that one on OpenBSD for obvious reasons ;).

> gdb/
> 2010-05-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* fbsd-nat.c (fbsd_make_corefile_notes): Add cast to NULL used as
> 	sentinel.
> 
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -208,7 +208,8 @@ fbsd_make_corefile_notes (bfd *obfd, int *note_size)
>        char *psargs = xstrdup (fname);
>  
>        if (get_inferior_args ())
> -	psargs = reconcat (psargs, psargs, " ", get_inferior_args (), NULL);
> +	psargs = reconcat (psargs, psargs, " ", get_inferior_args (),
> +			   (char *) NULL);
>  
>        note_data = elfcore_write_prpsinfo (obfd, note_data, note_size,
>  					  fname, psargs);
> 

Re: `sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Jan Kratochvil]

On Sat, 01 May 2010 10:53:32 +0200, Mark Kettenis wrote:
> > Date: Sat, 1 May 2010 00:18:39 +0200
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > On Fri, 30 Apr 2010 21:08:48 +0200, Mark Kettenis wrote:
> > > > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > > > __attribute__ ((sentinel)) availability for gcc >= 4.0 I have copied from
> > > > <glib-2.0/glib/gmacros.h>.  It roughly matches the GCC ChangeLog dates.
> > > 
> > > The OpenBSD system compile, which is based on GCC 3.3.5, already has
> > > the sentinel attribute.
> > 
> > FSF GCC 3.3.5 does not support this attribute:
[...]
> > Moreover I do not consider relevant to support this attribute on so obsolete
> > compiler.  Not using the attribute has no effect on users building GDB.
> > This attribute is useful only for the GDB developers.
> 
> Fair enough; this feature is nice to have but in no way essential.

Neither FSF GCC 3.4.6 which is the latest FSF GCC 3.x release.  Therefore it
must be an OpenBSD specific patch which is irrelevant to the FSF GDB codebase.


Reposted - using now existing ansidecl.h ATTRIBUTE_SENTINEL instead.


Thanks,
Jan


gdb/
2010-05-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (typename_concat): Use (char *) NULL terminated stdarg
	list for the obconcat call.
	* mdebugread.c (parse_symbol): Likewise.
	* stabsread.c (define_symbol, read_member_functions, read_cpp_abbrev):
	Likewise.
	* symfile.c (obconcat): Replace the s1, s2 and s3 parameters by `...'.
	New variable ap.  Remove variables len and val.
	* symfile.h (obconcat): Likewise for the prototype.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9200,7 +9200,7 @@ typename_concat (struct obstack *obs, const char *prefix, const char *suffix,
   else
     {
       /* We have an obstack.  */
-      return obconcat (obs, prefix, sep, suffix);
+      return obconcat (obs, prefix, sep, suffix, (char *) NULL);
     }
 }
 
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -998,8 +998,8 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
 	if (sh->iss == 0 || name[0] == '.' || name[0] == '\0')
 	  TYPE_TAG_NAME (t) = NULL;
 	else
-	  TYPE_TAG_NAME (t) = obconcat (&current_objfile->objfile_obstack,
-					"", "", name);
+	  TYPE_TAG_NAME (t) = obconcat (&current_objfile->objfile_obstack, name,
+					(char *) NULL);
 
 	TYPE_CODE (t) = type_code;
 	TYPE_LENGTH (t) = sh->value;
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -1279,9 +1279,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
           SYMBOL_VALUE (struct_sym) = valu;
           SYMBOL_DOMAIN (struct_sym) = STRUCT_DOMAIN;
           if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
-            TYPE_NAME (SYMBOL_TYPE (sym))
-              = obconcat (&objfile->objfile_obstack, "", "",
-                          SYMBOL_LINKAGE_NAME (sym));
+            TYPE_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      (char *) NULL);
           add_symbol_to_list (struct_sym, &file_symbols);
         }
       
@@ -1306,9 +1306,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
       SYMBOL_VALUE (sym) = valu;
       SYMBOL_DOMAIN (sym) = STRUCT_DOMAIN;
       if (TYPE_TAG_NAME (SYMBOL_TYPE (sym)) == 0)
-	TYPE_TAG_NAME (SYMBOL_TYPE (sym))
-	  = obconcat (&objfile->objfile_obstack, "", "",
-		      SYMBOL_LINKAGE_NAME (sym));
+	TYPE_TAG_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      (char *) NULL);
       add_symbol_to_list (sym, &file_symbols);
 
       if (synonym)
@@ -1321,9 +1321,9 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
 	  SYMBOL_VALUE (typedef_sym) = valu;
 	  SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 	  if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
-	    TYPE_NAME (SYMBOL_TYPE (sym))
-	      = obconcat (&objfile->objfile_obstack, "", "",
-			  SYMBOL_LINKAGE_NAME (sym));
+	    TYPE_NAME (SYMBOL_TYPE (sym)) = obconcat (&objfile->objfile_obstack,
+						      SYMBOL_LINKAGE_NAME (sym),
+						      (char *) NULL);
 	  add_symbol_to_list (typedef_sym, &file_symbols);
 	}
       break;
@@ -2609,8 +2609,8 @@ read_member_functions (struct field_info *fip, char **pp, struct type *type,
 	      make_cleanup (xfree, destr_fnlist);
 	      memset (destr_fnlist, 0, sizeof (struct next_fnfieldlist));
 	      destr_fnlist->fn_fieldlist.name
-		= obconcat (&objfile->objfile_obstack, "", "~",
-			    new_fnlist->fn_fieldlist.name);
+		= obconcat (&objfile->objfile_obstack, "~",
+			    new_fnlist->fn_fieldlist.name, (char *) NULL);
 
 	      destr_fnlist->fn_fieldlist.fn_fields = (struct fn_field *)
 		obstack_alloc (&objfile->objfile_obstack,
@@ -2747,8 +2747,8 @@ read_cpp_abbrev (struct field_info *fip, char **pp, struct type *type,
 	  {
 		  name = "";
 	  }
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack, vptr_name, name, "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack,
+					    vptr_name, name, (char *) NULL);
 	  break;
 
 	case 'b':		/* $vb -- a virtual bsomethingorother */
@@ -2760,15 +2760,15 @@ read_cpp_abbrev (struct field_info *fip, char **pp, struct type *type,
 			 symnum);
 	      name = "FOO";
 	    }
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack, vb_name, name, "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack, vb_name,
+					    name, (char *) NULL);
 	  break;
 
 	default:
 	  invalid_cpp_abbrev_complaint (*pp);
-	  fip->list->field.name =
-	    obconcat (&objfile->objfile_obstack,
-		      "INVALID_CPLUSPLUS_ABBREV", "", "");
+	  fip->list->field.name = obconcat (&objfile->objfile_obstack,
+					    "INVALID_CPLUSPLUS_ABBREV",
+					    (char *) NULL);
 	  break;
 	}
 
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -212,19 +212,29 @@ obsavestring (const char *ptr, int size, struct obstack *obstackp)
   return p;
 }
 
-/* Concatenate strings S1, S2 and S3; return the new string.  Space is found
-   in the obstack pointed to by OBSTACKP.  */
+/* Concatenate NULL terminated variable argument list of `const char *' strings;
+   return the new string.  Space is found in the OBSTACKP.  Argument list must
+   be terminated by a sentinel expression `(char *) NULL'.  */
 
 char *
-obconcat (struct obstack *obstackp, const char *s1, const char *s2,
-	  const char *s3)
-{
-  int len = strlen (s1) + strlen (s2) + strlen (s3) + 1;
-  char *val = (char *) obstack_alloc (obstackp, len);
-  strcpy (val, s1);
-  strcat (val, s2);
-  strcat (val, s3);
-  return val;
+obconcat (struct obstack *obstackp, ...)
+{
+  va_list ap;
+
+  va_start (ap, obstackp);
+  for (;;)
+    {
+      const char *s = va_arg (ap, const char *);
+
+      if (s == NULL)
+	break;
+
+      obstack_grow_str (obstackp, s);
+    }
+  va_end (ap);
+  obstack_1grow (obstackp, 0);
+
+  return obstack_finish (obstackp);
 }
 
 /* True if we are reading a symbol table. */
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -458,11 +458,11 @@ extern struct partial_symtab *start_psymtab_common (struct objfile *,
 
 extern char *obsavestring (const char *, int, struct obstack *);
 
-/* Concatenate strings S1, S2 and S3; return the new string.  Space is
-   found in the OBSTACKP  */
+/* Concatenate NULL terminated variable argument list of `const char *' strings;
+   return the new string.  Space is found in the OBSTACKP.  Argument list must
+   be terminated by a sentinel expression `(char *) NULL'.  */
 
-extern char *obconcat (struct obstack *obstackp, const char *, const char *,
-		       const char *);
+extern char *obconcat (struct obstack *obstackp, ...) ATTRIBUTE_SENTINEL;
 
 			/*   Variables   */
 

Re: `sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> 2010-05-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* dwarf2read.c (typename_concat): Use (char *) NULL terminated stdarg
Jan> 	list for the obconcat call.
Jan> 	* mdebugread.c (parse_symbol): Likewise.
Jan> 	* stabsread.c (define_symbol, read_member_functions, read_cpp_abbrev):
Jan> 	Likewise.
Jan> 	* symfile.c (obconcat): Replace the s1, s2 and s3 parameters by `...'.
Jan> 	New variable ap.  Remove variables len and val.
Jan> 	* symfile.h (obconcat): Likewise for the prototype.

Ok.

Tom

Re: `sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Jan Kratochvil]

On Sat, 08 May 2010 00:34:21 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> 2010-05-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> Jan> 	* dwarf2read.c (typename_concat): Use (char *) NULL terminated stdarg
> Jan> 	list for the obconcat call.
> Jan> 	* mdebugread.c (parse_symbol): Likewise.
> Jan> 	* stabsread.c (define_symbol, read_member_functions, read_cpp_abbrev):
> Jan> 	Likewise.
> Jan> 	* symfile.c (obconcat): Replace the s1, s2 and s3 parameters by `...'.
> Jan> 	New variable ap.  Remove variables len and val.
> Jan> 	* symfile.h (obconcat): Likewise for the prototype.
> 
> Ok.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-05/msg00090.html


Thanks,
Jan