829993 - nsCSSRenderingBorders.cpp:907:21: warning: variable 'borderRenderStyle' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

Build warning:
{
../../../../layout/base/nsCSSRenderingBorders.cpp:907:21: warning: variable 'borderRenderStyle' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
  NS_FOR_CSS_SIDES (i) {
                    ^~
../../../../layout/base/nsStyleConsts.h:22:76: note: expanded from macro 'NS_FOR_CSS_SIDES'
#define NS_FOR_CSS_SIDES(var_) for (mozilla::css::Side var_ = NS_SIDE_TOP; var_ <= NS_SIDE_LEFT; var_++)
                                                                           ^
../../../../layout/base/nsCSSRenderingBorders.cpp:916:7: note: uninitialized use occurs here
  if (borderRenderStyle == NS_STYLE_BORDER_STYLE_NONE ||
      ^~~~~~~~~~~~~~~~~
../../../../layout/base/nsCSSRenderingBorders.cpp:907:21: note: remove the condition if it is always true
  NS_FOR_CSS_SIDES (i) {
                    ^
../../../../layout/base/nsStyleConsts.h:22:76: note: expanded from macro 'NS_FOR_CSS_SIDES'
#define NS_FOR_CSS_SIDES(var_) for (mozilla::css::Side var_ = NS_SIDE_TOP; var_ <= NS_SIDE_LEFT; var_++)
                                                                           ^
../../../../layout/base/nsCSSRenderingBorders.cpp:899:28: note: initialize the variable 'borderRenderStyle' to silence this warning
  uint8_t borderRenderStyle;
                           ^
                            = '\0'
1 warning generated.
}

This is visible in e.g. the context in this build log (though it's not responsible for the build failure there -- it just happens to be in the same area of the shortlog):
https://tbpl.mozilla.org/php/getParsedLog.php?id=18748969&tree=Mozilla-Inbound

Looks like we probably want to initialize borderRenderStyle to NS_STYLE_BORDER_STYLE_NONE (?)

This code dates back to this cset:
https://hg.mozilla.org/mozilla-central/rev/f34a9f851cd1#l4.669
from bug 424423.

Comment 1

[Josh Aas]

Attachment: Fix v1.0

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 813982 [details] [diff] [review]
Fix v1.0

I'd rather not make changes like this; I think it's more valuable to gave valgrind warnings when there are real problems than cover them up to silence compiler warnings when the compiler isn't able to analyze the code fully.

Comment 3

[Cameron McCormack (:heycam)]

Is there another way we can silence the clang warning, with a #pragma maybe?

Comment 5

[John Daggett (:jtd)]

Attachment: alternate patch, wrap pragmas around borderRenderStyle to squelch compiler warning

Not the prettiest thing to behold, but this squelches the warning.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8339781 [details] [diff] [review]
alternate patch, wrap pragmas around borderRenderStyle to squelch compiler warning

This is even uglier; I guess I'd rather take Josh's patch.

Comment 7

[John Daggett (:jtd)]

Pushed Josh's patch to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b395bc2b9ba9

Comment 8

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/mozilla-central/rev/b395bc2b9ba9