Code Review也有潛規則

導讀

在Google，任何產品、任何項目的代碼，在沒有經過有效的代碼審查（Code Review）前是不能提交到代碼庫里的，這也是Google程序如此優秀的最重要原因之一。恩，這就是所謂別人家的公司，不過，Code Review的重要性，可見一斑。說起Code Review，通常會被認為是開發GG的事情，其實不然，作為測試人員，尤其是“測試左移”越來越成為趨勢的情況下，勢必要提高代碼能力，而Code Review就是一個很好的切入點。不僅可以學習開發GG的技術，還可以完善測分、提前發現bug、降低質量風險和測試成本，好處不言而喻。筆者作為Code Review的新手，經過一番學習和實踐，總結了一點點“潛規則”，希望可以拋磚引玉。

資源泄漏篇

試想，如果申請的資源未進行釋放，那勢必會資源泄漏，尤其是對于長時間運行的程序來說，會導致系統中可用的資源越來越少，嚴重的，系統會因為資源耗盡而崩潰。因此，資源泄漏的問題需要得到重視，除了提測后的資源掛機測試之外，在前期Code review階段更加需要注意，以便盡快盡早發現問題，降低成本和風險。

對于這類問題，筆者總結了如下需要注意的地方：

慧眼識珠：資源獲取和資源釋放函數需要成對使用

成對使用的資源獲取和釋放函數太多，這里就不一一列舉啦，總之， 看到資源獲取語句，必查資源釋放語句，反之，亦然。

異常處理篇

優雅編程需要在一開始就考慮異常事件的處理，不僅需要保證在正常情況下程序可以穩定運行，而且在發生錯誤和出現“意外事件”時仍然能繼續可靠運行。因此，需要盡可能多的預見所有這些異常事件。異常處理代碼也是bug的高發區域，不僅在設計用例階段需要考慮全面，Code Review的時候也需要特別關注。

慧眼識珠：異常處理 1)  任何可能出錯的函數調用（語句），必須加異常處理，這些函數調用，包括但不限于

• 網絡交互：是否有超時、是否考慮負載均衡、重試機制等
• 數據庫交互：是否連接成功、超時、重試、判斷返回值等
• 讀取請求數據包：是否判斷返回值，防止讀到臟數據等
• 文件系統操作： read,start, write,open,等，判斷各種正常/異常情況
• 邊界值考慮是否周全

2) 對于異常處理，務必注意如下：

• 異常判斷一定要有
• 異常判斷的時機、條件一定要正確
• 異常判斷的分支一定要完整
• 異常處理一定要充分
• 邊界考慮周全

數組越界篇

訪問數組時，如果訪問了數組定義之外的范圍，即下標落在區間[0, size-1]之外，會導致程序運行錯誤，而C++中數組下標越界，編譯器是不會檢查出這種錯誤的，但后果可能會比想象中嚴重，甚至程序崩潰。因此，這類看似不起眼的小問題，也需要得到重視。下圖就是一個缺少下標判斷的例子。

慧眼識珠：對于用到數組的地方，一定注意如下幾點：

1) 記住數組循環操作的代碼模板for (i = 0; i < size; i++)

2) 記住數組下標判斷的代碼模板if (i < 0 || i >= size)

// 錯誤區間

或者

if (i >= 0 && i < size)

// 正確區間

3) 看到下標操作，必查下標判斷

• 下標判斷一定要有，且出現在正確的地方即判斷要及時，并注意判斷條件要正確

多線程問題篇

多線程編程很容易遇上諸如丟失更新、臟讀、死鎖等線程沖突問題。多線程的問題一旦發生便很難定位和解決，所以要在編程的初始階段就要注意避免多線程程序常見的錯誤。多線程同時讀寫同一資源，例如變量，文件，同一緩沖區等，一旦出現競爭條件，很容易導致程序運行結果出錯。這類的用戶反饋問題也有很多，首先列舉下導致多線程問題的原因：

1) 資源的讀寫和更新沒有加鎖（此處經常會有用戶反饋）

2) 資源的獲取和訪問之間有時間間隔

3) 加鎖范圍太小

4) 使用了線程不安全函數

慧眼識珠：多線程問題

1) 識別全局資源

•  g開頭的變量，g_data.*，g_var.*
• 利用IDE，如source insight，將鼠標移到變量上面，會顯示變量類型

2) 看到資源的讀寫和更新，必查加鎖

若該資源會被同時讀寫，則檢查此變量的所有讀寫操作，確保正確加鎖。

3) 看到加鎖操作，必查加鎖范圍

加鎖太小，程序出錯；加鎖太大，降低性能；需要根據具體情況權衡。

4) 看到資源的獲取和訪問之間有時間間隔，必查資源是否會被更新

5) 識別線程不安全函數：

• 返回緩沖區的函數，例如inet_ntoa,localtime，建議分別使用inet_ntoa_r，localtime_s代替
• 會記錄函數狀態的函數，例如strtok
• 基礎庫的初始化函數，例如mysql_init, curl_easy_init

除零錯誤篇

雖然 C++ 加入了異常機制來處理很多運行時錯誤, 但是異常機制的功效非常受限, 很多錯誤還沒辦法用原生異常手段捕捉，例如這里所說的除零錯誤，而這個錯誤也經常導致程序崩潰，因此Code Review時需特別注意。

慧眼識珠：除零錯誤

1) 除法或者取模操作，必加除數為零的判斷

2) 浮點轉整型會丟失小數部分，特別需要關注0.*變成0的情況

3) 對于影響程序穩定性和健壯性的輸入，必做檢查

緩沖區溢出篇

通過往程序的緩沖區寫超出其長度的內容，造成緩沖區的溢出，從而破壞程序的堆棧，造成程序崩潰或使程序轉而執行其它指令。造成緩沖區溢出的原因是程序中沒有仔細檢查用戶輸入的參數。

慧眼識珠：緩沖區溢出問題

1) 識別緩沖區溢出高風險函數，慎用或者干脆不使用緩沖區溢出高風險函數

• 不保證補\0的函數，例如strncpy
• pathXXX系列函數有可能buffer溢出，需要排查一遍是否存在這些api的使用
• 參數中不帶目標緩沖區長度的字符串處理函數，例如strcpy，strcat，strncat，sprintf，等等
• memcpy最好使用安全版本

2) 看到緩沖區溢出高風險函數，必查溢出

3) 看到可寫緩沖區當參數，必查緩沖區長度

業務邏輯篇

除了上述和業務無關的較為通用的具體代碼問題外，業務邏輯錯誤，也需要關注，當然這就需要在深入理解業務需求的基礎上了。

慧眼識珠：業務邏輯錯誤

1) 前提：深入了解被測業務、需求，即深入需求分析、采用測試建模

2) 找開發了解架構設計、代碼結構，事半功倍

3) CR可以分階段進行：

階段一總覽：看到一塊代碼，不急于研究細節，而是首先根據上下文，函數原型，以及對代碼結構的快速掃描，簡單得出代碼與業務需求的映射；

階段二深入：根據代碼結構深入，可以從核心功能或者感興趣的部分入手，深入淺出

階段三回顧：再回頭總結思考一下：這個代碼塊的作用是什么？有何影響？是否正確按照預期實現了業務需求？

4) 識別邏輯錯誤，需要測試人員在做CR時候，能夠經常地從代碼中“跳”出來，使用測試思維而不是開發思維，來思考上面的問題、或者跟開發人員溝通。一些邏輯錯誤，確實可以在思考、溝通的時候自然而然暴露出來。

Code Review規則提取篇

以上介紹了一些常用的CR技巧，那么CR在一定程度上是否也可實現自動化呢，答案是：yes。由于業界的靜態代碼掃描工具（如klocwork ,cppcheck等），只專注于不存在誤報的、能夠普遍使用的規則，規則有限且是基礎校驗，于是管家測試組的 振宇大牛 開發了一套靈活自定義規則的缺陷規則代碼掃描工具，規則來源于CodeReview、crash分析、用戶反饋分析等。缺陷規則代碼掃描專注于靜態掃描存在誤報的規則以及只有在特定運行時態會Crash的代碼規則，可以說補齊了靜態代碼掃描的短板并實現了一定程度的CR自動化。

應用管理測試組的測試童鞋，從11月至今貢獻了 13 條掃描規則（8條已實現自動化，5條作為CR經驗），這些規則每天都在跑，并且會隨業務增量添加哦。據不完全統計，發現有效bug  60+ ，有效bug解決率 100% 。該自動化掃描目前還會存在一些誤報的情況，so，還需要人工過濾。下圖顯示的是某次的掃描結果，還需要人工check。待將來優化到零誤報，則可以節省掉這一部分工作。

綜上所述，作為CR新手的測試人員，筆者只是羅列了一些簡單的CR技巧，當然，主要是拋磚引玉，期待大家更多的分享交流，相信1+1>2哦。

 

來自：http://tmq.qq.com/2017/01/code-review1/