Closed Bug 774975 Opened 13 years ago Closed 13 years ago

PluginModuleParent.cpp:243:1: warning: unused function 'RemoveMinidump' [-Wunused-function]

Categories

(Core Graveyard :: Plug-ins, defect)

x86
macOS
defect
Not set
normal

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)

/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.
I believe we can safely move the function definition inside #ifdef MOZ_CRASHREPORTER_INJECTOR, closer to where it is being called.
Assignee: nobody → ujjwal.wadhawan
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
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.
Attachment #649115 - Flags: review?(benjamin)
Attachment #649115 - Flags: review?(benjamin) → review+
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: 13 years ago
Resolution: --- → DUPLICATE
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: