Re: [patch #3346] GNUMach: ifdef DEBUG -> ifndef NDEBUG

[Guillem Jover]

Hi,

On Tue, Sep 07, 2004 at 10:35:11AM -0400, Neal H. Walfield wrote:
> [patch #3346] Full Item Snapshot:
> 
> URL: <http://savannah.gnu.org/patch/?func=detailitem&item_id=3346>
> Project: The GNU Hurd
> Submitted by: Neal H. Walfield
> On: Tue 09/07/04 at 14:29

> Category:  GNU Mach

> Summary:  GNUMach: ifdef DEBUG -> ifndef NDEBUG
> 
> Original Submission:
> I think it would be useful to turn asserts on when building GNU Mach.
> One way to go about doing this would be to define DEBUG when
> building.  I think it might be useful, however, to be more like the
> posix interface and only disable asserts when NDEBUG is defined.

I've changed the patch, it enabled asserts by default, and they can be
disabled with NDEBUG (as Neal proposed), but all other stuff keeps
being under DEBUG control, so if you want to enable them just define
DEBUG. Current Debian packages are using that later patch and work
with asserts enabled by default, without outputting the previous
amount of data (patch attached at the end).

> This also helps get rid of some of the warnings:
> 
>         warning: extra tokens at end of #endif directive
> 
> It also includes a small patch which fixes a bad assert (basically
> suggesting that no one has compiled GNU Mach with asserts turned on
> for the past few years).

I found that, and removed that assert. Your original patch didn't
include that fix. Your patch on savannah is missing some files, or
the ChangeLog entry does not correspond to the patch.

> I used the attached shell script to automate the changes, however, I
> have verified all of the changes by hand and I have compiled the code
> (at least using my configuration which does not include all of the
> changed files).

> for i in $(find . -type f -name '*.[chS]' -print0 
>                | xargs -0 grep -E -l -e 'bDEBUGb')
> do
>   sed 
>     -e 
> 's/^(#[[:blank:]]*if[[:blank:]]*!?[[:blank:]]*)(DEBUG[[:blank:]]*)$/1!N2/g' 
>     -e 
> 's/^(#[[:blank:]]*if[[:blank:]]*!?[[:blank:]]*defined[[:blank:]]*([[:blank:]]*)DEBUG)/1!NDEBUG)/g'
>  
>     -e 's/^(#[[:blank:]]*if)(def[[:blank:]]*!?)(DEBUG[[:blank:]]*)$/1n2N3/g' 
>     -e 
> 's/^(#[[:blank:]]*(endif|else)[[:blank:]]+)(/*)?([[:blank:]]*!?)DEBUG([[:blank:]]**/)?[[:blank:]]*$/1
>  /* NDEBUG *//g' < $i > $i.tmp && mv $i.tmp $i
> done

I found this code needed a -r option for sed (at least for me).

regards,
guillem


2004-11-22  Guillem Jover  <guillem@hadrons.org>

        * bogus/mach_assert.h: Change #ifdef DEBUG to #ifndef NDEBUG
        for assert et al.
        * kern/assert.h: Likewise.
        * kern/debug.h: Likewise.
        * util/debug.h: Likewise.

        * i386/i386/debug.h: Move dump_ss definition out of [DEBUG].
        * linux/dev/glue/block.c (rdwr_full): Remove invalid assert.


diff -Naur bogus/mach_assert.h bogus/mach_assert.h
--- bogus/mach_assert.h 1997-02-25 22:28:02.000000000 +0100
+++ bogus/mach_assert.h 2004-11-22 02:27:51.000000000 +0100
@@ -1,4 +1,4 @@
-#ifdef DEBUG
+#ifndef NDEBUG
 #define MACH_ASSERT 1
 #else
 #define MACH_ASSERT 0
diff -Naur i386/i386/debug.h i386/i386/debug.h
--- i386/i386/debug.h   2001-04-05 08:39:20.000000000 +0200
+++ i386/i386/debug.h   2004-11-22 02:56:11.000000000 +0100
@@ -23,6 +23,10 @@
 #ifndef _I386_DEBUG_
 #define _I386_DEBUG_
 
+/* Dump a saved state.
+   Probably a good idea to have this around
+   even when DEBUG isn't turned on.  */
+void dump_ss(struct i386_saved_state *st);
 
 #ifdef DEBUG
 
@@ -37,11 +41,6 @@
    and all registers are saved.  */
 #ifndef ASSEMBLER
 
-/* Dump a saved state.
-   Probably a good idea to have this around
-   even when DEBUG isn't turned on.  */
-void dump_ss(struct i386_saved_state *st);
-
 #define DEBUG_TRACE _debug_trace(__FILE__,__LINE__)
 
 /* Reset the debug trace buffer so it contains no valid entries.  */
diff -Naur kern/assert.h kern/assert.h
--- kern/assert.h       2001-04-05 08:39:20.000000000 +0200
+++ kern/assert.h       2004-11-22 02:35:08.000000000 +0100
@@ -31,7 +31,7 @@
 
 #include <kern/macro_help.h>
 
-#ifdef DEBUG
+#ifndef NDEBUG
 #define MACH_ASSERT 1
 #endif
 
diff -Naur kern/debug.h kern/debug.h
--- kern/debug.h        2001-04-05 08:39:20.000000000 +0200
+++ kern/debug.h        2004-11-22 02:37:00.000000000 +0100
@@ -30,7 +30,7 @@
 
 #include <kern/assert.h> /*XXX*/
 
-#ifdef DEBUG
+#ifndef NDEBUG
 
 #define here() printf("@ %s:%d\n", __FILE__, __LINE__)
 #define message(args) ({ printf("@ %s:%d: ", __FILE__, __LINE__); printf args; 
printf("\n"); })
@@ -46,7 +46,7 @@
                        __FILE__, __LINE__, (p), (id), (p->struct_id)); \
        })
 
-#else /* !DEBUG */
+#else /* NDEBUG */
 
 #define otsan()
 
@@ -55,6 +55,6 @@
 #define struct_id_denit(p)
 #define struct_id_verify(p,id)
 
-#endif /* !DEBUG */
+#endif /* NDEBUG */
 
 #endif /* _mach_debug__debug_ */
diff -Naur util/debug.h util/debug.h
--- util/debug.h        1997-02-25 22:28:35.000000000 +0100
+++ util/debug.h        2004-11-22 02:33:56.000000000 +0100
@@ -34,7 +34,7 @@
 #include <mach/macro_help.h>
 
 
-#ifdef DEBUG
+#ifndef NDEBUG
 
 extern void panic(const char *fmt, ...);
 
@@ -64,7 +64,7 @@
                        __FILE__, __LINE__, (p), (id), (p->struct_id)); \
        })
 
-#else !DEBUG
+#else /* NDEBUG */
 
 #define otsan()
 #define assert(v)
@@ -76,6 +76,6 @@
 #define struct_id_denit(p)
 #define struct_id_verify(p,id)
 
-#endif !DEBUG
+#endif /* NDEBUG */
 
 #endif _MACH_UTIL_DEBUG_H_
diff -Naur linux/dev/glue/block.c linux/dev/glue/block.c
--- linux/dev/glue/block.c      2004-01-19 02:44:31.000000000 +0100
+++ linux/dev/glue/block.c      2004-11-22 03:26:58.000000000 +0100
@@ -595,7 +595,6 @@
   struct buffer_head bhead[MAX_BUF], *bh, *bhp[MAX_BUF];
 
   assert ((*off & BMASK) == 0);
-  assert (*resid >= bsize);
 
   nbuf = *resid >> bshift;
   blk = *off >> bshift;