potential crash [@ nsPluginHost::HandleBadPlugin] if !aInstance

RESOLVED FIXED

Status

()

defect
--
critical
RESOLVED FIXED
9 years ago
Last year

People

(Reporter: timeless, Assigned: timeless)

Tracking

(Blocks 1 bug, {coverity, crash})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, )

Attachments

(1 attachment, 1 obsolete attachment)

5.53 KB, patch
timeless
: review+
Details | Diff | Splinter Review
3184 nsPluginHost::HandleBadPlugin(PRLibrary* aLibrary, nsIPluginInstance *aInstance)
3185 {

3198   if (aInstance)
3199     aInstance->GetOwner(getter_AddRefs(owner));

3237     nsNPAPIPluginInstance *instance = static_cast<nsNPAPIPluginInstance*>(aInstance);
3238 
3239     nsNPAPIPlugin *plugin = instance->GetPlugin();
Posted patch patch (obsolete) — Splinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #493557 - Flags: review?(joshmoz)
Comment on attachment 493557 [details] [diff] [review]
patch

>-    nsXPIDLString brandName;
>-    if (NS_FAILED(rv = bundle->GetStringFromName(NS_LITERAL_STRING("brandShortName").get(),
>-                                 getter_Copies(brandName))))
>-      return rv;

I'd prefer if for readability you put "rv = ..." on its own line above "if (NS_FAILED(...". This pattern shows up a few times in the patch.

>-    // add plugin name to the message
>-    nsCString pluginname;
>-    if (!pluginTag->mName.IsEmpty())
>-      pluginname = pluginTag->mName;
>-    else
>-      pluginname = pluginTag->mFileName;

While you're at it can you put brackets around these if statements? I don't care much about simple return statements but I prefer bracketed if statements for other things.

Otherwise looks good, thanks.
Attachment #493557 - Flags: review?(joshmoz) → review+
Posted patch as requestedSplinter Review
Attachment #493557 - Attachment is obsolete: true
Attachment #495391 - Flags: review+
Attachment #495391 - Flags: approval2.0?
Attachment #495391 - Flags: approval2.0? → approval2.0+
Keywords: checkin-needed
Pushed http://hg.mozilla.org/mozilla-central/rev/e741896a62ae
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Crash Signature: [@ nsPluginHost::HandleBadPlugin]
You need to log in before you can comment on or make changes to this bug.