Last Comment Bug 605174 - Fix build warnings in dom/
: Fix build warnings in dom/
Status: RESOLVED FIXED
[build_warning]
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Ed Morley [:emorley]
:
Mentors:
Depends on:
Blocks: buildwarning
  Show dependency treegraph
 
Reported: 2010-10-18 10:26 PDT by :Ms2ger
Modified: 2011-09-19 13:09 PDT (History)
2 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (13.70 KB, patch)
2010-10-18 10:26 PDT, :Ms2ger
no flags Details | Diff | Splinter Review
Patch v2 (12.67 KB, patch)
2010-10-21 13:29 PDT, :Ms2ger
no flags Details | Diff | Splinter Review
Patch v2.1 (12.71 KB, patch)
2010-11-20 04:03 PST, :Ms2ger
no flags Details | Diff | Splinter Review
Patch v3 (16.69 KB, patch)
2011-09-01 05:50 PDT, Ed Morley [:emorley]
peterv: review+
Details | Diff | Splinter Review
Patch for checkin (13.21 KB, patch)
2011-09-17 09:29 PDT, Ed Morley [:emorley]
no flags Details | Diff | Splinter Review
Patch for checkin v2 (11.98 KB, patch)
2011-09-18 12:08 PDT, Ed Morley [:emorley]
no flags Details | Diff | Splinter Review

Description :Ms2ger 2010-10-18 10:26:41 PDT
Created attachment 484011 [details] [diff] [review]
Patch v1

Mainly uninitialized and unused variables.
Comment 1 :Ms2ger 2010-10-21 13:29:14 PDT
Created attachment 485126 [details] [diff] [review]
Patch v2

Merged to tip
Comment 2 :Ms2ger 2010-11-20 04:03:35 PST
Created attachment 492050 [details] [diff] [review]
Patch v2.1

Merged to tip.
Comment 3 Ed Morley [:emorley] 2011-09-01 05:50:53 PDT
Created attachment 557472 [details] [diff] [review]
Patch v3

Updated to tip & some new warning fixes added.

Peter, ping for review (the previous review has been open for 10 months) - thanks :-)
Comment 4 Ed Morley [:emorley] 2011-09-01 06:55:08 PDT
Just spotted an #ifdef DEBUG instance that can be converted to DebugOnly in one of the dom/ipc/ContentParent.cpp hunks. Will make the change after first pass review.
Comment 5 Peter Van der Beken [:peterv] - away till Aug 1st 2011-09-02 05:18:25 PDT
Comment on attachment 557472 [details] [diff] [review]
Patch v3

Review of attachment 557472 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think we should change for bogus warnings, rest looks ok.

::: dom/base/nsDOMClassInfo.cpp
@@ +7342,5 @@
>  static PRBool
>  GetBindingURL(Element *aElement, nsIDocument *aDocument,
>                nsCSSValue::URL **aResult)
>  {
> +  *aResult = nsnull;

Bogus warning, I don't see why we should change this.

@@ +8448,5 @@
>        return JS_TRUE;
>      }
>    }
>  
> +  nsHTMLDocument* doc = GetDocument(cx, obj);

Follow existing style, don't change this.

@@ +8450,5 @@
>    }
>  
> +  nsHTMLDocument* doc = GetDocument(cx, obj);
> +  nsISupports* result = NULL;
> +  nsWrapperCache* cache = NULL;

Bogus warning, I don't see why we should change this.

::: dom/base/nsDOMException.cpp
@@ +87,5 @@
>      NS_WARNING("Trying to create an exception for the wrong error module."); \
>      return NS_ERROR_FAILURE;                                                 \
>    }                                                                          \
> +  const char* name = NULL;                                                   \
> +  const char* message = NULL;                                                \

Fix this in NSResultToNameAndMessage.

::: dom/base/nsGlobalWindow.cpp
@@ +9208,5 @@
>      ++gRunningTimeoutDepth;
>      ++mTimeoutFiringDepth;
>  
>      PRBool trackNestingLevel = !timeout->mIsInterval;
> +    PRUint32 nestingLevel = 0;

Bogus warning, I don't see why we should change this.

::: dom/base/nsScriptNameSpaceManager.cpp
@@ +294,5 @@
>    nsCAutoString category_entry;
>    const char* if_name;
>    nsCOMPtr<nsISupports> entry;
>    nsCOMPtr<nsIInterfaceInfo> if_info;
> +  PRBool found_old = PR_FALSE, dom_prefix;

Bogus warning, I don't see why we should change this.

::: dom/indexedDB/LazyIdleThread.cpp
@@ +512,5 @@
>  NS_IMETHODIMP
>  LazyIdleThread::AfterProcessNextEvent(nsIThreadInternal* /* aThread */,
>                                        PRUint32 /* aRecursionDepth */)
>  {
> +  bool shouldNotifyIdle = false;

Bogus warning, I don't see why we should change this.

::: dom/ipc/ContentParent.cpp
@@ +488,5 @@
>  
>      nsCOMPtr<nsISimpleEnumerator> enumerator;
> +#ifdef DEBUG
> +    nsresult rv = 
> +#endif

DebugOnly<...>?

::: dom/plugins/ipc/PluginModuleParent.cpp
@@ +618,5 @@
>  
>  PluginIdentifierParent*
>  PluginModuleParent::GetIdentifierForNPIdentifier(NPP npp, NPIdentifier aIdentifier)
>  {
> +    PluginIdentifierParent* ident = NULL;

Bogus warning, I don't see why we should change this.
Comment 6 Ed Morley [:emorley] 2011-09-17 09:29:26 PDT
Created attachment 560716 [details] [diff] [review]
Patch for checkin

Changes made for review comments, carrying forwards r+.

Builds fine locally, sent to try:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=db82af19db7c
Comment 7 Ed Morley [:emorley] 2011-09-18 12:08:00 PDT
Created attachment 560805 [details] [diff] [review]
Patch for checkin v2

As comment 6 patch, but with dom/plugins/ipc/PluginModuleChild.cpp @@ -254,18 +252,17 @@ PluginModuleChild::Init(const std::strin hunk added to prevent OSX build error + the nsIDOMXULMultSelectCntrlEl.idl hunk removed as it caused win Moth and crashtest failures.

Carrying forwards r+.
Comment 9 Ed Morley [:emorley] 2011-09-19 13:09:49 PDT
https://hg.mozilla.org/mozilla-central/rev/d4826f400c89

Note You need to log in before you can comment on or make changes to this bug.