public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [review] Fix unused function error
@ 2019-12-11 15:00 Luis Machado (Code Review)
  2019-12-11 15:17 ` [review v2] " Luis Machado (Code Review)
                   ` (18 more replies)
  0 siblings, 19 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-11 15:00 UTC (permalink / raw)
  To: gdb-patches

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

Fix unused function error

Attempting to build GDB in Ubuntu 18.04.3 on x86_64, i ran into warnings
that caused the build to fail. For some reason the diagnostics macros don't
work for my case, but ATTRIBUTE_UNUSED does.

gdb/ChangeLog:

2019-12-11  Luis Machado  <luis.machado@linaro.org>

	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
	(select_strerror_r): Use ATTRIBUTED_UNUSED instead of the diagnostics
	macros.

Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
---
M gdb/gdbsupport/safe-strerror.c
1 file changed, 2 insertions(+), 7 deletions(-)



diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c
index 9973fa6..9126507 100644
--- a/gdb/gdbsupport/safe-strerror.c
+++ b/gdb/gdbsupport/safe-strerror.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "common-defs.h"
-#include "diagnostics.h"
 #include <string.h>
 
 /* There are two different versions of strerror_r; one is GNU-specific, the
@@ -29,25 +28,21 @@
 
 /* We only ever use one of the two overloads, so suppress the warning for
    an unused function.  */
-DIAGNOSTIC_PUSH
-DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
 
 /* Called if we have a XSI-compliant strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (int res, char *buf)
 {
   return res == 0 ? buf : nullptr;
 }
 
 /* Called if we have a GNU strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (char *res, char *)
 {
   return res;
 }
 
-DIAGNOSTIC_POP
-
 /* Implementation of safe_strerror as defined in common-utils.h.  */
 
 const char *

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 1
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-MessageType: newchange

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v2] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
@ 2019-12-11 15:17 ` Luis Machado (Code Review)
  2019-12-11 16:05 ` Pedro Alves (Code Review)
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-11 15:17 UTC (permalink / raw)
  To: gdb-patches

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

Fix unused function error

Attempting to build GDB in Ubuntu 18.04.3 on x86_64, i ran into warnings
that caused the build to fail. For some reason the diagnostics macros don't
work for my case, but ATTRIBUTE_UNUSED does.

The compiler version is gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0.

gdb/ChangeLog:

2019-12-11  Luis Machado  <luis.machado@linaro.org>

	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
	(select_strerror_r): Use ATTRIBUTED_UNUSED instead of the diagnostics
	macros.

Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
---
M gdb/gdbsupport/safe-strerror.c
1 file changed, 2 insertions(+), 7 deletions(-)



diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c
index 9973fa6..9126507 100644
--- a/gdb/gdbsupport/safe-strerror.c
+++ b/gdb/gdbsupport/safe-strerror.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "common-defs.h"
-#include "diagnostics.h"
 #include <string.h>
 
 /* There are two different versions of strerror_r; one is GNU-specific, the
@@ -29,25 +28,21 @@
 
 /* We only ever use one of the two overloads, so suppress the warning for
    an unused function.  */
-DIAGNOSTIC_PUSH
-DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
 
 /* Called if we have a XSI-compliant strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (int res, char *buf)
 {
   return res == 0 ? buf : nullptr;
 }
 
 /* Called if we have a GNU strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (char *res, char *)
 {
   return res;
 }
 
-DIAGNOSTIC_POP
-
 /* Implementation of safe_strerror as defined in common-utils.h.  */
 
 const char *

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 2
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v2] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
  2019-12-11 15:17 ` [review v2] " Luis Machado (Code Review)
@ 2019-12-11 16:05 ` Pedro Alves (Code Review)
  2019-12-11 17:05 ` Luis Machado (Code Review)
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-11 16:05 UTC (permalink / raw)
  To: Luis Machado, gdb-patches

Pedro Alves has posted comments on this change.

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


Patch Set 2:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,20 @@ 
| +Parent:     1d61b032 (Remove more shifts for sign/zero extension)
| +Author:     Luis Machado <luis.machado@linaro.org>
| +AuthorDate: 2019-12-11 11:55:49 -0300
| +Commit:     Luis Machado <luis.machado@linaro.org>
| +CommitDate: 2019-12-11 12:16:22 -0300
| +
| +Fix unused function error
| +
| +Attempting to build GDB in Ubuntu 18.04.3 on x86_64, i ran into warnings
| +that caused the build to fail. For some reason the diagnostics macros don't
| +work for my case, but ATTRIBUTE_UNUSED does.

PS2, Line 11:

What does the warning that you get look like?

Which gcc version do you have?

Does the DIAGNOSTIC_IGNORE_UNUSED_FUNCTION not work for you because it
ends up defined as empty for you for some reason?

Or can you reproduce this by unrolling the macros and using _Pragma
directly?

(typo: i -> I)

| +
| +The compiler version is gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0.
| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
| +	(select_strerror_r): Use ATTRIBUTED_UNUSED instead of the diagnostics

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 2
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 16:04:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v2] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
  2019-12-11 15:17 ` [review v2] " Luis Machado (Code Review)
  2019-12-11 16:05 ` Pedro Alves (Code Review)
@ 2019-12-11 17:05 ` Luis Machado (Code Review)
  2019-12-11 17:07 ` [review v3] " Luis Machado (Code Review)
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-11 17:05 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves

Luis Machado has posted comments on this change.

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


Patch Set 2:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,20 @@ 
| +Parent:     1d61b032 (Remove more shifts for sign/zero extension)
| +Author:     Luis Machado <luis.machado@linaro.org>
| +AuthorDate: 2019-12-11 11:55:49 -0300
| +Commit:     Luis Machado <luis.machado@linaro.org>
| +CommitDate: 2019-12-11 12:16:22 -0300
| +
| +Fix unused function error
| +
| +Attempting to build GDB in Ubuntu 18.04.3 on x86_64, i ran into warnings
| +that caused the build to fail. For some reason the diagnostics macros don't
| +work for my case, but ATTRIBUTE_UNUSED does.

PS2, Line 11:

It looks like this:

binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char*
select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-
function]  select_strerror_r (char *res, char *)

I reported a GCC version of 7.4.0, but in reality the box runs GCC
5.4.0.

As for the DIAGNOSTIC_IGNORE_UNUSED_FUNCTION macro, I gave this a try
and it seems the macros do get expanded, but they still have no effect
on the warning.

I replaced the macros with the pragmas to make sure. Still doesn't
work.

| +
| +The compiler version is gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0.
| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
| +	(select_strerror_r): Use ATTRIBUTED_UNUSED instead of the diagnostics

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 2
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 17:05:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v3] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (2 preceding siblings ...)
  2019-12-11 17:05 ` Luis Machado (Code Review)
@ 2019-12-11 17:07 ` Luis Machado (Code Review)
  2019-12-11 17:11 ` [review v4] " Luis Machado (Code Review)
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-11 17:07 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves

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

Fix unused function error

Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
that caused the build to fail:

binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)

The diagnostics macros seem to expand correctly to their respective pragmas,
but it doesn't seem to have an effect on the warning. I tried to use the
pragmas explicitly and got the same result.

ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
which should fix warnings for both gdb and gdbserver builds.

The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

gdb/ChangeLog:

2019-12-11  Luis Machado  <luis.machado@linaro.org>

	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
	macros.

Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
---
M gdb/gdbsupport/safe-strerror.c
1 file changed, 2 insertions(+), 7 deletions(-)



diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c
index 9973fa6..9126507 100644
--- a/gdb/gdbsupport/safe-strerror.c
+++ b/gdb/gdbsupport/safe-strerror.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "common-defs.h"
-#include "diagnostics.h"
 #include <string.h>
 
 /* There are two different versions of strerror_r; one is GNU-specific, the
@@ -29,25 +28,21 @@
 
 /* We only ever use one of the two overloads, so suppress the warning for
    an unused function.  */
-DIAGNOSTIC_PUSH
-DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
 
 /* Called if we have a XSI-compliant strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (int res, char *buf)
 {
   return res == 0 ? buf : nullptr;
 }
 
 /* Called if we have a GNU strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (char *res, char *)
 {
   return res;
 }
 
-DIAGNOSTIC_POP
-
 /* Implementation of safe_strerror as defined in common-utils.h.  */
 
 const char *

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 3
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v4] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (3 preceding siblings ...)
  2019-12-11 17:07 ` [review v3] " Luis Machado (Code Review)
@ 2019-12-11 17:11 ` Luis Machado (Code Review)
  2019-12-11 17:17 ` [review v3] " Pedro Alves (Code Review)
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-11 17:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves

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

Fix unused function error

Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
that caused the build to fail:

binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)

The diagnostics macros seem to expand correctly to their respective pragmas,
but it doesn't seem to have an effect on the warning. I tried to use the
pragmas explicitly and got the same result.

ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
which should fix warnings for both gdb and gdbserver builds.

The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

gdb/ChangeLog:

2019-12-11  Luis Machado  <luis.machado@linaro.org>

	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
	macros.

Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
---
M gdb/gdbsupport/safe-strerror.c
1 file changed, 2 insertions(+), 10 deletions(-)



diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c
index 9973fa6..a5ddf74 100644
--- a/gdb/gdbsupport/safe-strerror.c
+++ b/gdb/gdbsupport/safe-strerror.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "common-defs.h"
-#include "diagnostics.h"
 #include <string.h>
 
 /* There are two different versions of strerror_r; one is GNU-specific, the
@@ -27,27 +26,20 @@
    to solve this for us because IPA does not use Gnulib but uses this
    function.  */
 
-/* We only ever use one of the two overloads, so suppress the warning for
-   an unused function.  */
-DIAGNOSTIC_PUSH
-DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-
 /* Called if we have a XSI-compliant strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (int res, char *buf)
 {
   return res == 0 ? buf : nullptr;
 }
 
 /* Called if we have a GNU strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (char *res, char *)
 {
   return res;
 }
 
-DIAGNOSTIC_POP
-
 /* Implementation of safe_strerror as defined in common-utils.h.  */
 
 const char *

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v3] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (4 preceding siblings ...)
  2019-12-11 17:11 ` [review v4] " Luis Machado (Code Review)
@ 2019-12-11 17:17 ` Pedro Alves (Code Review)
  2019-12-11 17:26 ` [review v4] " Christian Biesinger (Code Review)
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-11 17:17 UTC (permalink / raw)
  To: Luis Machado, gdb-patches

Pedro Alves has posted comments on this change.

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


Patch Set 3:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,30 @@ 
| +Parent:     1d61b032 (Remove more shifts for sign/zero extension)
| +Author:     Luis Machado <luis.machado@linaro.org>
| +AuthorDate: 2019-12-11 11:55:49 -0300
| +Commit:     Luis Machado <luis.machado@linaro.org>
| +CommitDate: 2019-12-11 14:06:52 -0300
| +
| +Fix unused function error
| +
| +Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
| +that caused the build to fail:
| +
| +binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)
| +
| +The diagnostics macros seem to expand correctly to their respective pragmas,
| +but it doesn't seem to have an effect on the warning. I tried to use the
| +pragmas explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

PS3, Line 21:

If this is legitimately a GCC bug, then I see two choices:

#1 - adjust to DIAGNOSTIC_IGNORE_UNUSED_FUNCTION to ignore "-Wunused"
instead of "-Wunused-function" for buggy gcc versions, assuming that
works.

#2 - switch to ATTRIBUTE_UNUSED like you have, but then we should nuke
DIAGNOSTIC_IGNORE_UNUSED_FUNCTION, since it doesn't work as
advertised.

ATTRIBUTE_UNUSED on the function like you have seems simpler for not
having to push/pop, aka not requiring global context.  So I'm fine
with option #2.

What doesn't look good to me is working around one case but letting
the bug latent for another user of DIAGNOSTIC_IGNORE_UNUSED_FUNCTION.
There's no other user of DIAGNOSTIC_IGNORE_UNUSED_FUNCTION currently,
AFAICS, so #2 should be trivial.

| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 3
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 17:17:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v4] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (5 preceding siblings ...)
  2019-12-11 17:17 ` [review v3] " Pedro Alves (Code Review)
@ 2019-12-11 17:26 ` Christian Biesinger (Code Review)
  2019-12-11 17:58 ` Luis Machado (Code Review)
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-12-11 17:26 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: Christian Biesinger, Pedro Alves

Christian Biesinger has posted comments on this change.

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


Patch Set 4:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,30 @@ 
| +Parent:     1d61b032 (Remove more shifts for sign/zero extension)
| +Author:     Luis Machado <luis.machado@linaro.org>
| +AuthorDate: 2019-12-11 11:55:49 -0300
| +Commit:     Luis Machado <luis.machado@linaro.org>
| +CommitDate: 2019-12-11 14:06:52 -0300
| +
| +Fix unused function error
| +
| +Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
| +that caused the build to fail:
| +
| +binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)
| +
| +The diagnostics macros seem to expand correctly to their respective pragmas,
| +but it doesn't seem to have an effect on the warning. I tried to use the
| +pragmas explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

PS3, Line 21:

As a sidenote, ARI currently doesn't like ATTRIBUTE_UNUSED but tromey
is removing that in https://gnutoolchain-gerrit.osci.io/r/c/binutils-
gdb/+/742

| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 17:26:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v4] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (6 preceding siblings ...)
  2019-12-11 17:26 ` [review v4] " Christian Biesinger (Code Review)
@ 2019-12-11 17:58 ` Luis Machado (Code Review)
  2019-12-11 18:03 ` Pedro Alves (Code Review)
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-11 17:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger, Pedro Alves

Luis Machado has posted comments on this change.

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


Patch Set 4:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,30 @@ 
| +Parent:     1d61b032 (Remove more shifts for sign/zero extension)
| +Author:     Luis Machado <luis.machado@linaro.org>
| +AuthorDate: 2019-12-11 11:55:49 -0300
| +Commit:     Luis Machado <luis.machado@linaro.org>
| +CommitDate: 2019-12-11 14:06:52 -0300
| +
| +Fix unused function error
| +
| +Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
| +that caused the build to fail:
| +
| +binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)
| +
| +The diagnostics macros seem to expand correctly to their respective pragmas,
| +but it doesn't seem to have an effect on the warning. I tried to use the
| +pragmas explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

PS3, Line 21:

I tried #1 with "-Wunused" and it doesn't work.

I'm trying to find some more information on when exactly this ceased
to be a problem (if at all). I take it this was fixed, since Christian
didn't see this problem, neither did the buildbot after the first fix.

The warnings in include/diagnostics.h tell us to use these macros
guarded by known versions, like so:

#if __GNUC__ >= 10
#  pragma GCC diagnostic push
#  pragma GCC diagnostic ignored "-Wformat-diag"
#endif

We could have a construct like the above, guarded for known-to-work
compiler versions, and use ATTRIBUTE_UNUSED for the rest.

| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 17:58:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Comment-In-Reply-To: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v4] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (7 preceding siblings ...)
  2019-12-11 17:58 ` Luis Machado (Code Review)
@ 2019-12-11 18:03 ` Pedro Alves (Code Review)
  2019-12-11 18:19 ` Luis Machado (Code Review)
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-11 18:03 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: Christian Biesinger

Pedro Alves has posted comments on this change.

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


Patch Set 4:

(1 comment)

> Patch Set 4:
> 
> (1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,30 @@ 
| +Parent:     1d61b032 (Remove more shifts for sign/zero extension)
| +Author:     Luis Machado <luis.machado@linaro.org>
| +AuthorDate: 2019-12-11 11:55:49 -0300
| +Commit:     Luis Machado <luis.machado@linaro.org>
| +CommitDate: 2019-12-11 14:06:52 -0300
| +
| +Fix unused function error
| +
| +Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
| +that caused the build to fail:
| +
| +binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)
| +
| +The diagnostics macros seem to expand correctly to their respective pragmas,
| +but it doesn't seem to have an effect on the warning. I tried to use the
| +pragmas explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

PS3, Line 21:

> We could have a construct like the above, guarded for known-to-work compiler versions, and use ATTRIBUTE_UNUSED for the rest.

Can't see how that is better than the option #2 I gave?

| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 18:03:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Luis Machado <luis.machado@linaro.org>
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Comment-In-Reply-To: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v4] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (8 preceding siblings ...)
  2019-12-11 18:03 ` Pedro Alves (Code Review)
@ 2019-12-11 18:19 ` Luis Machado (Code Review)
  2019-12-11 18:23 ` Pedro Alves (Code Review)
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-11 18:19 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger, Pedro Alves

Luis Machado has posted comments on this change.

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


Patch Set 4:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,30 @@ 
| +Parent:     1d61b032 (Remove more shifts for sign/zero extension)
| +Author:     Luis Machado <luis.machado@linaro.org>
| +AuthorDate: 2019-12-11 11:55:49 -0300
| +Commit:     Luis Machado <luis.machado@linaro.org>
| +CommitDate: 2019-12-11 14:06:52 -0300
| +
| +Fix unused function error
| +
| +Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
| +that caused the build to fail:
| +
| +binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)
| +
| +The diagnostics macros seem to expand correctly to their respective pragmas,
| +but it doesn't seem to have an effect on the warning. I tried to use the
| +pragmas explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

PS3, Line 21:

Ok. Let's go with #2 then.

I actually found the PR and the fix here:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64079

Should we at least put a comment in place in case someone tries to put
this particular macro up again?

| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 18:19:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Luis Machado <luis.machado@linaro.org>
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Comment-In-Reply-To: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v4] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (9 preceding siblings ...)
  2019-12-11 18:19 ` Luis Machado (Code Review)
@ 2019-12-11 18:23 ` Pedro Alves (Code Review)
  2019-12-11 18:25 ` Pedro Alves (Code Review)
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-11 18:23 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: Christian Biesinger

Pedro Alves has posted comments on this change.

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


Patch Set 4:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,30 @@ 
| +Parent:     1d61b032 (Remove more shifts for sign/zero extension)
| +Author:     Luis Machado <luis.machado@linaro.org>
| +AuthorDate: 2019-12-11 11:55:49 -0300
| +Commit:     Luis Machado <luis.machado@linaro.org>
| +CommitDate: 2019-12-11 14:06:52 -0300
| +
| +Fix unused function error
| +
| +Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
| +that caused the build to fail:
| +
| +binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)
| +
| +The diagnostics macros seem to expand correctly to their respective pragmas,
| +but it doesn't seem to have an effect on the warning. I tried to use the
| +pragmas explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

PS3, Line 21:

> I actually found the PR and the fix here:

Thanks for digging.

> Should we at least put a comment in place in case someone tries to put this particular macro up again?

Sure.

| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h
| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 18:23:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Luis Machado <luis.machado@linaro.org>
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Comment-In-Reply-To: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v4] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (10 preceding siblings ...)
  2019-12-11 18:23 ` Pedro Alves (Code Review)
@ 2019-12-11 18:25 ` Pedro Alves (Code Review)
  2019-12-11 19:28 ` [review v5] " Luis Machado (Code Review)
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-11 18:25 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: Christian Biesinger

Pedro Alves has posted comments on this change.

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


Patch Set 4:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +17,15 @@ pragmas explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case, if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.
| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb/gdbsupport/safe-strerror.c: Remove diagnostics.h

PS4, Line 27:

Drop the leading "gdb/".

Missing period.

But while at it, say "Don't include diagnostics.h.".
"remove foo.h" sounds like you're deleting the file to me. :-)

| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +
| +Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 18:25:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v5] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (11 preceding siblings ...)
  2019-12-11 18:25 ` Pedro Alves (Code Review)
@ 2019-12-11 19:28 ` Luis Machado (Code Review)
  2019-12-11 19:54 ` Pedro Alves (Code Review)
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-11 19:28 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves, Christian Biesinger

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

Fix unused function error

Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
that caused the build to fail:

binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)

The diagnostic macro DIAGNOSTIC_IGNORE_UNUSED_FUNCTION seems to expand
correctly to its respective pragma, but this doesn't seem to have an effect on
the warning. I tried to use the pragma explicitly and got the same result.

ATTRIBUTE_UNUSED works fine in this case if you put it in both functions,
which should fix warnings for both gdb and gdbserver builds.

The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

This is likely the result of PR64079 in GCC, which was fixed by commit
9e96f1e1b9731c4e1ef4fbbbf0997319973f0537.

To prevent other developers from attempting to use this macro, only to get
confused by it not working as expected, it seems better to not define this
particular macro when the compiler is GCC.

gdb/ChangeLog:

2019-12-11  Luis Machado  <luis.machado@linaro.org>

	* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
	macros.

ChangeLog:

2019-12-11  Luis Machado  <luis.machado@linaro.org>

	* include/diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_FUNCTION). Remove
	definition and add comment explaining why.

Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
---
M gdb/gdbsupport/safe-strerror.c
M include/diagnostics.h
2 files changed, 7 insertions(+), 12 deletions(-)



diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c
index 9973fa6..a5ddf74 100644
--- a/gdb/gdbsupport/safe-strerror.c
+++ b/gdb/gdbsupport/safe-strerror.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "common-defs.h"
-#include "diagnostics.h"
 #include <string.h>
 
 /* There are two different versions of strerror_r; one is GNU-specific, the
@@ -27,27 +26,20 @@
    to solve this for us because IPA does not use Gnulib but uses this
    function.  */
 
-/* We only ever use one of the two overloads, so suppress the warning for
-   an unused function.  */
-DIAGNOSTIC_PUSH
-DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-
 /* Called if we have a XSI-compliant strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (int res, char *buf)
 {
   return res == 0 ? buf : nullptr;
 }
 
 /* Called if we have a GNU strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (char *res, char *)
 {
   return res;
 }
 
-DIAGNOSTIC_POP
-
 /* Implementation of safe_strerror as defined in common-utils.h.  */
 
 const char *
diff --git a/include/diagnostics.h b/include/diagnostics.h
index 2adaa4d..503eaae 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -65,8 +65,11 @@
 
 #elif defined (__GNUC__) /* GCC */
 
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
-  DIAGNOSTIC_IGNORE ("-Wunused-function")
+/* Before commit 9e96f1e1b9731c4e1ef4fbbbf0997319973f0537, GCC did not
+   silence the -Wunused-function warning through the GCC diagnostic push/pop
+   pragma as it should have.  Therefore we do not define a
+   DIAGNOSTIC_IGNORE_UNUSED_FUNCTION macro here and use ATTRIBUTE_UNUSED
+   locally instead.  */
 
 # define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
   DIAGNOSTIC_IGNORE ("-Wstringop-truncation")

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 5
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v5] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (12 preceding siblings ...)
  2019-12-11 19:28 ` [review v5] " Luis Machado (Code Review)
@ 2019-12-11 19:54 ` Pedro Alves (Code Review)
  2019-12-12 11:14 ` [review v6] " Luis Machado (Code Review)
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-11 19:54 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: Christian Biesinger

Pedro Alves has posted comments on this change.

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


Patch Set 5:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +16,22 @@ correctly to its respective pragma, but this doesn't seem to have an effect on
| +the warning. I tried to use the pragma explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.
| +
| +This is likely the result of PR64079 in GCC, which was fixed by commit
| +9e96f1e1b9731c4e1ef4fbbbf0997319973f0537.
| +
| +To prevent other developers from attempting to use this macro, only to get
| +confused by it not working as expected, it seems better to not define this
| +particular macro when the compiler is GCC.

PS5, Line 28:

No, remove the macro for all compilers.  Eliminate it completely.
There's no point in using it when we can use ATTRIBUTE_UNUSED instead.

| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 5
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 19:54:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v6] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (13 preceding siblings ...)
  2019-12-11 19:54 ` Pedro Alves (Code Review)
@ 2019-12-12 11:14 ` Luis Machado (Code Review)
  2019-12-12 11:15 ` Luis Machado (Code Review)
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-12 11:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves, Christian Biesinger

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

Fix unused function error

Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
that caused the build to fail:

binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)

The diagnostic macro DIAGNOSTIC_IGNORE_UNUSED_FUNCTION seems to expand
correctly to its respective pragma, but this doesn't seem to have an effect on
the warning. I tried to use the pragma explicitly and got the same result.

ATTRIBUTE_UNUSED works fine in this case if you put it in both functions,
which should fix warnings for both gdb and gdbserver builds.

The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

This is likely the result of PR64079 in GCC, which was fixed by commit
9e96f1e1b9731c4e1ef4fbbbf0997319973f0537.

To prevent other developers from attempting to use this macro, only to get
confused by it not working as expected, it seems better to not define this
particular macro.

gdb/ChangeLog:

2019-12-12  Luis Machado  <luis.machado@linaro.org>

	* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
	macros.

ChangeLog:

2019-12-12  Luis Machado  <luis.machado@linaro.org>

	* include/diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_FUNCTION). Remove
	definitions.

Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
---
M gdb/gdbsupport/safe-strerror.c
M include/diagnostics.h
2 files changed, 2 insertions(+), 19 deletions(-)



diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c
index 9973fa6..a5ddf74 100644
--- a/gdb/gdbsupport/safe-strerror.c
+++ b/gdb/gdbsupport/safe-strerror.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "common-defs.h"
-#include "diagnostics.h"
 #include <string.h>
 
 /* There are two different versions of strerror_r; one is GNU-specific, the
@@ -27,27 +26,20 @@
    to solve this for us because IPA does not use Gnulib but uses this
    function.  */
 
-/* We only ever use one of the two overloads, so suppress the warning for
-   an unused function.  */
-DIAGNOSTIC_PUSH
-DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-
 /* Called if we have a XSI-compliant strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (int res, char *buf)
 {
   return res == 0 ? buf : nullptr;
 }
 
 /* Called if we have a GNU strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (char *res, char *)
 {
   return res;
 }
 
-DIAGNOSTIC_POP
-
 /* Implementation of safe_strerror as defined in common-utils.h.  */
 
 const char *
diff --git a/include/diagnostics.h b/include/diagnostics.h
index 2adaa4d..019ade2 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -53,8 +53,6 @@
   DIAGNOSTIC_IGNORE ("-Wdeprecated-declarations")
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER \
   DIAGNOSTIC_IGNORE ("-Wdeprecated-register")
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
-  DIAGNOSTIC_IGNORE ("-Wunused-function")
 # if __has_warning ("-Wenum-compare-switch")
 #  define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES \
    DIAGNOSTIC_IGNORE ("-Wenum-compare-switch")
@@ -65,9 +63,6 @@
 
 #elif defined (__GNUC__) /* GCC */
 
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
-  DIAGNOSTIC_IGNORE ("-Wunused-function")
-
 # define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
   DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
 
@@ -88,10 +83,6 @@
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 #endif
 
-#ifndef DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-#endif
-
 #ifndef DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 # define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 #endif

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 6
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v6] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (14 preceding siblings ...)
  2019-12-12 11:14 ` [review v6] " Luis Machado (Code Review)
@ 2019-12-12 11:15 ` Luis Machado (Code Review)
  2019-12-12 11:20 ` Pedro Alves (Code Review)
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Luis Machado (Code Review) @ 2019-12-12 11:15 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger, Pedro Alves

Luis Machado has posted comments on this change.

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


Patch Set 6:

(1 comment)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +16,22 @@ correctly to its respective pragma, but this doesn't seem to have an effect on
| +the warning. I tried to use the pragma explicitly and got the same result.
| +
| +ATTRIBUTE_UNUSED works fine in this case if you put it in both functions,
| +which should fix warnings for both gdb and gdbserver builds.
| +
| +The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.
| +
| +This is likely the result of PR64079 in GCC, which was fixed by commit
| +9e96f1e1b9731c4e1ef4fbbbf0997319973f0537.
| +
| +To prevent other developers from attempting to use this macro, only to get
| +confused by it not working as expected, it seems better to not define this
| +particular macro when the compiler is GCC.

PS5, Line 28:

Got it. Done in this updated version now.

| +
| +gdb/ChangeLog:
| +
| +2019-12-11  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
| +	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
| +	macros.
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 6
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Thu, 12 Dec 2019 11:15:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [review v6] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (15 preceding siblings ...)
  2019-12-12 11:15 ` Luis Machado (Code Review)
@ 2019-12-12 11:20 ` Pedro Alves (Code Review)
  2019-12-12 12:13 ` [pushed] " Sourceware to Gerrit sync (Code Review)
  2019-12-12 12:13 ` Sourceware to Gerrit sync (Code Review)
  18 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-12 11:20 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: Christian Biesinger

Pedro Alves has posted comments on this change.

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


Patch Set 6: Code-Review+2


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 6
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-Comment-Date: Thu, 12 Dec 2019 11:19:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pushed] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (17 preceding siblings ...)
  2019-12-12 12:13 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-12-12 12:13 ` Sourceware to Gerrit sync (Code Review)
  18 siblings, 0 replies; 20+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-12-12 12:13 UTC (permalink / raw)
  To: Luis Machado, Pedro Alves, gdb-patches; +Cc: Christian Biesinger

The original change was created by Luis Machado.

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

Fix unused function error

Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
that caused the build to fail:

binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)

The diagnostic macro DIAGNOSTIC_IGNORE_UNUSED_FUNCTION seems to expand
correctly to its respective pragma, but this doesn't seem to have an effect on
the warning. I tried to use the pragma explicitly and got the same result.

ATTRIBUTE_UNUSED works fine in this case if you put it in both functions,
which should fix warnings for both gdb and gdbserver builds.

The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

This is likely the result of PR64079 in GCC, which was fixed by commit
9e96f1e1b9731c4e1ef4fbbbf0997319973f0537.

To prevent other developers from attempting to use this macro, only to get
confused by it not working as expected, it seems better to not define this
particular macro.

gdb/ChangeLog:

2019-12-12  Luis Machado  <luis.machado@linaro.org>

	* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
	macros.

include/ChangeLog:

2019-12-12  Luis Machado  <luis.machado@linaro.org>

	* diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_FUNCTION). Remove
	definitions.

Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
---
M gdb/ChangeLog
M gdb/gdbsupport/safe-strerror.c
M include/ChangeLog
M include/diagnostics.h
4 files changed, 13 insertions(+), 19 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c80c540..5ac4f9c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-12-12  Luis Machado  <luis.machado@linaro.org>
+
+	* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
+	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
+	macros.
+
 2019-12-11  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui-win.c (tui_set_win_height_command): Call
diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c
index 9973fa6..a5ddf74 100644
--- a/gdb/gdbsupport/safe-strerror.c
+++ b/gdb/gdbsupport/safe-strerror.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "common-defs.h"
-#include "diagnostics.h"
 #include <string.h>
 
 /* There are two different versions of strerror_r; one is GNU-specific, the
@@ -27,27 +26,20 @@
    to solve this for us because IPA does not use Gnulib but uses this
    function.  */
 
-/* We only ever use one of the two overloads, so suppress the warning for
-   an unused function.  */
-DIAGNOSTIC_PUSH
-DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-
 /* Called if we have a XSI-compliant strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (int res, char *buf)
 {
   return res == 0 ? buf : nullptr;
 }
 
 /* Called if we have a GNU strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (char *res, char *)
 {
   return res;
 }
 
-DIAGNOSTIC_POP
-
 /* Implementation of safe_strerror as defined in common-utils.h.  */
 
 const char *
diff --git a/include/ChangeLog b/include/ChangeLog
index 52cdc04..1444cc9 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@
+2019-12-12  Luis Machado  <luis.machado@linaro.org>
+
+	* diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_FUNCTION). Remove
+	definitions.
+
 2019-12-11  Alan Modra  <amodra@gmail.com>
 
 	* opcode/mmix.h (PUSHGO_INSN_BYTE): Make unsigned.
diff --git a/include/diagnostics.h b/include/diagnostics.h
index 2adaa4d..019ade2 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -53,8 +53,6 @@
   DIAGNOSTIC_IGNORE ("-Wdeprecated-declarations")
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER \
   DIAGNOSTIC_IGNORE ("-Wdeprecated-register")
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
-  DIAGNOSTIC_IGNORE ("-Wunused-function")
 # if __has_warning ("-Wenum-compare-switch")
 #  define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES \
    DIAGNOSTIC_IGNORE ("-Wenum-compare-switch")
@@ -65,9 +63,6 @@
 
 #elif defined (__GNUC__) /* GCC */
 
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
-  DIAGNOSTIC_IGNORE ("-Wunused-function")
-
 # define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
   DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
 
@@ -88,10 +83,6 @@
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 #endif
 
-#ifndef DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-#endif
-
 #ifndef DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 # define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 #endif

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 7
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [pushed] Fix unused function error
  2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
                   ` (16 preceding siblings ...)
  2019-12-12 11:20 ` Pedro Alves (Code Review)
@ 2019-12-12 12:13 ` Sourceware to Gerrit sync (Code Review)
  2019-12-12 12:13 ` Sourceware to Gerrit sync (Code Review)
  18 siblings, 0 replies; 20+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-12-12 12:13 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: Pedro Alves, Christian Biesinger

Sourceware to Gerrit sync has submitted this change.

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

Fix unused function error

Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
that caused the build to fail:

binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function]  select_strerror_r (char *res, char *)

The diagnostic macro DIAGNOSTIC_IGNORE_UNUSED_FUNCTION seems to expand
correctly to its respective pragma, but this doesn't seem to have an effect on
the warning. I tried to use the pragma explicitly and got the same result.

ATTRIBUTE_UNUSED works fine in this case if you put it in both functions,
which should fix warnings for both gdb and gdbserver builds.

The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

This is likely the result of PR64079 in GCC, which was fixed by commit
9e96f1e1b9731c4e1ef4fbbbf0997319973f0537.

To prevent other developers from attempting to use this macro, only to get
confused by it not working as expected, it seems better to not define this
particular macro.

gdb/ChangeLog:

2019-12-12  Luis Machado  <luis.machado@linaro.org>

	* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
	macros.

include/ChangeLog:

2019-12-12  Luis Machado  <luis.machado@linaro.org>

	* diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_FUNCTION). Remove
	definitions.

Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
---
M gdb/ChangeLog
M gdb/gdbsupport/safe-strerror.c
M include/ChangeLog
M include/diagnostics.h
4 files changed, 13 insertions(+), 19 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c80c540..5ac4f9c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-12-12  Luis Machado  <luis.machado@linaro.org>
+
+	* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
+	(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
+	macros.
+
 2019-12-11  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui-win.c (tui_set_win_height_command): Call
diff --git a/gdb/gdbsupport/safe-strerror.c b/gdb/gdbsupport/safe-strerror.c
index 9973fa6..a5ddf74 100644
--- a/gdb/gdbsupport/safe-strerror.c
+++ b/gdb/gdbsupport/safe-strerror.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "common-defs.h"
-#include "diagnostics.h"
 #include <string.h>
 
 /* There are two different versions of strerror_r; one is GNU-specific, the
@@ -27,27 +26,20 @@
    to solve this for us because IPA does not use Gnulib but uses this
    function.  */
 
-/* We only ever use one of the two overloads, so suppress the warning for
-   an unused function.  */
-DIAGNOSTIC_PUSH
-DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-
 /* Called if we have a XSI-compliant strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (int res, char *buf)
 {
   return res == 0 ? buf : nullptr;
 }
 
 /* Called if we have a GNU strerror_r.  */
-static char *
+ATTRIBUTE_UNUSED static char *
 select_strerror_r (char *res, char *)
 {
   return res;
 }
 
-DIAGNOSTIC_POP
-
 /* Implementation of safe_strerror as defined in common-utils.h.  */
 
 const char *
diff --git a/include/ChangeLog b/include/ChangeLog
index 52cdc04..1444cc9 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@
+2019-12-12  Luis Machado  <luis.machado@linaro.org>
+
+	* diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_FUNCTION). Remove
+	definitions.
+
 2019-12-11  Alan Modra  <amodra@gmail.com>
 
 	* opcode/mmix.h (PUSHGO_INSN_BYTE): Make unsigned.
diff --git a/include/diagnostics.h b/include/diagnostics.h
index 2adaa4d..019ade2 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -53,8 +53,6 @@
   DIAGNOSTIC_IGNORE ("-Wdeprecated-declarations")
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER \
   DIAGNOSTIC_IGNORE ("-Wdeprecated-register")
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
-  DIAGNOSTIC_IGNORE ("-Wunused-function")
 # if __has_warning ("-Wenum-compare-switch")
 #  define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES \
    DIAGNOSTIC_IGNORE ("-Wenum-compare-switch")
@@ -65,9 +63,6 @@
 
 #elif defined (__GNUC__) /* GCC */
 
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
-  DIAGNOSTIC_IGNORE ("-Wunused-function")
-
 # define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
   DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
 
@@ -88,10 +83,6 @@
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 #endif
 
-#ifndef DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
-#endif
-
 #ifndef DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 # define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 #endif

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
Gerrit-Change-Number: 753
Gerrit-PatchSet: 7
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: merged

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-12-12 12:13 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 15:00 [review] Fix unused function error Luis Machado (Code Review)
2019-12-11 15:17 ` [review v2] " Luis Machado (Code Review)
2019-12-11 16:05 ` Pedro Alves (Code Review)
2019-12-11 17:05 ` Luis Machado (Code Review)
2019-12-11 17:07 ` [review v3] " Luis Machado (Code Review)
2019-12-11 17:11 ` [review v4] " Luis Machado (Code Review)
2019-12-11 17:17 ` [review v3] " Pedro Alves (Code Review)
2019-12-11 17:26 ` [review v4] " Christian Biesinger (Code Review)
2019-12-11 17:58 ` Luis Machado (Code Review)
2019-12-11 18:03 ` Pedro Alves (Code Review)
2019-12-11 18:19 ` Luis Machado (Code Review)
2019-12-11 18:23 ` Pedro Alves (Code Review)
2019-12-11 18:25 ` Pedro Alves (Code Review)
2019-12-11 19:28 ` [review v5] " Luis Machado (Code Review)
2019-12-11 19:54 ` Pedro Alves (Code Review)
2019-12-12 11:14 ` [review v6] " Luis Machado (Code Review)
2019-12-12 11:15 ` Luis Machado (Code Review)
2019-12-12 11:20 ` Pedro Alves (Code Review)
2019-12-12 12:13 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-12-12 12:13 ` Sourceware to Gerrit sync (Code Review)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).