The default bug view has changed. See this FAQ.

memory leak in widget/src/android/AndroidBridge.cpp

RESOLVED FIXED in mozilla9

Status

()

Core
Widget: Android
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: David Volgyes, Assigned: dougt)

Tracking

(Blocks: 1 bug, {mlk, mobile})

Trunk
mozilla9
ARM
Android
mlk, mobile
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 553182 [details] [diff] [review]
AndroidBridge.diff

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110622232440

Steps to reproduce:

cppcheck 1.49 (http://cppcheck.sourceforge.net/) found a plenty of potential null pointer dereference. This is one of them.


Actual results:

cppcheck tells everything:
widget/src/android/AndroidBridge.cpp:87]: (error) Allocation with strdup, putenv doesn't release it.

The corresponding line is that:
    putenv(strdup("NSS_DISABLE_UNLOAD=1"));





Expected results:

I see that strdup should solve the "constness" problem.
However, you should release the memory after putenv.
char*tmp=strdup("NSS_DISABLE_UNLOAD=1");
putenv(tmp);
free(tmp);

This possible solution is attached.
(Reporter)

Updated

6 years ago
Version: 5 Branch → Trunk

Updated

6 years ago
Component: General → Widget: Android
Product: Firefox → Core
QA Contact: general → android

Updated

6 years ago
Attachment #553182 - Flags: review?(blassey.bugs)

Updated

6 years ago
Blocks: 679417
(Assignee)

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 553182 [details] [diff] [review]
AndroidBridge.diff

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

::: widget/src/android/AndroidBridge.cpp
@@ +85,5 @@
>       * Conveniently, NSS has an env var that can prevent it from unloading.
>       */
> +    char* tmp=strdup("NSS_DISABLE_UNLOAD=1");
> +    putenv(tmp);
> +    free(tmp);

how about this?
static char* nss_disable_unload = "NSS_DISABLE_UNLOAD=1";
putenv(nss_disable_unload);
Attachment #553182 - Flags: review?(blassey.bugs)
(Reporter)

Comment 2

6 years ago
Nice. Faster and safer.
Keywords: mlk, mobile
OS: Linux → Android
Hardware: x86_64 → ARM
Whiteboard: [MemShrink]
Assignee: nobody → david.volgyes

Comment 3

6 years ago
It would be nice for someone to pick this up. All that needs to be done is modify the patch to follow Brad's suggestion in comment 1.
Whiteboard: [MemShrink] → [MemShrink][good first bug][mentor=jdm]
Unassigning from David per his request (bug 679610 comment 4).

For anyone looking at this bug, feel free to take it, David has very kindly provided a patch, but will not have time to follow it through.
Assignee: david.volgyes → nobody
Whiteboard: [MemShrink][good first bug][mentor=jdm] → [MemShrink][good first bug][mentor=jdm][has patch, needs new assignee]
(Assignee)

Updated

6 years ago
Assignee: nobody → doug.turner
(Assignee)

Comment 5

6 years ago
Created attachment 557213 [details] [diff] [review]
patch v.2
Attachment #553182 - Attachment is obsolete: true
Attachment #557213 - Flags: review?(azakai)

Comment 6

6 years ago
Wouldn't just removing the strdup work here?
(Assignee)

Comment 7

6 years ago
that work work too.  r+ w/ that change?

Comment 8

6 years ago
Yes.
(Assignee)

Comment 9

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/f872cb091586
Whiteboard: [MemShrink][good first bug][mentor=jdm][has patch, needs new assignee] → [MemShrink][good first bug][mentor=jdm]
http://hg.mozilla.org/mozilla-central/rev/f872cb091586
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
(Assignee)

Updated

6 years ago
Attachment #557213 - Flags: review?(azakai) → review+
You need to log in before you can comment on or make changes to this bug.