The default bug view has changed. See this FAQ.

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?
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.