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

RESOLVED DUPLICATE of bug 743573

Status

()

Core
Plug-ins
RESOLVED DUPLICATE of bug 743573
6 years ago
6 years ago

People

(Reporter: Ehsan, Assigned: Ujjwal Wadhawan)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning][mentor=ehsan][lang=c++])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
/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

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

Updated

6 years ago
Assignee: nobody → ujjwal.wadhawan
(Assignee)

Comment 2

6 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

6 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

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

Updated

6 years ago
Attachment #649115 - Flags: review?(benjamin)

Updated

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 743573
You need to log in before you can comment on or make changes to this bug.