[RFC] Change OK: to ARI: for ARI rule ignore

[RFC] Change OK: to ARI: for ARI rule ignore

[Pierre Muller]

   Currently gdb_ari.sh
ignores all lines containing either
"/* OK ", "/* OK: " or "/* ARI: " patterns
completely.

   When looking for occurrences of these patterns, I discovered
that they were also sometimes used by people not really wanting to
inform ARI to disregard this line, but simply
to add a simple comment.

  Thus, I would like to restrict that rule to only accept
/* ARI: rule_name */.

  My patches has two parts:
one concerning gdb/utils.c and gdb/gdb_dirent.h sources
with OK to ARI replacements,
and a second patch for gdb_ari.sh that
only recognizes ARI pattern and remembers the name of the rule
and still parses the line for other possible warnings.

   
  The main problem if I commit both before next release is that,
if ARI is rerun on gdb 6.8 release, this change will generate
new false warnings.

  Thus, it is probably safer to postpone the second part of
the patch to after next release.

Comments?

Pierre Muller


ChangeLog entry:

 

2009-04-23  Pierre Muller  <muller.u-strasbg.fr>

	ARI change: Use "/* ARI: rule */" pattern.
	* utils.c: Update ARI patterns.
	* gdb_dirent.h: Likewise.


Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.209
diff -u -p -r1.209 utils.c
--- utils.c	15 Apr 2009 20:40:51 -0000	1.209
+++ utils.c	20 Apr 2009 16:18:44 -0000
@@ -68,10 +68,10 @@
 #include <time.h>
 
 #if !HAVE_DECL_MALLOC
-extern PTR malloc ();		/* OK: PTR */
+extern PTR malloc ();		/* ARI: PTR */
 #endif
 #if !HAVE_DECL_REALLOC
-extern PTR realloc ();		/* OK: PTR */
+extern PTR realloc ();		/* ARI: PTR */
 #endif
 #if !HAVE_DECL_FREE
 extern void free ();
@@ -1201,7 +1201,7 @@ nomem (long size)
 /* NOTE: These are declared using PTR to ensure consistency with
    "libiberty.h".  xfree() is GDB local.  */
 
-PTR				/* OK: PTR */
+PTR				/* ARI: PTR */
 xmalloc (size_t size)
 {
   void *val;
@@ -1211,7 +1211,7 @@ xmalloc (size_t size)
   if (size == 0)
     size = 1;
 
-  val = malloc (size);		/* OK: malloc */
+  val = malloc (size);		/* ARI: malloc */
   if (val == NULL)
     nomem (size);
 
@@ -1224,8 +1224,8 @@ xzalloc (size_t size)
   return xcalloc (1, size);
 }
 
-PTR				/* OK: PTR */
-xrealloc (PTR ptr, size_t size)	/* OK: PTR */
+PTR				/* ARI: PTR */
+xrealloc (PTR ptr, size_t size)	/* ARI: PTR */
 {
   void *val;
 
@@ -1235,16 +1235,16 @@ xrealloc (PTR ptr, size_t size)	/* OK: P
     size = 1;
 
   if (ptr != NULL)
-    val = realloc (ptr, size);	/* OK: realloc */
+    val = realloc (ptr, size);	/* ARI: realloc */
   else
-    val = malloc (size);		/* OK: malloc */
+    val = malloc (size);		/* ARI: malloc */
   if (val == NULL)
     nomem (size);
 
   return (val);
 }
 
-PTR				/* OK: PTR */
+PTR				/* ARI: PTR */
 xcalloc (size_t number, size_t size)
 {
   void *mem;
@@ -1257,7 +1257,7 @@ xcalloc (size_t number, size_t size)
       size = 1;
     }
 
-  mem = calloc (number, size);		/* OK: xcalloc */
+  mem = calloc (number, size);		/* ARI: xcalloc */
   if (mem == NULL)
     nomem (number * size);
 
@@ -1268,7 +1268,7 @@ void
 xfree (void *ptr)
 {
   if (ptr != NULL)
-    free (ptr);		/* OK: free */
+    free (ptr);		/* ARI: free */
 }
 

 
Index: gdb_dirent.h
===================================================================
RCS file: /cvs/src/src/gdb/gdb_dirent.h,v
retrieving revision 1.8
diff -u -p -r1.8 gdb_dirent.h
--- gdb_dirent.h	3 Jan 2009 05:57:51 -0000	1.8
+++ gdb_dirent.h	20 Apr 2009 16:18:44 -0000
@@ -21,11 +21,11 @@
 
 /* See description of `AC_HEADER_DIRENT' in the Autoconf manual.  */
 #ifdef HAVE_DIRENT_H
-# include <dirent.h>		/* OK: dirent.h */
-# define NAMELEN(dirent) strlen ((dirent)->d_name)	/* OK: strlen d_name
*/
+# include <dirent.h>		/* ARI: dirent.h */
+# define NAMELEN(dirent) strlen ((dirent)->d_name)	/* ARI: strlen
d_name */
 #else
 # define dirent direct
-# define NAMELEN(dirent) (dirent)->d_namelen	/* OK: d_namelen */
+# define NAMELEN(dirent) (dirent)->d_namelen	/* ARI: d_namelen */
 # ifdef HAVE_SYS_NDIR_H
 #  include <sys/ndir.h>
 # endif


Index: gdb_ari.sh
===================================================================
RCS file: /cvs/gdbadmin/ss/gdb_ari.sh,v
retrieving revision 1.89
diff -u -r1.89 gdb_ari.sh
--- gdb_ari.sh	20 Apr 2009 15:55:23 -0000	1.89
+++ gdb_ari.sh	21 Apr 2009 22:31:36 -0000
@@ -147,6 +147,9 @@
 	exit
     }
 
+    if (ARI_OK == bug) {
+	exit
+    }
     # Trim the filename down to just DIRECTORY/FILE so that it can be
     # robustly used by the FIX code.
 
@@ -206,7 +209,18 @@
 /(^|[^_[:alnum:]])OBSOLETE([^_[:alnum:]]|$)/ { next; }
 
 # Skip OK lines
-/\/\* OK \*\// || /\/\* OK: / || /\/\* ARI: / { next; }
+/\/\* OK \*\// || /\/\* OK: / {
+    print "Skipping " FILENAME ":" FNR ":" $0
+    next; 
+}
+
+/\/\* ARI:[[:space:]]*(.*)[[:space:]]*\*\// {
+    ARI_OK = $1
+}
+! /\/\* ARI:[[:space:]]*(.*)[[:space:]]*\*\// {
+    ARI_OK = ""
+}
+
 
 # Things in comments
 

Re: [RFC] Change OK: to ARI: for ARI rule ignore

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <muller@ics.u-strasbg.fr> writes:

Pierre>   Thus, I would like to restrict that rule to only accept
Pierre> /* ARI: rule_name */.

I think this sounds like a good idea.

Pierre>   The main problem if I commit both before next release is that,
Pierre> if ARI is rerun on gdb 6.8 release, this change will generate
Pierre> new false warnings.

Is that something we would want to do?  Can't we just run it once, say
today, and save the results?  I mean, if that is something we need.

Tom

RE: [RFC] Change OK: to ARI: for ARI rule ignore

[Pierre Muller]

> -----Message d'origine-----
> De : Tom Tromey [mailto:tromey@redhat.com]
> Envoyé : Thursday, April 23, 2009 1:44 AM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFC] Change OK: to ARI: for ARI rule ignore
> 
> >>>>> "Pierre" == Pierre Muller <muller@ics.u-strasbg.fr> writes:
> 
> Pierre>   Thus, I would like to restrict that rule to only accept
> Pierre> /* ARI: rule_name */.
> 
> I think this sounds like a good idea.

  
> Pierre>   The main problem if I commit both before next release is
> that,
> Pierre> if ARI is rerun on gdb 6.8 release, this change will generate
> Pierre> new false warnings.
> 
> Is that something we would want to do?  Can't we just run it once, say
> today, and save the results?  I mean, if that is something we need.

You are right, the ARI page for 6.8 release dates
from 2008/03/27 which is the release date...
There is nothing in the automatic script that seems to update
this page, so maybe I can commit both together?

Pierre

Re: [RFC] Change OK: to ARI: for ARI rule ignore

[Joel Brobecker]

> Pierre>   Thus, I would like to restrict that rule to only accept
> Pierre> /* ARI: rule_name */.
> 
> I think this sounds like a good idea.

Seconded.

> Pierre>   The main problem if I commit both before next release is that,
> Pierre> if ARI is rerun on gdb 6.8 release, this change will generate
> Pierre> new false warnings.
> 
> Is that something we would want to do?  Can't we just run it once, say
> today, and save the results?  I mean, if that is something we need.

I'm wondering - how did the gdb-6.8 ARI results get computed and
published. Was this done automatically?

-- 
Joel

Re: [RFC] Change OK: to ARI: for ARI rule ignore

[Tom Tromey]

Pierre> There is nothing in the automatic script that seems to update
Pierre> this page, so maybe I can commit both together?

Sounds good to me.

Tom

RE: [RFC] Change OK: to ARI: for ARI rule ignore

[Pierre Muller]

Thanks to Joel and Tom for their answers.

  I checked the source patch in.

The only thing is that the 6.8 branch will 
have new warnings because the OK patterns
are not disregarded anymore, but
I don't think that we will ever release a 6.8.1 anyhow!

  My initial idea for gdb_ari.sh turned out to be
wrong (I got more regression)
The final diff for gdb_ari.sh looks like this:

$ cvs diff gdb_ari.sh
Index: gdb_ari.sh
===================================================================
RCS file: /cvs/gdbadmin/ss/gdb_ari.sh,v
retrieving revision 1.89
diff -r1.89 gdb_ari.sh
149a150,152
>     if (ARI_OK == bug) {
>       return
>     }
208,209c211,225
< # Skip OK lines
< /\/\* OK \*\// || /\/\* OK: / || /\/\* ARI: / { next; }
---
> # Skip ARI lines
>
> BEGIN {
>     ARI_OK = ""
> }
>
> /\/\* ARI:[[:space:]]*(.*)[[:space:]]*\*\// {
>     ARI_OK = gensub(/^.*\/\*
ARI:[[:space:]]*(.*[^[:space:]])[[:space:]]*\*\/.*$/, "\\1", 1, $0)
>     # print "ARI line found \"" $0 "\""
>     # print "ARI_OK \"" ARI_OK "\""
> }
> ! /\/\* ARI:[[:space:]]*(.*)[[:space:]]*\*\// {
>     ARI_OK = ""
> }
>

Is this OK?

Pierre

Re: [RFC] Change OK: to ARI: for ARI rule ignore

[Daniel Jacobowitz]

I have no comments on your change, but I do have a request:

On Sat, Apr 25, 2009 at 01:24:27AM +0200, Pierre Muller wrote:
> $ cvs diff gdb_ari.sh
> Index: gdb_ari.sh
> ===================================================================
> RCS file: /cvs/gdbadmin/ss/gdb_ari.sh,v
> retrieving revision 1.89
> diff -r1.89 gdb_ari.sh
> 149a150,152
> >     if (ARI_OK == bug) {
> >       return
> >     }

My ~/.cvsrc contains "diff -upN".  It does not really matter what the
options are - but please use either unified (my preference for most
changes) or context diff.  Classic diff is too hard to read.


-- 
Daniel Jacobowitz
CodeSourcery

RE: [RFC] Change OK: to ARI: for ARI rule ignore

[Pierre Muller]

Sorry about this, Daniel.

  It is just that I do have the bad habit of
thinking of '-p' and '-u' options together, and
'-p' isn't very useful when it's not C code...

Here is that unified diff.

Is this OK for everyone?


Pierre Muller
Pascal language support maintainer for GDB






$ cvs diff -u gdb_ari.sh
Index: gdb_ari.sh
===================================================================
RCS file: /cvs/gdbadmin/ss/gdb_ari.sh,v
retrieving revision 1.89
diff -u -r1.89 gdb_ari.sh
--- gdb_ari.sh  20 Apr 2009 15:55:23 -0000      1.89
+++ gdb_ari.sh  1 May 2009 08:13:59 -0000
@@ -147,6 +147,9 @@
        exit
     }

+    if (ARI_OK == bug) {
+       return
+    }
     # Trim the filename down to just DIRECTORY/FILE so that it can be
     # robustly used by the FIX code.

@@ -205,8 +208,21 @@
 # Skip OBSOLETE lines
 /(^|[^_[:alnum:]])OBSOLETE([^_[:alnum:]]|$)/ { next; }

-# Skip OK lines
-/\/\* OK \*\// || /\/\* OK: / || /\/\* ARI: / { next; }
+# Skip ARI lines
+
+BEGIN {
+    ARI_OK = ""
+}
+
+/\/\* ARI:[[:space:]]*(.*)[[:space:]]*\*\// {
+    ARI_OK = gensub(/^.*\/\*
ARI:[[:space:]]*(.*[^[:space:]])[[:space:]]*\*\/.*$/, "\\1", 1, $0)
+    # print "ARI line found \"" $0 "\""
+    # print "ARI_OK \"" ARI_OK "\""
+}
+! /\/\* ARI:[[:space:]]*(.*)[[:space:]]*\*\// {
+    ARI_OK = ""
+}
+

 # Things in comments

Re: [RFC] Change OK: to ARI: for ARI rule ignore

[Joel Brobecker]

> Is this OK for everyone?

Sure. Not sure if you meant to keep the commented-out debug code
in your patch, but looks OK otherwise.

-- 
Joel

RE: [RFC] Change OK: to ARI: for ARI rule ignore

[Pierre Muller]

Thanks, committed.

Pierre

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Wednesday, May 06, 2009 6:16 PM
> À : Pierre Muller
> Cc : 'Daniel Jacobowitz'; 'Tom Tromey'; gdb-patches@sourceware.org
> Objet : Re: [RFC] Change OK: to ARI: for ARI rule ignore
> 
> > Is this OK for everyone?
> 
> Sure. Not sure if you meant to keep the commented-out debug code
> in your patch, but looks OK otherwise.
> 
> --
> Joel