refresh_product_info_block leaks file handle on failure

RESOLVED FIXED in mozilla34

Status

()

Toolkit
Application Update
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: mccr8, Assigned: Shashank VRSN Sabniveesu, Mentored)

Tracking

({coverity, memory-leak})

unspecified
mozilla34
coverity, memory-leak
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [CID 1123276] )

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
fp = fopen(path, "r+b");
  ...
    if (fread(&additionalBlockSize, 
              sizeof(additionalBlockSize), 
              1, fp) != 1) {
      // fp isn't closed
      return -1;
    }
  ...
    if (fread(&additionalBlockID, 
              sizeof(additionalBlockID), 
              1, fp) != 1) {
      // fp isn't closed
      return -1;
    }

Comment 1

4 years ago
Can you provide a DXR link to that, and I'll shop it around?
Whiteboard: [CID 1123276] → [CID 1123276] [mentor=mccr8]
Mentor: continuation@gmail.com
Whiteboard: [CID 1123276] [mentor=mccr8] → [CID 1123276]
(Assignee)

Comment 3

4 years ago
Do two simple 'fclose(fp)' statements suffice?
(Reporter)

Comment 4

4 years ago
Yes, I think so.
(Assignee)

Comment 5

3 years ago
Created attachment 8479512 [details] [diff] [review]
BUG 1021142 - 'fclose()' at 2 places to avoid leaks on failure r=mccr8
Attachment #8479512 - Flags: review?(continuation)
(Reporter)

Comment 6

3 years ago
Comment on attachment 8479512 [details] [diff] [review]
BUG 1021142 - 'fclose()' at 2 places to avoid leaks on failure r=mccr8

Review of attachment 8479512 [details] [diff] [review]:
-----------------------------------------------------------------

Is this something you could review, Brian?  Thanks.
Attachment #8479512 - Flags: review?(continuation) → review?(netzen)
Comment on attachment 8479512 [details] [diff] [review]
BUG 1021142 - 'fclose()' at 2 places to avoid leaks on failure r=mccr8

Review of attachment 8479512 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay and thanks for the patch!
Attachment #8479512 - Flags: review?(netzen) → review+
(Reporter)

Comment 8

3 years ago
I'll do a try run later today.
Assignee: nobody → shashank
Flags: needinfo?(continuation)
(Reporter)

Comment 9

3 years ago
https://tbpl.mozilla.org/?tree=Try&rev=a2333e4a5ec3
Flags: needinfo?(continuation)
(Reporter)

Comment 10

3 years ago
Try looks good.  Thanks for the patch!
Keywords: checkin-needed
(Reporter)

Comment 11

3 years ago
Well, I can just land it myself.

https://hg.mozilla.org/integration/mozilla-inbound/rev/af568e909c62
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/af568e909c62
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.