Closed Bug 873346 Opened 6 years ago Closed 6 years ago

Fix some sometimes-uninitialized warnings in layout

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: reuben, Assigned: u454397)

References

Details

Attachments

(1 file)

No description provided.
Attachment #750861 - Flags: review?(dbaron)
These are all cases where we do

int foo;
switch(GetValue()) {
case bar:
  foo = 1;
break;
case baz:
  foo = 2;
break;
default:
  NS_NOTREACHED("Invalid value returned by GetValue");
}

So there's no harm, but I figure it doesn't hurt to fix the warnings anyway.
Comment on attachment 750861 [details] [diff] [review]
Fix some warnings

I think we've had some previous discussions of some of these; there are certainly some cases where I prefer not to fix these warnings because fixing the gcc warning would suppress a substantially-more-accurate valgrind warning.  I haven't looked closely at these yet, though.
Comment on attachment 750861 [details] [diff] [review]
Fix some warnings

>diff --git a/layout/base/nsCSSRenderingBorders.cpp b/layout/base/nsCSSRenderingBorders.cpp
>--- a/layout/base/nsCSSRenderingBorders.cpp
>+++ b/layout/base/nsCSSRenderingBorders.cpp
>@@ -892,17 +892,17 @@ nsCSSBorderRenderer::DrawBorderSidesComp
> void
> nsCSSBorderRenderer::DrawBorderSides(int aSides)
> {
>   if (aSides == 0 || (aSides & ~SIDE_BITS_ALL) != 0) {
>     NS_WARNING("DrawBorderSides: invalid sides!");
>     return;
>   }
> 
>-  uint8_t borderRenderStyle;
>+  uint8_t borderRenderStyle = 0;

Please leave this as-is;  the early return quoted ensures that this variable will always be initialized.  The compiler isn't smart enough, but it is provable statically.

>diff --git a/layout/generic/nsImageMap.cpp b/layout/generic/nsImageMap.cpp
>--- a/layout/generic/nsImageMap.cpp
>+++ b/layout/generic/nsImageMap.cpp
>@@ -802,17 +802,17 @@ nsImageMap::AddArea(nsIContent* aArea)
> {
>   static nsIContent::AttrValuesArray strings[] =
>     {&nsGkAtoms::rect, &nsGkAtoms::rectangle,
>      &nsGkAtoms::circle, &nsGkAtoms::circ,
>      &nsGkAtoms::_default,
>      &nsGkAtoms::poly, &nsGkAtoms::polygon,
>      nullptr};
> 
>-  Area* area;
>+  Area* area = nullptr;

Instead of this, put:
  area = nullptr;
in the default case of the switch below.

>diff --git a/layout/style/nsStyleUtil.cpp b/layout/style/nsStyleUtil.cpp
>--- a/layout/style/nsStyleUtil.cpp
>+++ b/layout/style/nsStyleUtil.cpp
>@@ -351,17 +351,17 @@ nsStyleUtil::ComputeFunctionalAlternates
>     const nsCSSValue::Array *func = curr->mValue.GetArrayValue();
> 
>     // lookup propval
>     nsAutoString keywordStr;
>     func->Item(0).GetStringValue(keywordStr);
>     nsCSSKeyword key = nsCSSKeywords::LookupKeyword(keywordStr);
>     NS_ASSERTION(key != eCSSKeyword_UNKNOWN, "unknown alternate property value");
> 
>-    int32_t alternate;
>+    int32_t alternate = eFeatureAlternates_numFeatures;

Please leave this as is; it's not a sensible value, and I'd want valgrind to find any errors here.



So r=dbaron on the nsImageMap change revised as described; please skip the other two.
Attachment #750861 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/mozilla-central/rev/67661dd96576
Assignee: nobody → reuben.morais
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Duplicate of this bug: 846556
You need to log in before you can comment on or make changes to this bug.