diff mbox

[review] Avoid ARI warnings with #include

Message ID gerrit.1575560698000.I215683952dec75379ea0a259e79c8dafbae210b9@gnutoolchain-gerrit.osci.io
State New, archived
Headers

Commit Message

Simon Marchi (Code Review) Dec. 5, 2019, 3:44 p.m. UTC 
  Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/746
......................................................................

Avoid ARI warnings with #include

ARI will complain about the use of certain include files, but in
isolated spots, these includes are actually necessary.  This patch
marks these spots to silence ARI.

gdb/ChangeLog
2019-12-05  Tom Tromey  <tromey@adacore.com>

	* gdb_regex.h: Add ARI comments.
	* gdb_vfork.h: Add ARI comment.

Change-Id: I215683952dec75379ea0a259e79c8dafbae210b9
---
M gdb/ChangeLog
M gdb/gdb_regex.h
M gdb/gdb_vfork.h
3 files changed, 8 insertions(+), 3 deletions(-)

Comments

Simon Marchi (Code Review) Dec. 5, 2019, 4:55 p.m. UTC | #1 
Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/746
......................................................................


Patch Set 1:

(1 comment)

| --- gdb/gdb_vfork.h
| +++ gdb/gdb_vfork.h
| @@ -13,14 +13,14 @@ /* GDB-friendly replacement for <vfork.h>.
|     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
|     GNU General Public License for more details.
|  
|     You should have received a copy of the GNU General Public License
|     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
|  
|  #ifndef GDB_VFORK_H
|  #define GDB_VFORK_H
|  
|  #if HAVE_VFORK_H
| -#include <vfork.h>
| +#include <vfork.h>		/* ARI: vfork.h */
|  #endif

PS1, Line 24:

Isn't this:

 BEGIN { doc["vfork.h"] = "\
 Do not include vfork.h, instead include gdb_vfork.h"
     fix("vfork.h", "gdb/gdb_vfork.h", 1);
                     ^^^^^^^^^^^^^^^
     category["vfork.h"] = ari_regression
 }

... supposed to suppress this case?

Same for the gdb_regex.h file.

|  
|  #endif /* GDB_VFORK_H */
Simon Marchi (Code Review) Dec. 5, 2019, 4:57 p.m. UTC | #2 
Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/746
......................................................................


Patch Set 1:

(1 comment)

| --- gdb/gdb_vfork.h
| +++ gdb/gdb_vfork.h
| @@ -13,14 +13,14 @@ /* GDB-friendly replacement for <vfork.h>.
|     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
|     GNU General Public License for more details.
|  
|     You should have received a copy of the GNU General Public License
|     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
|  
|  #ifndef GDB_VFORK_H
|  #define GDB_VFORK_H
|  
|  #if HAVE_VFORK_H
| -#include <vfork.h>
| +#include <vfork.h>		/* ARI: vfork.h */
|  #endif

PS1, Line 24:

> Isn't this:
...
> ... supposed to suppress this case?

Note, I wouldn't be opposed to always tag in the header file directly.
But in that case, it seems like the patch should remove the fix() call
from
the ARI at the same time.

But I'm curious why that isn't working.

|  
|  #endif /* GDB_VFORK_H */
Simon Marchi (Code Review) Dec. 12, 2019, 5:22 p.m. UTC | #3 
Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/746
......................................................................


Patch Set 1:

(1 comment)

| --- gdb/gdb_vfork.h
| +++ gdb/gdb_vfork.h
| @@ -13,14 +13,14 @@ /* GDB-friendly replacement for <vfork.h>.
|     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
|     GNU General Public License for more details.
|  
|     You should have received a copy of the GNU General Public License
|     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
|  
|  #ifndef GDB_VFORK_H
|  #define GDB_VFORK_H
|  
|  #if HAVE_VFORK_H
| -#include <vfork.h>
| +#include <vfork.h>		/* ARI: vfork.h */
|  #endif

PS1, Line 24:

> > Isn't this:
> ...
> > ... supposed to suppress this case?
> 
> Note, I wouldn't be opposed to always tag in the header file directly.
> But in that case, it seems like the patch should remove the fix() call from
> the ARI at the same time.
> 
> But I'm curious why that isn't working.

I ran it incorrectly -- I ran it from the gdb directory, but actually
you have to run it from the top-level source directory, so that the
paths match.  This is one reason I think removing the fix() calls
in favor of explicit annotations is better.

|  
|  #endif /* GDB_VFORK_H */
Simon Marchi (Code Review) Dec. 13, 2019, 2:35 p.m. UTC | #4 
Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/746
......................................................................


Patch Set 1:

(1 comment)

| --- gdb/gdb_vfork.h
| +++ gdb/gdb_vfork.h
| @@ -13,14 +13,14 @@ /* GDB-friendly replacement for <vfork.h>.
|     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
|     GNU General Public License for more details.
|  
|     You should have received a copy of the GNU General Public License
|     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
|  
|  #ifndef GDB_VFORK_H
|  #define GDB_VFORK_H
|  
|  #if HAVE_VFORK_H
| -#include <vfork.h>
| +#include <vfork.h>		/* ARI: vfork.h */
|  #endif

PS1, Line 24:

> > Note, I wouldn't be opposed to always tag in the header file directly.
> > But in that case, it seems like the patch should remove the fix() call from
> > the ARI at the same time.
> > 
> > But I'm curious why that isn't working.
> 
> I ran it incorrectly -- I ran it from the gdb directory, but actually
> you have to run it from the top-level source directory, so that the
> paths match.  This is one reason I think removing the fix() calls
> in favor of explicit annotations is better.

Yeah, let's remove the fix() calls.

|  
|  #endif /* GDB_VFORK_H */
Simon Marchi (Code Review) Dec. 16, 2019, 7:23 p.m. UTC | #5 
Tom Tromey has abandoned this change. ( https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/746 )

Change subject: Avoid ARI warnings with #include
......................................................................


Abandoned

I dropped this one for the time being.
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fe81ff8..aa7e9ab 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@ 
 2019-12-05  Tom Tromey  <tromey@adacore.com>
 
+	* gdb_regex.h: Add ARI comments.
+	* gdb_vfork.h: Add ARI comment.
+
+2019-12-05  Tom Tromey  <tromey@adacore.com>
+
 	* target-float.c (host_float_ops<T>::from_target): Add ARI
 	comment.
 
diff --git a/gdb/gdb_regex.h b/gdb/gdb_regex.h
index 58a0f22..5b81642 100644
--- a/gdb/gdb_regex.h
+++ b/gdb/gdb_regex.h
@@ -20,11 +20,11 @@ 
 #define GDB_REGEX_H 1
 
 #ifdef USE_INCLUDED_REGEX
-# include "xregex.h"
+# include "xregex.h"		/* ARI: xregex.h */
 #else
 /* Request 4.2 BSD regex functions.  */
 # define _REGEX_RE_COMP
-# include <regex.h>
+# include <regex.h>		/* ARI: regex.h */
 #endif
 
 /* A compiled regex.  This is mainly a wrapper around regex_t.  The
diff --git a/gdb/gdb_vfork.h b/gdb/gdb_vfork.h
index 8bf72d6..b76776b 100644
--- a/gdb/gdb_vfork.h
+++ b/gdb/gdb_vfork.h
@@ -20,7 +20,7 @@ 
 #define GDB_VFORK_H
 
 #if HAVE_VFORK_H
-#include <vfork.h>
+#include <vfork.h>		/* ARI: vfork.h */
 #endif
 
 #endif /* GDB_VFORK_H */