* [PATCH 0/3] iwlwifi print formatting fixes
@ 2012-05-16 20:40 Johannes Berg
2012-05-16 20:40 ` [PATCH 1/3] iwlwifi: fix prints in iwl_rx_handle Johannes Berg
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Johannes Berg @ 2012-05-16 20:40 UTC (permalink / raw)
To: linville; +Cc: linux-wireless
These are fixes for iwlwifi's debug statements and the
addition of the __printf attribute.
I ended up marking the %pM one for stable since I think
%pM can cause crashes if the pointer is completely bogus.
johannes
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] iwlwifi: fix prints in iwl_rx_handle 2012-05-16 20:40 [PATCH 0/3] iwlwifi print formatting fixes Johannes Berg @ 2012-05-16 20:40 ` Johannes Berg 2012-05-16 20:40 ` [PATCH 2/3] iwlwifi: fix debug print in iwl_sta_calc_ht_flags Johannes Berg 2012-05-16 20:40 ` [PATCH 3/3] iwlwifi: add __printf argument checking Johannes Berg 2 siblings, 0 replies; 8+ messages in thread From: Johannes Berg @ 2012-05-16 20:40 UTC (permalink / raw) To: linville; +Cc: linux-wireless, Emmanuel Grumbach From: Emmanuel Grumbach <emmanuel.grumbach@intel.com> The debug prints were wrong and buggy. The HW pointer wasn't printed correctly, it was mixed up with the pointer to the rxbuf. Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> Reviewed-by: Wey-Yi W Guy <wey-yi.w.guy@intel.com> Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c index 08517d3..1a7eb2a 100644 --- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c +++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c @@ -497,7 +497,7 @@ static void iwl_rx_handle(struct iwl_trans *trans) /* Rx interrupt, but nothing sent from uCode */ if (i == r) - IWL_DEBUG_RX(trans, "r = %d, i = %d\n", r, i); + IWL_DEBUG_RX(trans, "HW = SW = %d\n", r); /* calculate total frames need to be restock after handling RX */ total_empty = r - rxq->write_actual; @@ -513,8 +513,8 @@ static void iwl_rx_handle(struct iwl_trans *trans) rxb = rxq->queue[i]; rxq->queue[i] = NULL; - IWL_DEBUG_RX(trans, "rxbuf: r = %d, i = %d (%p)\n", rxb); - + IWL_DEBUG_RX(trans, "rxbuf: HW = %d, SW = %d (%p)\n", + r, i, rxb); iwl_rx_handle_rxbuf(trans, rxb); i = (i + 1) & RX_QUEUE_MASK; -- 1.7.10 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] iwlwifi: fix debug print in iwl_sta_calc_ht_flags 2012-05-16 20:40 [PATCH 0/3] iwlwifi print formatting fixes Johannes Berg 2012-05-16 20:40 ` [PATCH 1/3] iwlwifi: fix prints in iwl_rx_handle Johannes Berg @ 2012-05-16 20:40 ` Johannes Berg 2012-05-16 20:40 ` [PATCH 3/3] iwlwifi: add __printf argument checking Johannes Berg 2 siblings, 0 replies; 8+ messages in thread From: Johannes Berg @ 2012-05-16 20:40 UTC (permalink / raw) To: linville; +Cc: linux-wireless, Meenakshi Venkataraman From: Meenakshi Venkataraman <meenakshi.venkataraman@intel.com> We missed passing an argument to the debug print. Fix it. Cc: stable@kernel.org Signed-off-by: Meenakshi Venkataraman <meenakshi.venkataraman@intel.com> Reviewed-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- drivers/net/wireless/iwlwifi/iwl-agn-sta.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-sta.c b/drivers/net/wireless/iwlwifi/iwl-agn-sta.c index 67e6f1d..76c1f72 100644 --- a/drivers/net/wireless/iwlwifi/iwl-agn-sta.c +++ b/drivers/net/wireless/iwlwifi/iwl-agn-sta.c @@ -236,6 +236,7 @@ static void iwl_sta_calc_ht_flags(struct iwl_priv *priv, mimo_ps_mode = (sta_ht_inf->cap & IEEE80211_HT_CAP_SM_PS) >> 2; IWL_DEBUG_INFO(priv, "STA %pM SM PS mode: %s\n", + sta->addr, (mimo_ps_mode == WLAN_HT_CAP_SM_PS_STATIC) ? "static" : (mimo_ps_mode == WLAN_HT_CAP_SM_PS_DYNAMIC) ? -- 1.7.10 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] iwlwifi: add __printf argument checking 2012-05-16 20:40 [PATCH 0/3] iwlwifi print formatting fixes Johannes Berg 2012-05-16 20:40 ` [PATCH 1/3] iwlwifi: fix prints in iwl_rx_handle Johannes Berg 2012-05-16 20:40 ` [PATCH 2/3] iwlwifi: fix debug print in iwl_sta_calc_ht_flags Johannes Berg @ 2012-05-16 20:40 ` Johannes Berg 2012-05-16 20:58 ` Joe Perches 2012-05-16 21:05 ` [PATCH 3/3 v2] " Johannes Berg 2 siblings, 2 replies; 8+ messages in thread From: Johannes Berg @ 2012-05-16 20:40 UTC (permalink / raw) To: linville; +Cc: linux-wireless, Johannes Berg From: Johannes Berg <johannes.berg@intel.com> Joe Perches suggested adding the __printf attribute to the __iwl_dbg function to check arguments; add it to all of the logging functions (err, warn, info, dbg and crit.) Change-Id: I7a2ffcd3f625ff29c65519b0c3d474bace884f9d Signed-off-by: Johannes Berg <johannes.berg@intel.com> Reviewed-on: http://git-mwg.jer.intel.com/gerrit/2022 Tested-by: Jenkins Reviewed-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> --- drivers/net/wireless/iwlwifi/iwl-debug.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/iwl-debug.h b/drivers/net/wireless/iwlwifi/iwl-debug.h index 8376b84..f6bf91c 100644 --- a/drivers/net/wireless/iwlwifi/iwl-debug.h +++ b/drivers/net/wireless/iwlwifi/iwl-debug.h @@ -38,10 +38,10 @@ static inline bool iwl_have_debug_level(u32 level) } void __iwl_err(struct device *dev, bool rfkill_prefix, bool only_trace, - const char *fmt, ...); -void __iwl_warn(struct device *dev, const char *fmt, ...); -void __iwl_info(struct device *dev, const char *fmt, ...); -void __iwl_crit(struct device *dev, const char *fmt, ...); + const char *fmt, ...) __printf(4, 5); +void __iwl_warn(struct device *dev, const char *fmt, ...) __printf(2, 3); +void __iwl_info(struct device *dev, const char *fmt, ...) __printf(2, 3); +void __iwl_crit(struct device *dev, const char *fmt, ...) __printf(2, 3); /* No matter what is m (priv, bus, trans), this will work */ #define IWL_ERR(m, f, a...) __iwl_err((m)->dev, false, false, f, ## a) @@ -52,9 +52,9 @@ void __iwl_crit(struct device *dev, const char *fmt, ...); #if defined(CONFIG_IWLWIFI_DEBUG) || defined(CONFIG_IWLWIFI_DEVICE_TRACING) void __iwl_dbg(struct device *dev, u32 level, bool limit, const char *function, - const char *fmt, ...); + const char *fmt, ...) __printf(5, 6); #else -static inline void +__printf(5, 6) static inline void __iwl_dbg(struct device *dev, u32 level, bool limit, const char *function, const char *fmt, ...) -- 1.7.10 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] iwlwifi: add __printf argument checking 2012-05-16 20:40 ` [PATCH 3/3] iwlwifi: add __printf argument checking Johannes Berg @ 2012-05-16 20:58 ` Joe Perches 2012-05-16 21:03 ` Johannes Berg 2012-05-16 21:05 ` [PATCH 3/3 v2] " Johannes Berg 1 sibling, 1 reply; 8+ messages in thread From: Joe Perches @ 2012-05-16 20:58 UTC (permalink / raw) To: Johannes Berg; +Cc: linville, linux-wireless, Johannes Berg On Wed, 2012-05-16 at 22:40 +0200, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > Joe Perches suggested adding the __printf attribute > to the __iwl_dbg function to check arguments; add it > to all of the logging functions (err, warn, info, dbg > and crit.) > > Change-Id: I7a2ffcd3f625ff29c65519b0c3d474bace884f9d > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > Reviewed-on: http://git-mwg.jer.intel.com/gerrit/2022 > Tested-by: Jenkins > Reviewed-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> Trivial comments: Odd Reviewed-on: and Tested-by: lines. The intel.com link does not resolve here too. > diff --git a/drivers/net/wireless/iwlwifi/iwl-debug.h b/drivers/net/wireless/iwlwifi/iwl-debug.h [] > @@ -38,10 +38,10 @@ static inline bool iwl_have_debug_level(u32 level) > } > > void __iwl_err(struct device *dev, bool rfkill_prefix, bool only_trace, > - const char *fmt, ...); > -void __iwl_warn(struct device *dev, const char *fmt, ...); > -void __iwl_info(struct device *dev, const char *fmt, ...); > -void __iwl_crit(struct device *dev, const char *fmt, ...); > + const char *fmt, ...) __printf(4, 5); > +void __iwl_warn(struct device *dev, const char *fmt, ...) __printf(2, 3); > +void __iwl_info(struct device *dev, const char *fmt, ...) __printf(2, 3); > +void __iwl_crit(struct device *dev, const char *fmt, ...) __printf(2, 3); As far as I know, there are very few in-kernel uses of __printf after the function prototype (6 out of 183). $ git grep -E "\b__printf\s*\(\s*\w+\s*,\s*\w+\s*\)\s*;" | wc -l 6 $ git grep -E "\b__printf\s*\(\s*\w+\s*,\s*\w+\s*\)" | wc -l 183 I'd prefer to keep the __printf before the return type. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] iwlwifi: add __printf argument checking 2012-05-16 20:58 ` Joe Perches @ 2012-05-16 21:03 ` Johannes Berg 2012-05-16 21:11 ` Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: Johannes Berg @ 2012-05-16 21:03 UTC (permalink / raw) To: Joe Perches; +Cc: linville, linux-wireless On Wed, 2012-05-16 at 13:58 -0700, Joe Perches wrote: > > Change-Id: I7a2ffcd3f625ff29c65519b0c3d474bace884f9d > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > > Reviewed-on: http://git-mwg.jer.intel.com/gerrit/2022 > > Tested-by: Jenkins > > Reviewed-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> > > Trivial comments: > > Odd Reviewed-on: and Tested-by: lines. > The intel.com link does not resolve here too. Meh, second time I've messed this up... It's not supposed to, so I guess that's good. I'll send a new version of this. > > diff --git a/drivers/net/wireless/iwlwifi/iwl-debug.h b/drivers/net/wireless/iwlwifi/iwl-debug.h > [] > > @@ -38,10 +38,10 @@ static inline bool iwl_have_debug_level(u32 level) > > } > > > > void __iwl_err(struct device *dev, bool rfkill_prefix, bool only_trace, > > - const char *fmt, ...); > > -void __iwl_warn(struct device *dev, const char *fmt, ...); > > -void __iwl_info(struct device *dev, const char *fmt, ...); > > -void __iwl_crit(struct device *dev, const char *fmt, ...); > > + const char *fmt, ...) __printf(4, 5); > > +void __iwl_warn(struct device *dev, const char *fmt, ...) __printf(2, 3); > > +void __iwl_info(struct device *dev, const char *fmt, ...) __printf(2, 3); > > +void __iwl_crit(struct device *dev, const char *fmt, ...) __printf(2, 3); > > As far as I know, there are very few in-kernel uses of > __printf after the function prototype (6 out of 183). > > $ git grep -E "\b__printf\s*\(\s*\w+\s*,\s*\w+\s*\)\s*;" | wc -l > 6 > $ git grep -E "\b__printf\s*\(\s*\w+\s*,\s*\w+\s*\)" | wc -l > 183 > > I'd prefer to keep the __printf before the return type. Well, maybe now it's 3 more? :-) To me, before looks really strange, and while maybe __printf is almost always before, if you do the same grep with, say, __releases() or __acquires(), it's almost always after, so ... johannes ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] iwlwifi: add __printf argument checking 2012-05-16 21:03 ` Johannes Berg @ 2012-05-16 21:11 ` Joe Perches 0 siblings, 0 replies; 8+ messages in thread From: Joe Perches @ 2012-05-16 21:11 UTC (permalink / raw) To: Johannes Berg; +Cc: linville, linux-wireless On Wed, 2012-05-16 at 23:03 +0200, Johannes Berg wrote: > On Wed, 2012-05-16 at 13:58 -0700, Joe Perches wrote: > > > > Change-Id: I7a2ffcd3f625ff29c65519b0c3d474bace884f9d > > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > > > Reviewed-on: http://git-mwg.jer.intel.com/gerrit/2022 > > > Tested-by: Jenkins > > > Reviewed-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> > > > > Trivial comments: > > > > Odd Reviewed-on: and Tested-by: lines. > > The intel.com link does not resolve here too. > > Meh, second time I've messed this up... It's not supposed to, so I guess > that's good. I'll send a new version of this. > > > > diff --git a/drivers/net/wireless/iwlwifi/iwl-debug.h b/drivers/net/wireless/iwlwifi/iwl-debug.h > > [] > > > @@ -38,10 +38,10 @@ static inline bool iwl_have_debug_level(u32 level) > > > } > > > > > > void __iwl_err(struct device *dev, bool rfkill_prefix, bool only_trace, > > > - const char *fmt, ...); > > > -void __iwl_warn(struct device *dev, const char *fmt, ...); > > > -void __iwl_info(struct device *dev, const char *fmt, ...); > > > -void __iwl_crit(struct device *dev, const char *fmt, ...); > > > + const char *fmt, ...) __printf(4, 5); > > > +void __iwl_warn(struct device *dev, const char *fmt, ...) __printf(2, 3); > > > +void __iwl_info(struct device *dev, const char *fmt, ...) __printf(2, 3); > > > +void __iwl_crit(struct device *dev, const char *fmt, ...) __printf(2, 3); > > > > As far as I know, there are very few in-kernel uses of > > __printf after the function prototype (6 out of 183). > > > > $ git grep -E "\b__printf\s*\(\s*\w+\s*,\s*\w+\s*\)\s*;" | wc -l > > 6 > > $ git grep -E "\b__printf\s*\(\s*\w+\s*,\s*\w+\s*\)" | wc -l > > 183 > > > > I'd prefer to keep the __printf before the return type. > > Well, maybe now it's 3 more? :-) Hope not. You noticed you _have_ to put it before the function on the static inlines so why not keep it there in all cases? ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3 v2] iwlwifi: add __printf argument checking 2012-05-16 20:40 ` [PATCH 3/3] iwlwifi: add __printf argument checking Johannes Berg 2012-05-16 20:58 ` Joe Perches @ 2012-05-16 21:05 ` Johannes Berg 1 sibling, 0 replies; 8+ messages in thread From: Johannes Berg @ 2012-05-16 21:05 UTC (permalink / raw) To: linville; +Cc: linux-wireless From: Johannes Berg <johannes.berg@intel.com> Joe Perches suggested adding the __printf attribute to the __iwl_dbg function to check arguments; add it to all of the logging functions (err, warn, info, dbg and crit.) Reviewed-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- v2: remove gerrit junk from commit log diff --git a/drivers/net/wireless/iwlwifi/iwl-debug.h b/drivers/net/wireless/iwlwifi/iwl-debug.h index 8376b84..f6bf91c 100644 --- a/drivers/net/wireless/iwlwifi/iwl-debug.h +++ b/drivers/net/wireless/iwlwifi/iwl-debug.h @@ -38,10 +38,10 @@ static inline bool iwl_have_debug_level(u32 level) } void __iwl_err(struct device *dev, bool rfkill_prefix, bool only_trace, - const char *fmt, ...); -void __iwl_warn(struct device *dev, const char *fmt, ...); -void __iwl_info(struct device *dev, const char *fmt, ...); -void __iwl_crit(struct device *dev, const char *fmt, ...); + const char *fmt, ...) __printf(4, 5); +void __iwl_warn(struct device *dev, const char *fmt, ...) __printf(2, 3); +void __iwl_info(struct device *dev, const char *fmt, ...) __printf(2, 3); +void __iwl_crit(struct device *dev, const char *fmt, ...) __printf(2, 3); /* No matter what is m (priv, bus, trans), this will work */ #define IWL_ERR(m, f, a...) __iwl_err((m)->dev, false, false, f, ## a) @@ -52,9 +52,9 @@ void __iwl_crit(struct device *dev, const char *fmt, ...); #if defined(CONFIG_IWLWIFI_DEBUG) || defined(CONFIG_IWLWIFI_DEVICE_TRACING) void __iwl_dbg(struct device *dev, u32 level, bool limit, const char *function, - const char *fmt, ...); + const char *fmt, ...) __printf(5, 6); #else -static inline void +__printf(5, 6) static inline void __iwl_dbg(struct device *dev, u32 level, bool limit, const char *function, const char *fmt, ...) ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-05-16 21:11 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-16 20:40 [PATCH 0/3] iwlwifi print formatting fixes Johannes Berg 2012-05-16 20:40 ` [PATCH 1/3] iwlwifi: fix prints in iwl_rx_handle Johannes Berg 2012-05-16 20:40 ` [PATCH 2/3] iwlwifi: fix debug print in iwl_sta_calc_ht_flags Johannes Berg 2012-05-16 20:40 ` [PATCH 3/3] iwlwifi: add __printf argument checking Johannes Berg 2012-05-16 20:58 ` Joe Perches 2012-05-16 21:03 ` Johannes Berg 2012-05-16 21:11 ` Joe Perches 2012-05-16 21:05 ` [PATCH 3/3 v2] " Johannes Berg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.