[0/1] Fix some warnings in the public headers

Message ID 20210411103126.714706-1-rdiezmail-newlib@yahoo.de
State New
Headers show

Commit Message

R. Diez via Newlib April 11, 2021, 10:31 a.m.
Hi all:

I am experimenting with Newlib and I am not using option -isystem <path>, but the normal -I<path>, so I get to see more compilation warnings than usual.

Maybe I am seeing more warnings because I am building in C++ mode.

I have prepared a patch to fix those warnings, see below. Notes about the patch are:

About __ARM_FP:

The documentation for __ARM_FP states "Set if hardware floating-point is available", so I was getting an "is not defined, evaluates to 0 [-Wundef]" warning.


About _FORTIFY_SOURCE:

The GCC manual states "when the _FORTIFY_SOURCE macro is defined to a non-zero value", and that symbol was not defined in my build, so I was getting an "is not defined, evaluates to 0 [-Wundef]" warning.


About __assert_func():

I was getting this warning:

redundant redeclaration of 'void __assert_func(const char*, int, const char*, const char*)' in same scope [-Wredundant-decls]


About __STDC_VERSION__:

I was getting an "is not defined, evaluates to 0 [-Wundef]" warning when compiling in C++ mode.


About _sig_func:

I was getting this warning:

 warning: unnecessary parentheses in declaration of '_sig_func' [-Wparentheses]
 

Thanks in advance,
  rdiez


R. Diez (1):
  Fix some compilation warnings.

 newlib/libc/include/assert.h         | 5 ++++-
 newlib/libc/include/machine/ieeefp.h | 2 +-
 newlib/libc/include/sys/features.h   | 2 +-
 newlib/libc/include/sys/reent.h      | 4 ++--
 4 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.31.1

From dfb65e9d0b3e3335318e3cf85828ea4a14e252f0 Mon Sep 17 00:00:00 2001
From: "R. Diez" <rdiezmail-newlib@yahoo.de>

Date: Sun, 11 Apr 2021 11:48:02 +0200
Subject: [PATCH 1/1] Fix some compilation warnings.

Signed-off-by: R. Diez <rdiezmail-newlib@yahoo.de>

---
 newlib/libc/include/assert.h         | 5 ++++-
 newlib/libc/include/machine/ieeefp.h | 2 +-
 newlib/libc/include/sys/features.h   | 2 +-
 newlib/libc/include/sys/reent.h      | 4 ++--
 4 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.31.1

Comments

Brian Inglis April 11, 2021, 3:12 p.m. | #1
On 2021-04-11 04:31, R. Diez via Newlib wrote:
> I am experimenting with Newlib and I am not using option -isystem <path>, but the normal -I<path>, so I get to see more compilation warnings than usual.

> Maybe I am seeing more warnings because I am building in C++ mode.


What do you mean by C++ mode?
Perhaps you should be using C++ alternatives to these features and headers, or 
using them from a C wrapper module?

> I have prepared a patch to fix those warnings, see below. Notes about the patch are:

> About __ARM_FP:

> The documentation for __ARM_FP states "Set if hardware floating-point is available", so I was getting an "is not defined, evaluates to 0 [-Wundef]" warning.

> About _FORTIFY_SOURCE:

> The GCC manual states "when the _FORTIFY_SOURCE macro is defined to a non-zero value", and that symbol was not defined in my build, so I was getting an "is not defined, evaluates to 0 [-Wundef]" warning.

> About __assert_func():

> I was getting this warning:

> redundant redeclaration of 'void __assert_func(const char*, int, const char*, const char*)' in same scope [-Wredundant-decls]

> About __STDC_VERSION__:

> I was getting an "is not defined, evaluates to 0 [-Wundef]" warning when compiling in C++ mode.

> About _sig_func:

> I was getting this warning:

>   warning: unnecessary parentheses in declaration of '_sig_func' [-Wparentheses]


> R. Diez (1):

>    Fix some compilation warnings.

> 

>   newlib/libc/include/assert.h         | 5 ++++-

>   newlib/libc/include/machine/ieeefp.h | 2 +-

>   newlib/libc/include/sys/features.h   | 2 +-

>   newlib/libc/include/sys/reent.h      | 4 ++--

>   4 files changed, 8 insertions(+), 5 deletions(-)


Programs and builds really should always expect undefined macro variables, as 
much code only cares whether the variable is defined, and not the value - 
anything but 1 is just as good as 1.
Extra parentheses are often necessary in macros around arguments, call a 
function and not a macro, ensure operation priority is correct, and as a defense 
against careless changes or definitions.

The solution here is to suppress the warnings while you are including C headers 
in your C++ application code using:

	#pragma GCC diagnostic push
	#pragma GCC diagnostic ignored "-Wundef"
	#pragma GCC diagnostic ignored "-Wparentheses"
	#pragma GCC diagnostic ignored "-Wredundant-decls" /* optionally */

	#include ...
	...

	#pragma GCC diagnostic pop

or the equivalent in your compiler.

Beware of allowed redefinitions in assert.h(0p):

"The <assert.h> header shall define the assert() macro. It refers to the macro 
NDEBUG which is not defined in the header. If NDEBUG is defined as a macro name 
before the inclusion of this header, the assert() macro shall be defined simply as:

	#define assert(ignore)((void) 0)

Otherwise, the macro behaves as described in assert().
The assert() macro shall be redefined according to the current state of NDEBUG 
each time <assert.h> is included."

In this file, __ASSERT_FUNC appears to be your first time flag, as it is only 
defined if NDEBUG is ever undefined and and the functions are needed, so you 
should be able to move the function declarations just before:

	# endif /* !__ASSERT_FUNC */
	#endif /* !NDEBUG */

Why are you swapping the && conditions in the test for static_assert?
If it's not causing a problem, don't touch it, there may well be good reasons 
for the order, that you are not testing.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]

Patch

diff --git a/newlib/libc/include/assert.h b/newlib/libc/include/assert.h
index b9e5e9b4a..a15e7fbae 100644
--- a/newlib/libc/include/assert.h
+++ b/newlib/libc/include/assert.h
@@ -36,12 +36,15 @@  extern "C" {
 # endif /* !__ASSERT_FUNC */
 #endif /* !NDEBUG */
 
+#ifndef __ASSERT_WAS_DECLARED  /* Prevent "redundant redeclaration" warning when including this file multiple times. */
+#define __ASSERT_WAS_DECLARED
 void __assert (const char *, int, const char *)
 	    _ATTRIBUTE ((__noreturn__));
 void __assert_func (const char *, int, const char *, const char *)
 	    _ATTRIBUTE ((__noreturn__));
+#endif /* #ifndef __ASSERT_WAS_DECLARED */
 
-#if __STDC_VERSION__ >= 201112L && !defined __cplusplus
+#if !defined __cplusplus && __STDC_VERSION__ >= 201112L
 # define static_assert _Static_assert
 #endif
 
diff --git a/newlib/libc/include/machine/ieeefp.h b/newlib/libc/include/machine/ieeefp.h
index 3c1f41e03..37f7661cb 100644
--- a/newlib/libc/include/machine/ieeefp.h
+++ b/newlib/libc/include/machine/ieeefp.h
@@ -78,7 +78,7 @@ 
 # else
 #  define __IEEE_BIG_ENDIAN
 # endif
-# if __ARM_FP & 0x8
+# if defined(__ARM_FP) && (__ARM_FP & 0x8)
 #  define __OBSOLETE_MATH_DEFAULT 0
 # endif
 #else
diff --git a/newlib/libc/include/sys/features.h b/newlib/libc/include/sys/features.h
index 218807178..65f5af763 100644
--- a/newlib/libc/include/sys/features.h
+++ b/newlib/libc/include/sys/features.h
@@ -319,7 +319,7 @@  extern "C" {
 #define	__XSI_VISIBLE		0
 #endif
 
-#if _FORTIFY_SOURCE > 0 && !defined(__cplusplus) && !defined(__lint__) && \
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0 && !defined(__cplusplus) && !defined(__lint__) && \
    (__OPTIMIZE__ > 0 || defined(__clang__)) && __GNUC_PREREQ__(4, 1)
 #  if _FORTIFY_SOURCE > 1
 #    define __SSP_FORTIFY_LEVEL 2
diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index 74b70e9c0..f62e5817b 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -409,7 +409,7 @@  struct _reent
   char *_asctime_buf;
 
   /* signal info */
-  void (**(_sig_func))(int);
+  void (**_sig_func)(int);
 
 # ifndef _REENT_GLOBAL_ATEXIT
   /* atexit stuff */
@@ -682,7 +682,7 @@  struct _reent
 # endif
 
   /* signal info */
-  void (**(_sig_func))(int);
+  void (**_sig_func)(int);
 
   /* These are here last so that __FILE can grow without changing the offsets
      of the above members (on the off chance that future binary compatibility