diff mbox

[03/14] net: Convert conditional compilation of debug printfs to regular ifs

Message ID 1398673575-7773-3-git-send-email-marc.mari.barcelo@gmail.com
State New
Headers show

Commit Message

Marc Marí April 28, 2014, 8:26 a.m. UTC 
From: Marc Marí <5.markmb.5@gmail.com>

Modify debug macros as explained in https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg03642.html

Signed-off-by: Marc Marí <5.markmb.5@gmail.com>
---
 hw/net/cadence_gem.c    |   19 ++++++++++++-------
 hw/net/eepro100.c       |   15 ++++++++++-----
 hw/net/lan9118.c        |   27 ++++++++++++++++++---------
 hw/net/spapr_llan.c     |   15 ++++++++++-----
 hw/net/stellaris_enet.c |   24 ++++++++++++++++--------
 hw/net/xgmac.c          |   17 +++++++++++------
 6 files changed, 77 insertions(+), 40 deletions(-)

Comments

Stefan Hajnoczi April 28, 2014, 10:10 a.m. UTC | #1 
On Mon, Apr 28, 2014 at 10:26:04AM +0200, Marc Marí wrote:
>  /* Debug EEPRO100 card. */
>  #if 0
> -# define DEBUG_EEPRO100
> +#define DEBUG_EEPRO100 1
>  #endif

There seem to be a few ways of doing this (commenting out the line,
putting it in #if 0, etc).

Feel free to get rid of #define DEBUG_foo 1 lines completely, leaving
just the #ifndef (I think Kevin suggested this).

Editing the file is bad since it's easy to forget to remove changes when
doing git-commit(1).  Debugging should be enabled by recompiling instead
of editing source files:
$ ./configure --extra-cflags=-DDEBUG_FOO=1 && make

But this is just a personal preference, if you want to leave the lines
it's fine.

Stefan
Marc Marí April 28, 2014, 10:20 a.m. UTC | #2 
I left the original definition as it was, just adding the "1", that's why
there is so much diversity. I'll remove them for v2.


2014-04-28 12:10 GMT+02:00 Stefan Hajnoczi <stefanha@gmail.com>:

> On Mon, Apr 28, 2014 at 10:26:04AM +0200, Marc Marí wrote:
> >  /* Debug EEPRO100 card. */
> >  #if 0
> > -# define DEBUG_EEPRO100
> > +#define DEBUG_EEPRO100 1
> >  #endif
>
> There seem to be a few ways of doing this (commenting out the line,
> putting it in #if 0, etc).
>
> Feel free to get rid of #define DEBUG_foo 1 lines completely, leaving
> just the #ifndef (I think Kevin suggested this).
>
> Editing the file is bad since it's easy to forget to remove changes when
> doing git-commit(1).  Debugging should be enabled by recompiling instead
> of editing source files:
> $ ./configure --extra-cflags=-DDEBUG_FOO=1 && make
>
> But this is just a personal preference, if you want to leave the lines
> it's fine.
>
> Stefan
>
Kevin Wolf April 28, 2014, 11:57 a.m. UTC | #3 
Am 28.04.2014 um 12:10 hat Stefan Hajnoczi geschrieben:
> On Mon, Apr 28, 2014 at 10:26:04AM +0200, Marc Marí wrote:
> >  /* Debug EEPRO100 card. */
> >  #if 0
> > -# define DEBUG_EEPRO100
> > +#define DEBUG_EEPRO100 1
> >  #endif
> 
> There seem to be a few ways of doing this (commenting out the line,
> putting it in #if 0, etc).
> 
> Feel free to get rid of #define DEBUG_foo 1 lines completely, leaving
> just the #ifndef (I think Kevin suggested this).

/me is innocent.

> Editing the file is bad since it's easy to forget to remove changes when
> doing git-commit(1).  Debugging should be enabled by recompiling instead
> of editing source files:
> $ ./configure --extra-cflags=-DDEBUG_FOO=1 && make

Generally I prefer editing the source file and recompiling just that one
file instead of reconfiguring and rebuilding the whole of qemu, so I'm
happy if there is an obvious line to uncomment when I want to enable
debugging.

Kevin
diff mbox

Patch

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index e34b25e..94782a6 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -28,15 +28,20 @@ 
 #include "net/net.h"
 #include "net/checksum.h"
 
-#ifdef CADENCE_GEM_ERR_DEBUG
-#define DB_PRINT(...) do { \
-    fprintf(stderr,  ": %s: ", __func__); \
-    fprintf(stderr, ## __VA_ARGS__); \
-    } while (0);
-#else
-    #define DB_PRINT(...)
+//#define CADENCE_GEM_ERR_DEBUG 1
+
+#ifndef CADENCE_GEM_ERR_DEBUG
+#define CADENCE_GEM_ERR_DEBUG 0
 #endif
 
+#define DB_PRINT(...) \
+    do { \
+        if(CADENCE_GEM_ERR_DEBUG) { \
+            fprintf(stderr,  ": %s: ", __func__); \
+            fprintf(stderr, ## __VA_ARGS__); \
+        } \
+    } while (0);
+
 #define GEM_NWCTRL        (0x00000000/4) /* Network Control reg */
 #define GEM_NWCFG         (0x00000004/4) /* Network Config reg */
 #define GEM_NWSTATUS      (0x00000008/4) /* Network Status reg */
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 3b891ca..f0db5bd 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -62,15 +62,20 @@ 
 
 /* Debug EEPRO100 card. */
 #if 0
-# define DEBUG_EEPRO100
+#define DEBUG_EEPRO100 1
 #endif
 
-#ifdef DEBUG_EEPRO100
-#define logout(fmt, ...) fprintf(stderr, "EE100\t%-24s" fmt, __func__, ## __VA_ARGS__)
-#else
-#define logout(fmt, ...) ((void)0)
+#ifndef DEBUG_EEPRO100
+#define DEBUG_EEPRO100 0
 #endif
 
+#define logout(fmt, ...) \
+    do { \
+        if(DEBUG_EEPRO100) { \
+            fprintf(stderr, "EE100\t%-24s" fmt, __func__, ## __VA_ARGS__); \
+        } \
+    } while(0)
+
 /* Set flags to 0 to disable debug output. */
 #define INT     1       /* interrupt related actions */
 #define MDI     1       /* mdi related actions */
diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index e528290..0d7e2f7 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -18,18 +18,27 @@ 
 /* For crc32 */
 #include <zlib.h>
 
-//#define DEBUG_LAN9118
+//#define DEBUG_LAN9118 1
+
+#ifndef DEBUG_LAN9118
+#define DEBUG_LAN9118 0
+#endif
 
-#ifdef DEBUG_LAN9118
 #define DPRINTF(fmt, ...) \
-do { printf("lan9118: " fmt , ## __VA_ARGS__); } while (0)
-#define BADF(fmt, ...) \
-do { hw_error("lan9118: error: " fmt , ## __VA_ARGS__);} while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while(0)
+    do { \
+        if(DEBUG_LAN9118) { \
+            printf("lan9118: " fmt , ## __VA_ARGS__); \
+        } \
+    } while (0)
+
 #define BADF(fmt, ...) \
-do { fprintf(stderr, "lan9118: error: " fmt , ## __VA_ARGS__);} while (0)
-#endif
+    do { \
+        if(DEBUG_LAN9118) { \
+            hw_error("lan9118: error: " fmt , ## __VA_ARGS__); \
+        }else{ \
+            fprintf(stderr, "lan9118: error: " fmt , ## __VA_ARGS__); \
+        } \
+    } while (0)
 
 #define CSR_ID_REV      0x50
 #define CSR_IRQ_CFG     0x54
diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index c433337..9140ab2 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -36,14 +36,19 @@ 
 #define ETH_ALEN        6
 #define MAX_PACKET_SIZE 65536
 
-/*#define DEBUG*/
+/*#define DEBUG 1*/
 
-#ifdef DEBUG
-#define DPRINTF(fmt...) do { fprintf(stderr, fmt); } while (0)
-#else
-#define DPRINTF(fmt...)
+#ifndef DEBUG
+#define DEBUG 0
 #endif
 
+#define DPRINTF(fmt...) \
+    do { \
+        if(DEBUG) { \
+            fprintf(stderr, fmt); \
+        } \
+    } while (0)
+
 /*
  * Virtual LAN device
  */
diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index d04e6a4..7e24ae8 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -12,16 +12,24 @@ 
 
 //#define DEBUG_STELLARIS_ENET 1
 
-#ifdef DEBUG_STELLARIS_ENET
+#ifndef DEBUG_STELLARIS_ENET
+#define DEBUG_STELLARIS_ENET 0
+#endif
+
 #define DPRINTF(fmt, ...) \
-do { printf("stellaris_enet: " fmt , ## __VA_ARGS__); } while (0)
-#define BADF(fmt, ...) \
-do { fprintf(stderr, "stellaris_enet: error: " fmt , ## __VA_ARGS__); exit(1);} while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while(0)
+    do { \
+        if(DEBUG_STELLARIS_ENET) { \
+            printf("stellaris_enet: " fmt , ## __VA_ARGS__); \
+        } \
+    } while (0)
+
 #define BADF(fmt, ...) \
-do { fprintf(stderr, "stellaris_enet: error: " fmt , ## __VA_ARGS__);} while (0)
-#endif
+    do { \
+        fprintf(stderr, "stellaris_enet: error: " fmt , ## __VA_ARGS__); \
+        if(DEBUG_STELLARIS_ENET) { \
+            exit(1); \
+        } \
+    } while (0)
 
 #define SE_INT_RX       0x01
 #define SE_INT_TXER     0x02
diff --git a/hw/net/xgmac.c b/hw/net/xgmac.c
index 9384fa0..a25c36e 100644
--- a/hw/net/xgmac.c
+++ b/hw/net/xgmac.c
@@ -30,14 +30,19 @@ 
 #include "net/net.h"
 #include "net/checksum.h"
 
-#ifdef DEBUG_XGMAC
-#define DEBUGF_BRK(message, args...) do { \
-                                         fprintf(stderr, (message), ## args); \
-                                     } while (0)
-#else
-#define DEBUGF_BRK(message, args...) do { } while (0)
+//#define DEBUG_XGMAC 1
+
+#ifndef DEBUG_XGMAC
+#define DEBUG_XGMAC 0
 #endif
 
+#define DEBUGF_BRK(message, args...) \
+    do { \
+        if(DEBUG_XGMAC) { \
+            fprintf(stderr, (message), ## args); \
+        } \
+    } while (0)
+
 #define XGMAC_CONTROL           0x00000000   /* MAC Configuration */
 #define XGMAC_FRAME_FILTER      0x00000001   /* MAC Frame Filter */
 #define XGMAC_FLOW_CTRL         0x00000006   /* MAC Flow Control */