Last Comment Bug 678982 - potential null pointer dereference in gfx/layers/d3d10/ImageLayerD3D10.cpp
: potential null pointer dereference in gfx/layers/d3d10/ImageLayerD3D10.cpp
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Windows 7
-- normal (vote)
: mozilla12
Assigned To: :aceman
: Milan Sreckovic [:milan]
Depends on:
Blocks: cppcheck
  Show dependency treegraph
Reported: 2011-08-15 07:33 PDT by David Volgyes
Modified: 2012-02-01 14:00 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

ImageLayerD3D10.diff (870 bytes, patch)
2011-08-15 07:33 PDT, David Volgyes
no flags Details | Diff | Splinter Review
fix per comment 1 (2.07 KB, patch)
2012-01-18 14:51 PST, :aceman
bas: review+
Details | Diff | Splinter Review
fix per nit (2.10 KB, patch)
2012-01-18 15:12 PST, :aceman
acelists: review+
Details | Diff | Splinter Review

Description User image David Volgyes 2011-08-15 07:33:53 PDT
Created attachment 553172 [details] [diff] [review]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110622232440

Steps to reproduce:

cppcheck 1.49 ( found a plenty of potential null pointer dereference. This is one of them.

Actual results:

in the line #53 there is a check for 'aSurface' is null or not,
but at line #65 there is an unchecked aSurface->
which is a potential null pointer dereference.

Expected results:

Well, it depends. I do not know the code well enough, but you definitely should check the pointer before dereference. A possible fix is attached, but this fix seems ugly for me. (But I have no better idea.)
Comment 1 User image Bas Schouten (:bas.schouten) 2011-08-15 07:50:25 PDT
Realistically, aSurface should never be NULL there, and if it is, we've got bigger problems! The right solution is probably to remove the NULL check at the top, and add a 'if (!aSurface) { return NULL; }' to the top of that function. I doubt that wouldn't break further down the flow though.
Comment 2 User image Ed Morley [:emorley] 2011-08-31 06:08:59 PDT
Unassigning from David per his request (bug 679610 comment 4).

For anyone looking at this bug, feel free to take it, David has very kindly provided a patch, but will not have time to follow it through.
Comment 3 User image :aceman 2012-01-18 14:51:20 PST
Created attachment 589659 [details] [diff] [review]
fix per comment 1
Comment 4 User image Bas Schouten (:bas.schouten) 2012-01-18 14:55:47 PST
Comment on attachment 589659 [details] [diff] [review]
fix per comment 1

Review of attachment 589659 [details] [diff] [review]:

::: gfx/layers/d3d10/ImageLayerD3D10.cpp
@@ +49,5 @@
>  SurfaceToTexture(ID3D10Device *aDevice,
>                   gfxASurface *aSurface,
>                   const gfxIntSize &aSize)
>  {
> +  if (!aSurface) { return NULL; }

nit: as per coding style

if (!aSurface) {
  return NULL;
Comment 5 User image :aceman 2012-01-18 15:12:57 PST
Created attachment 589667 [details] [diff] [review]
fix per nit

Carrying over review=bas.schouten .
Comment 7 User image Ed Morley [:emorley] 2012-01-19 17:49:00 PST

Note You need to log in before you can comment on or make changes to this bug.