1123553 - Fix -Wuninitialized warning in GonkDisplayJB.cpp by removing unnecessary error check

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Vendcom, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: Wuninitialized_GonkDisplayJB_error.patch

Bug 1032180 caused this -Wuninitialized warning when it removed the code that initialized `status_t error`, but not the code that checks whether `error == NO_ERROR`:

> widget/gonk/libdisplay/GonkDisplayJB.cpp:141:5: error: 'error' is used uninitialized in this function [-Werror=uninitialized]

NO_ERROR is 0. If local variable `error` is 0, then it's just pure luck that there is no garbage data on the call stack.

This bug affects B2G 1.4, 2.1, 2.2, and 3.0 (master) but *not* 2.0.

Comment 1

[Chris Peterson [:cpeterson]]

mwu: Should this fix be uplifted to the B2G 2.1 or 2.2 branches? Is JB relevant for 2.1 or 2.2?

This particular -Wuninitialized warning is not a serious bug, but it means that GonkDisplayJB might not always call StartBootAnimation.(). GonkDisplayICS does not have this bug.

btw, here is a green Try build:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4f6b3c4599ec

Comment 2

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/48409acb6e98

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/48409acb6e98

Comment 4

[Chris Peterson [:cpeterson]]

Comment on attachment 8551616 [details] [diff] [review]
Wuninitialized_GonkDisplayJB_error.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 2.1 fix for bug 1032180
User impact if declined: An uninitialized variable might cause the boot animation to intermittently not run on devices running Gonk JB or later.
Testing completed: I found this bug from a compiler warning about an uninitialized variable. I have not reproduced this bug myself.
Risk to taking this patch (and alternatives if risky): Low risk because this patch ensures the normal code path always runs.
String or UUID changes made by this patch: None.

Comment 5

[bhavana bajaj [:bajaj]]

(In reply to Chris Peterson [:cpeterson] from comment #4)
> Comment on attachment 8551616 [details] [diff] [review]
> Wuninitialized_GonkDisplayJB_error.patch
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): 2.1 fix for bug 1032180
> User impact if declined: An uninitialized variable might cause the boot
> animation to intermittently not run on devices running Gonk JB or later.
> Testing completed: I found this bug from a compiler warning about an
> uninitialized variable. I have not reproduced this bug myself.
> Risk to taking this patch (and alternatives if risky): Low risk because this
> patch ensures the normal code path always runs.
> String or UUID changes made by this patch: None.

cpeterson, does not look severe enough to uplift to b2g34/b2g37. I am not sure if we would need this on b2g37 as most of the builds are going to be shipped on "L". :mwu please correct me here and NI me if you think we should be uplifting this and I am missing the  user impact.

Comment 6

[Chris Peterson [:cpeterson]]

(In reply to bhavana bajaj [:bajaj] from comment #5)
> cpeterson, does not look severe enough to uplift to b2g34/b2g37. I am not
> sure if we would need this on b2g37 as most of the builds are going to be
> shipped on "L". :mwu please correct me here and NI me if you think we should
> be uplifting this and I am missing the  user impact.

OK. If no one is reporting boot animation problems, then uplifting this fix is a low priority.

btw, despite its filename, GonkDisplayJB.cpp is used on all Gonk versions >= JB, including LL.