Search Linux Wireless

Re: [PATCH v1] rtlwifi:rtl8723ae: Fix embedded function names with __func__

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/20/2017 06:53 AM, venkat.prashanth2498@xxxxxxxxx wrote:
From: Venkat Prashanth B U <venkat.prashanth2498@xxxxxxxxx>

Prefer and make it generic by using %s and __func__ to print functions name
instead of embedding functions name in print statements

Signed-off-by: Venkat Prashanth B U <venkat.prashanth2498@xxxxxxxxx>

What changed between the first one you sent, and the second that you labeled "V1". The reviewers should not have to discover that for themselves.

Certainly adding the function name to the printouts might be useful; however, your commit message says "instead of embedding functions name". That is obviously not true. None of the changes ever included the function name in the message.

There are other comments shown below.


---
  drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
index f713c72..bd56bb6 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
@@ -389,7 +389,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,
if (pci_dma_mapping_error(rtlpci->pdev, mapping)) {
  		RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
-			 "DMA mapping error\n");
+			 "%s():DMA Mapping Error", __func__);
  		return;
  	}
  	if (mac->opmode == NL80211_IFTYPE_STATION) {
@@ -498,7 +498,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,
  		if (ieee80211_is_data_qos(fc)) {
  			if (mac->rdg_en) {
  				RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
-					 "Enable RDG function.\n");
+			"%s():Enable RDG function.\n", __func__);

The above line fails to align the continuation statement. Always run scripts/checkpatch.pl on all your submissions.

  				SET_TX_DESC_RDG_ENABLE(pdesc, 1);
  				SET_TX_DESC_HTC(pdesc, 1);
  			}
@@ -537,7 +537,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,
  		SET_TX_DESC_BMC(pdesc, 1);
  	}
- RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE, "\n");
+	RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE, "%s():\n", __func__);

This one is NOT an error condition. Why would the function name be needed here? If the user has DBG_TRACE turned on, then the function where a COMP_SEND message is emitted will be known. The code is bloated enough already.

  }
void rtl8723e_tx_fill_cmddesc(struct ieee80211_hw *hw,
@@ -557,7 +557,7 @@ void rtl8723e_tx_fill_cmddesc(struct ieee80211_hw *hw,
if (pci_dma_mapping_error(rtlpci->pdev, mapping)) {
  		RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
-			 "DMA mapping error\n");
+			 "%s():DMA Mapping Error", __func__);
  		return;
  	}
  	CLEAR_PCI_TX_DESC_CONTENT(pdesc, TX_DESC_SIZE);


The patch needs more work. NACK.

Larry



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux