Last Comment Bug 678785 - resource leak (not closed directory) in gfx/thebes/gfxFT2FontList.cpp
: resource leak (not closed directory) in gfx/thebes/gfxFT2FontList.cpp
Status: RESOLVED FIXED
[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
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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 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 David Volgyes 2011-08-13 22:07:05 PDT
Created attachment 552934 [details] [diff] [review]
My proposed fix.
Comment 2 [retired] 2011-08-13 22:09:36 PDT
Setting status to NEW
Comment 3 John Daggett (:jtd) 2011-08-16 21:07:21 PDT
I think you want to include a null check there, i.e.

  if (d) {
    closedir(d);
  }
Comment 4 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 John Daggett (:jtd) 2011-08-23 05:06:10 PDT
David, could you post a revised patch so I can review+ it?
Comment 6 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 Atul Aggarwal 2011-09-01 13:55:59 PDT
Created attachment 557632 [details] [diff] [review]
Updated Patch

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

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