| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1291223825.1845.170.camel@Joe-Laptop>
Date: Wed, 01 Dec 2010 09:17:05 -0800
From: Joe Perches <joe@...ches.com>
To: Felix Fietkau <nbd@...nwrt.org>
Cc: "Luis R. Rodriguez" <mcgrof@...il.com>,
linux-wireless <linux-wireless@...r.kernel.org>,
ath9k-devel@...ts.ath9k.org, peter@...ge.se,
linux-kernel@...r.kernel.org
Subject: Re: [ath9k-devel] [PATCH wireless-next] ath: Rename ath_print to
ath_debug
On Wed, 2010-12-01 at 15:37 +0100, Felix Fietkau wrote:
> On 2010-12-01 3:27 PM, Joe Perches wrote:
> > On Tue, 2010-11-30 at 23:56 -0800, Luis R. Rodriguez wrote:
> >> On Tue, Nov 30, 2010 at 12:19 PM, Joe Perches <joe@...ches.com> wrote:
> >> > Poor function naming is just that.
> >> > It reduces readability and the uses are counter expectation.
> >> The name is perfect, we use it to print anything, even non-debugging stuff.
> >
> > 'fraid not.
> >
> > ath/debug.h
> >
> > #ifdef CONFIG_ATH_DEBUG
> > void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...)
> > __attribute__ ((format (printf, 3, 4)));
> > #else
> > static inline void __attribute__ ((format (printf, 3, 4)))
> > ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...)
> > {
> > }
> > #endif /* CONFIG_ATH_DEBUG */
> Now we're getting closer to something worth fixing. IMHO we should
> change the code so that ath_print(common, ATH_DBG_FATAL, ...) prints
> something even with CONFIG_ATH_DEBUG unset. To get this done, some
> renaming would make sense here.
Perhaps the function name is bad after all
if Luis believed it be be always active.
If there are going to be other non-debug uses,
I suggest adding the more standard styles of
ath_printk and ath_<level> similar to
dev_printk and dev_<level> from device.h
Something like this for a start, then a more
gradual rename of ath_print to ath_dbg (or
ath_debug) as the original patch suggested.
This basically removes debug.h leaving
only an #define ath_printk ath_dbg
there and moving all the ATH_DBG_<foo>
enums to ath.h
I do not think there's much purpose for struct
ath_common * being passed to the ath_printk
functions, but perhaps there might be.
Signed-off-by: Joe Perches <joe@...ches.com>
---
drivers/net/wireless/ath/ath.h | 103 ++++++++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/debug.c | 20 -------
drivers/net/wireless/ath/debug.h | 72 +--------------------------
drivers/net/wireless/ath/main.c | 20 +++++++
4 files changed, 124 insertions(+), 91 deletions(-)
diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 26bdbee..5a598b9 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -186,4 +186,107 @@ bool ath_hw_keyreset(struct ath_common *common, u16 entry);
void ath_hw_cycle_counters_update(struct ath_common *common);
int32_t ath_hw_get_listen_time(struct ath_common *common);
+extern __attribute__ ((format (printf, 3, 4))) int
+ath_printk(const char *level, struct ath_common *common, const char *fmt, ...);
+
+#define ath_emerg(common, fmt, ...) \
+ ath_printk(KERN_EMERG, common, fmt, ##__VA_ARGS)
+#define ath_alert(common, fmt, ...) \
+ ath_printk(KERN_ALERT, common, fmt, ##__VA_ARGS)
+#define ath_crit(common, fmt, ...) \
+ ath_printk(KERN_CRIT, common, fmt, ##__VA_ARGS)
+#define ath_err(common, fmt, ...) \
+ ath_printk(KERN_ERR, common, fmt, ##__VA_ARGS)
+#define ath_warn(common, fmt, ...) \
+ ath_printk(KERN_WARNING, common, fmt, ##__VA_ARGS)
+#define ath_notice(common, fmt, ...) \
+ ath_printk(KERN_NOTICE, common, fmt, ##__VA_ARGS)
+#define ath_info(common, fmt, ...) \
+ ath_printk(KERN_INFO, common, fmt, ##__VA_ARGS)
+
+/**
+ * enum ath_debug_level - atheros wireless debug level
+ *
+ * @ATH_DBG_RESET: reset processing
+ * @ATH_DBG_QUEUE: hardware queue management
+ * @ATH_DBG_EEPROM: eeprom processing
+ * @ATH_DBG_CALIBRATE: periodic calibration
+ * @ATH_DBG_INTERRUPT: interrupt processing
+ * @ATH_DBG_REGULATORY: regulatory processing
+ * @ATH_DBG_ANI: adaptive noise immunitive processing
+ * @ATH_DBG_XMIT: basic xmit operation
+ * @ATH_DBG_BEACON: beacon handling
+ * @ATH_DBG_CONFIG: configuration of the hardware
+ * @ATH_DBG_FATAL: fatal errors, this is the default, DBG_DEFAULT
+ * @ATH_DBG_PS: power save processing
+ * @ATH_DBG_HWTIMER: hardware timer handling
+ * @ATH_DBG_BTCOEX: bluetooth coexistance
+ * @ATH_DBG_BSTUCK: stuck beacons
+ * @ATH_DBG_ANY: enable all debugging
+ *
+ * The debug level is used to control the amount and type of debugging output
+ * we want to see. Each driver has its own method for enabling debugging and
+ * modifying debug level states -- but this is typically done through a
+ * module parameter 'debug' along with a respective 'debug' debugfs file
+ * entry.
+ */
+enum ATH_DEBUG {
+ ATH_DBG_RESET = 0x00000001,
+ ATH_DBG_QUEUE = 0x00000002,
+ ATH_DBG_EEPROM = 0x00000004,
+ ATH_DBG_CALIBRATE = 0x00000008,
+ ATH_DBG_INTERRUPT = 0x00000010,
+ ATH_DBG_REGULATORY = 0x00000020,
+ ATH_DBG_ANI = 0x00000040,
+ ATH_DBG_XMIT = 0x00000080,
+ ATH_DBG_BEACON = 0x00000100,
+ ATH_DBG_CONFIG = 0x00000200,
+ ATH_DBG_FATAL = 0x00000400,
+ ATH_DBG_PS = 0x00000800,
+ ATH_DBG_HWTIMER = 0x00001000,
+ ATH_DBG_BTCOEX = 0x00002000,
+ ATH_DBG_WMI = 0x00004000,
+ ATH_DBG_BSTUCK = 0x00008000,
+ ATH_DBG_ANY = 0xffffffff
+};
+
+#define ATH_DBG_DEFAULT (ATH_DBG_FATAL)
+
+#ifdef CONFIG_ATH_DEBUG
+
+#define ath_dbg(common, dbg_mask, fmt, ...) \
+({ \
+ int rtn; \
+ if ((common)->debug_mask & dbg_mask) \
+ rtn = ath_printk(KERN_DEBUG, common, fmt, \
+ ##__VA_ARGS__); \
+ else \
+ rtn = 0; \
+ \
+ rtn; \
+})
+#define ATH_DBG_WARN(foo, arg...) WARN(foo, arg)
+
+#else
+
+static inline __attribute__ ((format (printf, 3, 4))) int
+ath_dbg(struct ath_common *common, enum ATH_DEBUG dbg_mask,
+ const char *fmt, ...)
+{
+ return 0;
+}
+#define ATH_DBG_WARN(foo, arg) do {} while (0)
+
+#endif /* CONFIG_ATH_DEBUG */
+
+/** Returns string describing opmode, or NULL if unknown mode. */
+#ifdef CONFIG_ATH_DEBUG
+const char *ath_opmode_to_string(enum nl80211_iftype opmode);
+#else
+static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
+{
+ return "UNKNOWN";
+}
+#endif
+
#endif /* ATH_H */
diff --git a/drivers/net/wireless/ath/debug.c b/drivers/net/wireless/ath/debug.c
index a9600ba..5367b10 100644
--- a/drivers/net/wireless/ath/debug.c
+++ b/drivers/net/wireless/ath/debug.c
@@ -15,26 +15,6 @@
*/
#include "ath.h"
-#include "debug.h"
-
-void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...)
-{
- struct va_format vaf;
- va_list args;
-
- if (likely(!(common->debug_mask & dbg_mask)))
- return;
-
- va_start(args, fmt);
-
- vaf.fmt = fmt;
- vaf.va = &args;
-
- printk(KERN_DEBUG "ath: %pV", &vaf);
-
- va_end(args);
-}
-EXPORT_SYMBOL(ath_print);
const char *ath_opmode_to_string(enum nl80211_iftype opmode)
{
diff --git a/drivers/net/wireless/ath/debug.h b/drivers/net/wireless/ath/debug.h
index f207007..cec951c 100644
--- a/drivers/net/wireless/ath/debug.h
+++ b/drivers/net/wireless/ath/debug.h
@@ -17,76 +17,6 @@
#ifndef ATH_DEBUG_H
#define ATH_DEBUG_H
-#include "ath.h"
-
-/**
- * enum ath_debug_level - atheros wireless debug level
- *
- * @ATH_DBG_RESET: reset processing
- * @ATH_DBG_QUEUE: hardware queue management
- * @ATH_DBG_EEPROM: eeprom processing
- * @ATH_DBG_CALIBRATE: periodic calibration
- * @ATH_DBG_INTERRUPT: interrupt processing
- * @ATH_DBG_REGULATORY: regulatory processing
- * @ATH_DBG_ANI: adaptive noise immunitive processing
- * @ATH_DBG_XMIT: basic xmit operation
- * @ATH_DBG_BEACON: beacon handling
- * @ATH_DBG_CONFIG: configuration of the hardware
- * @ATH_DBG_FATAL: fatal errors, this is the default, DBG_DEFAULT
- * @ATH_DBG_PS: power save processing
- * @ATH_DBG_HWTIMER: hardware timer handling
- * @ATH_DBG_BTCOEX: bluetooth coexistance
- * @ATH_DBG_BSTUCK: stuck beacons
- * @ATH_DBG_ANY: enable all debugging
- *
- * The debug level is used to control the amount and type of debugging output
- * we want to see. Each driver has its own method for enabling debugging and
- * modifying debug level states -- but this is typically done through a
- * module parameter 'debug' along with a respective 'debug' debugfs file
- * entry.
- */
-enum ATH_DEBUG {
- ATH_DBG_RESET = 0x00000001,
- ATH_DBG_QUEUE = 0x00000002,
- ATH_DBG_EEPROM = 0x00000004,
- ATH_DBG_CALIBRATE = 0x00000008,
- ATH_DBG_INTERRUPT = 0x00000010,
- ATH_DBG_REGULATORY = 0x00000020,
- ATH_DBG_ANI = 0x00000040,
- ATH_DBG_XMIT = 0x00000080,
- ATH_DBG_BEACON = 0x00000100,
- ATH_DBG_CONFIG = 0x00000200,
- ATH_DBG_FATAL = 0x00000400,
- ATH_DBG_PS = 0x00000800,
- ATH_DBG_HWTIMER = 0x00001000,
- ATH_DBG_BTCOEX = 0x00002000,
- ATH_DBG_WMI = 0x00004000,
- ATH_DBG_BSTUCK = 0x00008000,
- ATH_DBG_ANY = 0xffffffff
-};
-
-#define ATH_DBG_DEFAULT (ATH_DBG_FATAL)
-
-#ifdef CONFIG_ATH_DEBUG
-void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...)
- __attribute__ ((format (printf, 3, 4)));
-#define ATH_DBG_WARN(foo, arg...) WARN(foo, arg)
-#else
-static inline void __attribute__ ((format (printf, 3, 4)))
-ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...)
-{
-}
-#define ATH_DBG_WARN(foo, arg)
-#endif /* CONFIG_ATH_DEBUG */
-
-/** Returns string describing opmode, or NULL if unknown mode. */
-#ifdef CONFIG_ATH_DEBUG
-const char *ath_opmode_to_string(enum nl80211_iftype opmode);
-#else
-static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode)
-{
- return "UNKNOWN";
-}
-#endif
+#define ath_print ath_dbg
#endif /* ATH_DEBUG_H */
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 487193f..c325202 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -56,3 +56,23 @@ struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
return skb;
}
EXPORT_SYMBOL(ath_rxbuf_alloc);
+
+int ath_printk(const char *level, struct ath_common *common,
+ const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+ int rtn;
+
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ rtn = printk("%sath: %pV", level, &vaf);
+
+ va_end(args);
+
+ return rtn;
+}
+EXPORT_SYMBOL(ath_printk);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists