diff mbox

Use of #if DEBUG in reent.h

Message ID 210267905.11859124.1484004324939.JavaMail.zimbra@redhat.com
State New
Headers show

Commit Message

Jeff Johnston Jan. 9, 2017, 11:25 p.m. UTC 
Patch attached.  I will check in if no objections.  It was only used for
debugging the _REENT_CHECK macros so I renamed it _REENT_CHECK_DEBUG.

-- Jeff J.

----- Original Message -----
> Hmm, I appear to have put that code in ages ago (2008) because it originally

> had asserts in

> place by default from code changes in 2002.  Not sure anyone ever used this.

> 

> I think the choice of DEBUG was unfortunate and non-standard so, yes,

> I think renaming it makes sense with at least an under-score.  The flag was

> meant to be a switch, opposite to NDEBUG to turn on use of the asserts so

> #ifdef should be used.  I will post a patch shortly.

> 

> -- Jeff J.

> 

> ----- Original Message -----

> > Jeff?

> > 

> > Thanks,

> > Corinna

> > 

> > On Jan  3 16:41, Joe Seymour wrote:

> > > Users can enable assertions in newlib/libc/include/sys/reent.h by

> > > defining the macro DEBUG. As DEBUG is a relatively obvious and general

> > > purpose name, it is also used by other projects. I've seen it used by

> > > the Atmel Software Framework (ASF) and some TI Energia projects.  A web

> > > search finds a few more examples:

> > > 

> > > https://forum.pjrc.com/threads/19916-reent-h-space-problems-or-just-my-pc

> > > http://forum.arduino.cc/index.php?topic=151655.0

> > > 

> > > Actually, these are all examples of projects that have been seen to fail

> > > to build with the following error:

> > > 

> > > > include\sys\reent.h:458:10: error: #if with no expression

> > > 

> > > This occurs when DEBUG is defined but has no value. Which seems to

> > > happen with the following test case:

> > > 

> > > > #define DEBUG

> > > > #include <sys/reent.h>

> > > 

> > > Notably, I don't see an error if I remove the #define and instead pass

> > > -DDEBUG (with no value) to my gcc build [msp430-elf-gcc (GCC) 7.0.0

> > > 20161220].

> > > 

> > > My initial reaction to seeing the error message was that this was a bug

> > > in reent.h, which should use #ifdef DEBUG instead. Unfortunately if you

> > > do that then "#define DEBUG 0" and -DDEBUG=0 result in debug being

> > > turned on, which seems undesirable. This leads me to believe that the

> > > use of #if DEBUG is deliberate and not considered a bug?

> > > 

> > > I mention this here because I couldn't find any previous discussion of

> > > it on this list, and as it has been discussed elsewhere I think there's

> > > value in having a definitive statement about it here.

> > > 

> > > Again for the record, I'll ask whether "#if DEBUG" could be changed to

> > > "#if _SYS_REENT_H_DEBUG", or similar? That would avoid the error and the

> > > naming conflict.  I'm assuming that the examples listed above aren't

> > > trying to turn newlib assertions on. I suspect that at the very least

> > > DEBUG has been used for long enough that there's a stronger argument for

> > > keeping it the same than for changing it to something else?

> > > 

> > > Thanks,

> > > 

> > > Joe Seymour

> > 

> > --

> > Corinna Vinschen

> > Cygwin Maintainer

> > Red Hat

> > 

>

Comments

Corinna Vinschen Jan. 10, 2017, 10:51 a.m. UTC | #1 
On Jan  9 18:25, Jeff Johnston wrote:
> Patch attached.  I will check in if no objections.  It was only used for

> debugging the _REENT_CHECK macros so I renamed it _REENT_CHECK_DEBUG.


Looks good to me.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
Jeff Johnston Jan. 10, 2017, 5:20 p.m. UTC | #2 
Ok, done.

-- Jeff J.

----- Original Message -----
> On Jan  9 18:25, Jeff Johnston wrote:

> > Patch attached.  I will check in if no objections.  It was only used for

> > debugging the _REENT_CHECK macros so I renamed it _REENT_CHECK_DEBUG.

> 

> Looks good to me.

> 

> 

> Thanks,

> Corinna

> 

> --

> Corinna Vinschen

> Cygwin Maintainer

> Red Hat

>
diff mbox

Patch

From 81b770557b3390a4cac631a9662e56a553b97305 Mon Sep 17 00:00:00 2001
From: Jeff Johnston <jjohnstn@redhat.com>
Date: Mon, 9 Jan 2017 18:21:19 -0500
Subject: [PATCH] Fix sys/reent.h to remove use of DEBUG flag.

- use of DEBUG flag is non-standard and interferes with other
  project's using same flag
- change to be _REENT_CHECK_DEBUG which means the flag is
  allowing debugging of _REENT_CHECK macros
- use #ifdef instead of #if
---
 newlib/libc/include/sys/reent.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h
index e1ed8b4..8b67889 100644
--- a/newlib/libc/include/sys/reent.h
+++ b/newlib/libc/include/sys/reent.h
@@ -454,8 +454,8 @@  extern const struct __sFILE_fake __sf_fake_stderr;
     (var)->_stderr = (__FILE *)&__sf_fake_stderr; \
   }
 
-/* Only built the assert() calls if we are built with debugging.  */
-#if DEBUG
+/* Only add assert() calls if we are specified to debug.  */
+#ifdef _REENT_CHECK_DEBUG
 #include <assert.h>
 #define __reent_assert(x) assert(x)
 #else
-- 
2.5.5