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

Status: RESOLVED DUPLICATE of bug 743573

Description

[:Ehsan Akhgari]

/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.

Comment 1

[Ujjwal Wadhawan]

I believe we can safely move the function definition inside #ifdef MOZ_CRASHREPORTER_INJECTOR, closer to where it is being called.

Comment 2

[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) {

Comment 3

[Josh Matthews [:jdm]]

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.

Comment 4

[Ujjwal Wadhawan]

Created attachment 649115 [details] [diff] [review]
Proposed changes to removed compilation warning.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Thanks a lot for taking the time to patch Mozilla, but I'm afraid I already fixed this issue over in bug 743573.