resource leak (not closed directory) in gfx/thebes/gfxFT2FontList.cpp

RESOLVED FIXED in mozilla9

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: David Volgyes, Assigned: Atul Aggarwal)

Tracking

(Blocks: 1 bug)

Trunk
mozilla9
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=jdm])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
Created attachment 552934 [details] [diff] [review]
My proposed fix.

Updated

6 years ago
Assignee: nobody → david.volgyes
Component: General → Graphics
Product: Firefox → Core
QA Contact: general → thebes
Setting status to NEW
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

6 years ago
Blocks: 679417

Updated

6 years ago
Attachment #552934 - Attachment is patch: true
Attachment #552934 - Flags: review?(jdaggett)

Comment 3

6 years ago
I think you want to include a null check there, i.e.

  if (d) {
    closedir(d);
  }

Updated

6 years ago
Whiteboard: [good first bug] [mentor=jdm]
(Reporter)

Comment 4

6 years ago
(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

6 years ago
David, could you post a revised patch so I can review+ it?

Comment 6

6 years ago
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.
Assignee: david.volgyes → nobody
Whiteboard: [good first bug] [mentor=jdm] → [good first bug] [mentor=jdm] [has patch, needs new assignee]
(Assignee)

Comment 7

6 years ago
Created attachment 557632 [details] [diff] [review]
Updated Patch

Updated the patch
Assignee: nobody → atulagrwl
Attachment #552934 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #552934 - Flags: review?(jdaggett)
Attachment #557632 - Flags: review?(jdaggett)

Updated

6 years ago
Attachment #557632 - Flags: review?(jdaggett) → review+
Checkin?

Updated

6 years ago
Keywords: checkin-needed

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: [good first bug] [mentor=jdm] [has patch, needs new assignee] → [good first bug][mentor=jdm][inbound]
http://hg.mozilla.org/mozilla-central/rev/120aedf2c84f
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=jdm][inbound] → [good first bug][mentor=jdm]
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.