710969 - Possible bad null-pointer check in nsFrameSelection::MoveCaret

Status: RESOLVED INVALID

(Core :: Layout, defect)

Description

[Justin Dolske [:Dolske]]

From http://www.viva64.com/en/a/0078/

Example 3. Checking a pointer for NULL only after it had been used

nsresult
v(...)
{
  ...
  mShell->FlushPendingNotifications(Flush_Layout);
  if (!mShell) {
    return NS_OK;
  }
  ...
}

PVS-Studio diagnostic message: V595 The 'mShell' pointer was utilized before it was verified against nullptr. Check lines: 1107, 1109. nsselection.cpp 1107

If the pointer is equal to null, we must handle this special occasion and return NS_OK from the function. What confuses me is that the mShell pointer has already been used before this moment.

Probably, this code must is operational only because the mShell pointer never equals to NULL. I cite this example to demonstrate that one can easily make a mistake even in the simplest of checks. We have it but still it is useless.

Comment 1

[Justin Dolske [:Dolske]]

bz points out on IRC that this code is good. There's already a null-check above it via NS_ENSURE_STATE(), and the analysis incorrectly assumes FlushPendingNotifications() won't change mShell -- which it apparently can and does!

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yep.  This code is pure and wholesome.