Closed
Bug 774975
Opened 12 years ago
Closed 12 years ago
PluginModuleParent.cpp:243:1: warning: unused function 'RemoveMinidump' [-Wunused-function]
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 743573
People
(Reporter: ehsan.akhgari, Assigned: ujjwal.wadhawan)
Details
(Whiteboard: [build_warning][mentor=ehsan][lang=c++])
Attachments
(1 file)
2.04 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
/Users/ehsanakhgari/moz/mozilla-central/dom/plugins/ipc/PluginModuleParent.cpp:243:1: warning: unused function 'RemoveMinidump' [-Wunused-function] RemoveMinidump(nsIFile* minidump) ^ The issue is that RemoveMinidump is only used if MOZ_CRASHREPORTER_INJECTOR is #defined, but is defined if MOZ_CRASHREPORTER is #defined.
Assignee | ||
Comment 1•12 years ago
|
||
I believe we can safely move the function definition inside #ifdef MOZ_CRASHREPORTER_INJECTOR, closer to where it is being called.
Updated•12 years ago
|
Assignee: nobody → ujjwal.wadhawan
Assignee | ||
Comment 2•12 years ago
|
||
Quick check: As per this ticket, the warning has been raised om Mac OSX. I compiled and checked on Linux, where it's present as well. The above suggested change removes the warning. I am not sure If I have permission to push the change. Can you suggest? diff -r defbe00ca091 dom/plugins/ipc/PluginModuleParent.cpp --- a/dom/plugins/ipc/PluginModuleParent.cpp Sat Jul 21 14:32:25 2012 -0400 +++ b/dom/plugins/ipc/PluginModuleParent.cpp Sat Aug 04 23:51:12 2012 -0400 @@ -235,30 +235,16 @@ PluginModuleParent::ShouldContinueFromRe CrashReporterParent* PluginModuleParent::CrashReporter() { return static_cast<CrashReporterParent*>(ManagedPCrashReporterParent()[1]); } #endif #ifdef MOZ_CRASHREPORTER -static void -RemoveMinidump(nsIFile* minidump) -{ - if (!minidump) - return; - - minidump->Remove(false); - nsCOMPtr<nsIFile> extraFile; - if (GetExtraFileForMinidump(minidump, - getter_AddRefs(extraFile))) { - extraFile->Remove(true); - } -} - void PluginModuleParent::ProcessFirstMinidump() { CrashReporterParent* crashReporter = CrashReporter(); if (!crashReporter) return; AnnotationTable notes; @@ -271,16 +257,30 @@ PluginModuleParent::ProcessFirstMinidump } PRUint32 sequence = PR_UINT32_MAX; nsCOMPtr<nsIFile> dumpFile; nsCAutoString flashProcessType; TakeMinidump(getter_AddRefs(dumpFile), &sequence); #ifdef MOZ_CRASHREPORTER_INJECTOR + static void + RemoveMinidump(nsIFile* minidump) + { + if (!minidump) + return; + + minidump->Remove(false); + nsCOMPtr<nsIFile> extraFile; + if (GetExtraFileForMinidump(minidump, + getter_AddRefs(extraFile))) { + extraFile->Remove(true); + } + } + nsCOMPtr<nsIFile> childDumpFile; PRUint32 childSequence; if (mFlashProcess1 && TakeMinidumpForChild(mFlashProcess1, getter_AddRefs(childDumpFile), &childSequence)) { if (childSequence < sequence) {
Status: NEW → ASSIGNED
Comment 3•12 years ago
|
||
Two comments, Ujjwal: 1) I don't think the RemoveMinidump function should be moved inside another function. I didn't even know that that was allowed by standard C++. 2) You need to add an #endif after RemoveMinidump as well You're right that you cannot push this change yourself; you need to attach the patch to this bug and get a reviewer's approval, and then someone else will push it for you. See https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch#Attaching_the_patch for more information.
Assignee | ||
Comment 4•12 years ago
|
||
Updated•12 years ago
|
Attachment #649115 -
Flags: review?(benjamin)
Updated•12 years ago
|
Attachment #649115 -
Flags: review?(benjamin) → review+
Comment 5•12 years ago
|
||
Thanks a lot for taking the time to patch Mozilla, but I'm afraid I already fixed this issue over in bug 743573.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•