DotNetNuke

BUG OF THE MONTH | The ‘settings’ object was used before it was verified against null

public string NavigateURL(int tabID, 
                          bool isSuperTab, 
                          IPortalSettings settings, 
                          ....)
{
  ....
  if (isSuperTab)
  {
    url += "&portalid=" + settings.PortalId;
  }

  TabInfo tab = null;
  if (settings != null)
  {
    tab = TabController.Instance.GetTab(tabID, 
            isSuperTab ? Null.NullInteger : settings.PortalId, false);
  }
  ....
}

The PVS-Studio warning: V3095 The ‘settings’ object was used before it was verified against null. Check lines: 190, 195. DotNetNuke.Library NavigationManager.cs 190

I wonder why at first we access the settings.PortalId instance property, and then we check settings for null inequality. Thus, if settings – null and isSuperTab – true, we get NullReferenceException.

Surprisingly, this code fragment has a second contract that links isSuperTab and settings parameters – the ternary operator: isSuperTab ? Null.NullInteger : settings.PortalId. Note that in this case, unlike if, settings.PortalId is used when isSuperTab is false.

If isSuperTab is true, the settings.PortalId value is not processed. You may think that it’s just an implicit contract, and everything is fine.

Nope.

The code must be easy to read and understandable – you don’t have to think like Sherlock. If you intend to create this contract, write it explicitly in the code. Thus, the developers, the static analyzer, and you will not be confused. 😉

Please click here to see more bugs from this project.