Last Comment Bug 678785 - resource leak (not closed directory) in gfx/thebes/gfxFT2FontList.cpp
: resource leak (not closed directory) in gfx/thebes/gfxFT2FontList.cpp
[good first bug][mentor=jdm]
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86_64 Linux
-- normal (vote)
: mozilla9
Assigned To: Atul Aggarwal
: Milan Sreckovic [:milan]
Depends on:
Blocks: cppcheck
  Show dependency treegraph
Reported: 2011-08-13 22:04 PDT by David Volgyes
Modified: 2011-09-08 14:02 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

My proposed fix. (652 bytes, patch)
2011-08-13 22:07 PDT, David Volgyes
no flags Details | Diff | Splinter Review
Updated Patch (815 bytes, patch)
2011-09-01 13:55 PDT, Atul Aggarwal
jd.bugzilla: review+
Details | Diff | Splinter Review

Description User image David Volgyes 2011-08-13 22:04:46 PDT
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110622232440

Steps to reproduce:

Cppcheck found a resource leak in gfx/thebes/gfxFT2FontList.cpp
in gfxFT2FontList::FindFonts() method.

Actual results:

First of all, I think it's Android specific, because it is in a section wich begins with #elif defined(ANDROID).

In line #216 there is this:
 DIR *d = opendir("/system/fonts");

But nowhere is closed the directory.

Expected results:

You should close DIR *d, when it is not used anymore.
I have attached a proposed fix.
Comment 1 User image David Volgyes 2011-08-13 22:07:05 PDT
Created attachment 552934 [details] [diff] [review]
My proposed fix.
Comment 2 User image Tobias (:Tobbi) Markus 2011-08-13 22:09:36 PDT
Setting status to NEW
Comment 3 User image John Daggett (:jtd) 2011-08-16 21:07:21 PDT
I think you want to include a null check there, i.e.

  if (d) {
Comment 4 User image David Volgyes 2011-08-16 22:19:23 PDT
(In reply to John Daggett (:jtd) from comment #3)
> I think you want to include a null check there, i.e.
>   if (d) {
>     closedir(d);
>   }

Definitely. Thanks.
Comment 5 User image John Daggett (:jtd) 2011-08-23 05:06:10 PDT
David, could you post a revised patch so I can review+ it?
Comment 6 User image Ed Morley [:emorley] 2011-08-31 06:06:49 PDT
Unassigning from David per his request (bug 679610 comment 4).

For anyone looking at this bug, feel free to request it be assigned to you, David has very kindly provided a patch, but will not have time to follow it through.
Comment 7 User image Atul Aggarwal 2011-09-01 13:55:59 PDT
Created attachment 557632 [details] [diff] [review]
Updated Patch

Updated the patch
Comment 8 User image Marco Castelluccio [:marco] 2011-09-07 13:36:36 PDT

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