【譯】華為雲——程式碼質量“多雲”

阿新 • • 發佈：2020-06-24

使用靜態程式碼分析看一看華為的原始碼。

出於各種原因，有許多企業進入了雲市場並建立了他們自己的雲服務。最近，我們的團隊致力於將PVS-Studio程式碼分析工具整合到我們的雲架構中。我們的忠實讀者可能已經猜到我們這次要拿什麼專案開刀，沒錯，就是華為的雲服務。

介紹

如果你訂閱了PVS-Studio團隊的帖子，你可能已經注意到我們最近正在深挖雲技術。我們已經發布了好幾篇相關的文章：

當我正在為這篇文章苦思冥想一個非同尋常的專案的時候，我收到了一封來自於華為的郵件，郵件中給了我一份Offer。查看了一下這個公司的資訊之後，我發現他們擁有自己的雲服務，最重要的是它們的原始碼在GitHub上開源。這就是我在這篇文章中選擇華為雲的原因。正如一句中國老話說的那樣：“相請不如偶遇”。

讓我詳細介紹一下我們的分析工具。PVS-Studio是一個靜態分析工具，用來檢查bug，適用於C，C++，C#以及Java。分析工具可以在Windows,Linux以及macOS上工作。除了可以作為常用開發工具例如Visual Studio，IntelliJ IDEA的外掛之外，還可以整合到SonarQube以及Jenkins。

專案分析

在我為這篇文章做調研的時候，我發現華為有一個研發中心，提供了可用的資訊，手冊以及他們雲服務的原始碼。這些服務由許多種不同的語言編寫，其中Go,Java，Python最常見。

由於我司職Java，我就挑選了相應的專案。你可以從華為的GitHub倉庫中獲取原始碼。

要分析這個專案，要做一些準備工作：

從倉庫中獲取原始碼；
使用Java分析工具指南分析每個專案。

分析這些專案之後，我選擇了其中的三個，原因是其他的Java專案太小了，我們著重看一看這三個專案。

分析結果（警告數已經檔案數）：

huaweicloud-sdk-java:31——高，2——中，16——低，2700+檔案。
huaweicloud-dis-agent:7——高，6——中，6——低，100+檔案。
huaweicloud-sdk-java-dis:15——高，6——中，16——低，270+檔案。

警告很少，說明程式碼質量很高。而且並不是所有的警告都指向真正的錯誤，這更能說明程式碼質量很高。這是因為有時候分析器缺少足夠的資訊來區分正確的程式碼與錯誤的程式碼。基於此，我們求助於使用者提供的資訊日復一日地調整分析器的診斷。歡迎檢視這篇文章靜態分析器與誤報的鬥爭以及原因。

對於分析的專案，我挑選了最值得關注的警告用於接下來的討論。

欄位初始化順序

V6050存在類初始化迴圈。INSTANCE的初始化出現在LOG的初始化之前。UntrustedSSL.java(32),UntrustedSSL.java(59),UntrustedSSL.java(33)

public class UntrustedSSL {
  private static final UntrustedSSL INSTANCE = new UntrustedSSL();
  private static final Logger LOG = LoggerFactory.getLogger(UntrustedSSL.class);
  .... 
  private UntrustedSSL() 
  {
    try
    {
      ....
    }
    catch (Throwable t) {
      LOG.error(t.getMessage(),t);           // <=
    }
  }
}
複製程式碼

如果UntrustedSSL類的建構函式出現異常，會在catch塊中使用LOG記錄異常資訊。然而，由於靜態欄位的初始化書序，在初始化INSTANCE欄位的時候，LOG還沒有初始化，因此，會導致NullPointerException，這個異常是引發另一個異常ExceptionInInitializerError的原因，該異常會在靜態欄位初始化異常時丟擲。解決這個問題的辦法是將LOG的初始化放到INSTANCE之前。

不起眼的打字錯誤

V6005 變數this.metricSchema被賦值給自己。OpenTSDBSchema.java(72):

public class OpenTSDBSchema
{
  @JsonProperty("metric")
  private List<SchemaField> metricSchema;
  ....
  public void setMetricsSchema(List<SchemaField> metricsSchema)
  {
    this.metricSchema = metricSchema;           // <=
  }
  public void setMetricSchema(List<SchemaField> metricSchema)
  {
    this.metricSchema = metricSchema;
  }
  ....
}
複製程式碼

兩個方法都設定metricSchema欄位，但是方法名一個s之差，程式設計師根據方法名來命名引數名，結果，程式碼分析工具指出，metricSchema被賦值給自身，而且引數metricsSchema未使用。

V6005 變數suspend被賦值給自己。 SuspendTransferTaskRequest.java(77)

public class SuspendTransferTaskRequest 
{
  ....
  private boolean suspend;
  ....
  public void setSuspend(boolean suspend)
  {
    suspend = suspend;                        
  }
  .... 
}
複製程式碼

這是因為粗心大意引起的錯誤，結果suspend不能按給定的引數賦值。正確的形式是：

public void setSuspend(boolean suspend)
{
  this.suspend = suspend;                        
}
複製程式碼

條件預定義

V6007規則在警告數量上常常有所突破。

V6007 表示式firewallPolicyId == null總是false。FirewallPolicyServiceImpl.java(125):

public FirewallPolicy
removeFirewallRuleFromPolicy(String firewallPolicyId,String firewallRuleId) 
{
  checkNotNull(firewallPolicyId);
  checkNotNull(firewallRuleId);
  checkState(!(firewallPolicyId == null && firewallRuleId == null),"Either a Firewall Policy or Firewall Rule identifier must be set"); 
  .... 
}
複製程式碼

在這個方法中，checkNotNull方法檢查引數是否為null：

@CanIgnoreReturnValue
public static <T> T checkNotNull(T reference) 
{
  if (reference == null) {
    throw new NullPointerException();
  } else {
    return reference;
  }
}
複製程式碼

在checkNotNull方法之後，你可以100%確定引數不會為null，由於removeFirewallRuleFromPolicy都經過checkNotNull方法檢查，之後再檢查是不是null沒有意義。

一個類似的警告也是由於firewallRuleId：

V6007 表示式firewallRuleId == null總是false。FirewallPolicyServiceImpl.java(125)。

filteringParams != null總是true。NetworkPolicyServiceImpl.java(60)

private Invocation<NetworkServicePolicies> buildInvocation(Map<String,String> filteringParams) 
{
  .... 
  if (filteringParams == null) {
    return servicePoliciesInvocation;
  }
  if (filteringParams != null) {       // <=
    ....
  }
  return servicePoliciesInvocation;
}
複製程式碼

在這個方法中，如果引數filteringParams是null,方法返回一個值，這就是為什麼分析器指出檢查結果總是true,這意味著檢查沒有意義。

還有13個類似的警告：

V6007 表示式'filteringParams != null'總是true. PolicyRuleServiceImpl.java(58)
V6007 表示式'filteringParams != null' 總是true. GroupServiceImpl.java(58)
V6007 表示式'filteringParams != null' 總是true.ExternalSegmentServiceImpl.java(57)
V6007 表示式'filteringParams != null' 總是true. L3policyServiceImpl.java(57)
V6007 表示式n 'filteringParams != null' 總是true. PolicyRuleSetServiceImpl.java(58)
...

空引用

V6008 潛在的空引用m.blockDeviceMapping。NovaServerCreate.java(390)


@Override
public ServerCreateBuilder blockDevice(BlockDeviceMappingCreate blockDevice) {
  if (blockDevice != null && m.blockDeviceMapping == null) {
    m.blockDeviceMapping = Lists.newArrayList();
  }
  m.blockDeviceMapping.add(blockDevice);       // <=
  return this;
}
複製程式碼

在這個方法裡，如果blockDevice為空，m.blockDeviceMapping就不會初始化，該欄位僅在這個方法中被初始化，因此如果呼叫m.blockDeviceMapping的add方法就會出現空指標異常。

V6008 潛在的空引用FileId.get(path)。 TrackedFile.java(140),TrackedFile.java(115)

public TrackedFile(FileFlow<?> flow,Path path) throws IOException 
{
  this(flow,path,FileId.get(path),....);
}
複製程式碼

TrackedFile的建構函式取靜態方法FileId.get(path)的結果最為第三個引數，但是該方法可能會返回null：

public static FileId get(Path file) throws IOException
{
  if (!Files.exists(file))
  {
    return null;
  }
  ....
}
複製程式碼

public TrackedFile(....,....,FileId id,....) throws IOException
{
  ....
  FileId newId = FileId.get(path);
  if (!id.equals(newId))
  {
    ....
  }
}
複製程式碼

可見，如果傳入null作為第三個引數，會引起異常。

還有一個類似的地方： V6008 潛在的空引用buffer. PublishingQueue.java(518)

V6008 潛在的空引用dataTmpFile . CacheManager.java(91)

@Override
public void putToCache(PutRecordsRequest putRecordsRequest)
{
  .... 
  if (dataTmpFile == null || !dataTmpFile.exists())
  {
    try
    {
      dataTmpFile.createNewFile();  // <=
    }
    catch (IOException e)
    {
      LOGGER.error("Failed to create cache tmp file,return.",e);
      return ;
    }
  }
  ....
}
複製程式碼

我認為這裡有兩處打字錯誤，正確的應該是：

if (dataTmpFile != null && !dataTmpFile.exists())
複製程式碼

Substrings與負數

V6009 substring函式需要非負數作為引數，但是可能接收到"-1"，請檢查第二個引數。RemoveVersionProjectIdFromURL.java(37)

@Override
public String apply(String url) {
  String urlRmovePojectId = url.substring(0,url.lastIndexOf("/"));
  return urlRmovePojectId.substring(0,urlRmovePojectId.lastIndexOf("/"));
}
複製程式碼

假設方法獲得一個URL作為字串，沒有做任何驗證。之後，使用lastIndexOf方法將字串切開很多次，如果lastIndexOf沒有找到匹配的字串，會返回"-1"。這會導致StringIndexOutOfBoundsException，因為substring只接受非負數。正確的操作，需要加一個引數驗證，或者檢查lastIndexOf的返回值。

還有幾處片段有同樣的問題

V6009 substring函式需要非負數作為引數，但是可能接收到"-1"，請檢查第二個引數。RemoveProjectIdFromURL.java(37)
V6009 substring函式需要非負數作為引數，但是可能接收到"-1"，請檢查第二個引數。 RemoveVersionProjectIdFromURL.java(38)

被遺忘的結果

V6021 變數url未使用。TriggerV2Service.java(95)

public ActionResponse deleteAllTriggersForFunction(String functionUrn) 
{
  checkArgument(!Strings.isNullOrEmpty(functionUrn),....);
  String url = ClientConstants.FGS_TRIGGERS_V2 +
               ClientConstants.URI_SEP + 
               functionUrn;
  return deleteWithResponse(uri(triggersUrlFmt,functionUrn)).execute();
}
複製程式碼

在這個方法裡，引數url在初始化後沒有使用。很可能url是uri方法的第二個引數，而不是functionUrn，因為functionUrn參與了url的初始化。

引數在建構函式未使用

V6022 引數returnType在建構函式未使用。 HttpRequest.java(68)

public class HttpReQuest<R> 
{
  ....
  Class<R> returnType;
  ....
  public HttpRequest(....,Class<R> returnType) // <=
  {      
    this.endpoint = endpoint;
    this.path = path;
    this.method = method;
    this.entity = entity;
  }
  ....
  public Class<R> getReturnType() 
  {
    return returnType;
  }
  ....
}
複製程式碼

在建構函式中，程式設計師忘記使用returnType引數，並給returnType欄位賦值。這就是為什麼呼叫getReturnType預設返回null。

相同的函式

V6032 很奇怪enable與disable方法的方法體完全相同。 ServiceAction.java(32),ServiceAction.java(36)

public class ServiceAction implements ModelEntity 
{    
  private String binary;
  private String host;
  private ServiceAction(String binary,String host) {
    this.binary = binary;
    this.host = host;
  }
  public static ServiceAction enable(String binary,String host) { // <=
    return new ServiceAction(binary,host);
  }
  public static ServiceAction disable(String binary,host);
  }
  ....
}
複製程式碼

有兩個完全一樣的方法並不是一個錯誤，但是兩個方法起一樣的作用至少看起來會很奇怪。看一看上面方法的名字，它們應該是執行相反的操作，但事實上它們卻完全相同，並返回ServiceAction物件，很可能disable函式是複製的enable的程式碼，方法體忘記修改了。

忘記重要的檢查

V6060 params在驗證是否為null之前使用。DomainService.java(49),DomainService.java(46)


public Domains list(Map<String,String> params)
{
  Preconditions.checkNotNull(params.get("page_size"),....);
  Preconditions.checkNotNull(params.get("page_number"),....);
  Invocation<Domains> domainInvocation = get(Domains.class,uri("/domains"));
  if (params != null) {                                      // <=
    ....
  }
  return domainInvocation.execute(this.buildExecutionOptions(Domains.class));
}
複製程式碼

params在if裡被檢查是否為null，但是在檢查之前，就已經呼叫了它的get方法，兩次，如果它是null的話，就會丟擲異常。

還有幾處類似的錯誤：

V6060 DomainService.java(389),DomainService.java(387)
V6060 DomainService.java(372),DomainService.java(369)
V6060 DomainService.java(353),DomainService.java(350)

總結

缺少雲服務，如今的大公司將難以運作。有很多人使用這些服務。因此，即使是一個小錯誤也會給很多人造成麻煩，公司也將為彌補這些錯誤造成的不良影響而產生額外的損失。一定要將人類的缺點考慮在內，人總是會犯錯誤，正如這篇文章中提到的，這一事實證明瞭需要有工具來提高程式碼的質量。

【譯】華為雲——程式碼質量“多雲”

介紹

專案分析

欄位初始化順序

不起眼的打字錯誤

條件預定義

空引用

Substrings與負數

被遺忘的結果

引數在建構函式未使用

相同的函式

忘記重要的檢查

總結

【譯】華為雲——程式碼質量“多雲”

【視訊】華為 Mate X2 深度體驗：它真的值 17999

【視訊】華為智慧屏 V 75 Super 上手體驗：主宰客廳影音的超大杯

【北京】華為職位招聘

【視訊】華為 MateBook E 上手體驗：辦公娛樂，一屏多用

【視訊】華為鴻蒙電視 V75 Pro 體驗評測，專治低清大糊片

【“網際網路+”大賽華為雲賽道】API命題攻略：釐清三步解題思路，用好開發工具

【“網際網路+”大賽華為雲賽道】EI命題攻略：華為雲EI的能力超豐富，助你實現AI夢想

【HMS Core 6.0全球上線】華為3D建模服務（3D Modeling Kit），輕鬆構建高質量3D模型

【技術乾貨】華為雲FusionInsight MRS的自研超級排程器Superior Scheduler

【譯】利用Lombok消除重複程式碼

【譯】在Azure上開發雲原生應用——工具與技巧

【譯】Pulumi：.NET Core雲架構

【初級篇】華為NAT技術（靜態NAT）

【初級篇】華為三層交換的配置

【IT之家開箱】華為 Mate X2 摺疊屏旗艦圖賞：大展於世的震撼

【IT之家評測室】華為 WATCH FIT 雅緻款評測：運動健康生活，從這塊方形表開始

【IT之家評測室】華為智慧體脂秤 3 Pro 測評：上個秤，測測你的肝臟脂肪

【IT之家評測室】華為 MateView 無線原色顯示器圖文評測：極簡、專業、智慧、互聯

【IT之家開箱】華為 MateBook 16 筆記本圖賞：大塊頭更有高顏值

【譯】華為雲——程式碼質量“多雲”

介紹

專案分析

欄位初始化順序

不起眼的打字錯誤

條件預定義

空引用

Substrings與負數

被遺忘的結果

引數在建構函式未使用

相同的函式

忘記重要的檢查

總結

相關推薦