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