Closed Bug 1123553 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox OS Graveyard :: Vendcom, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v1.4 affected, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 affected, b2g-v2.1S affected, b2g-v2.2 affected, b2g-master fixed)

RESOLVED FIXED
Tracking Status
b2g-v1.4 --- affected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- affected
b2g-v2.1S --- affected
b2g-v2.2 --- affected
b2g-master --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Attachment #8551616 - Flags: review?(mwu)
Attachment #8551616 - Flags: review?(mwu) → review+
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
Flags: needinfo?(mwu)
https://hg.mozilla.org/mozilla-central/rev/48409acb6e98
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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.
Flags: needinfo?(mwu)
Attachment #8551616 - Flags: approval-mozilla-b2g37?
Attachment #8551616 - Flags: approval-mozilla-b2g34?
(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.
Attachment #8551616 - Flags: approval-mozilla-b2g37?
Attachment #8551616 - Flags: approval-mozilla-b2g37-
Attachment #8551616 - Flags: approval-mozilla-b2g34?
Attachment #8551616 - Flags: approval-mozilla-b2g34-
(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.
You need to log in before you can comment on or make changes to this bug.