Closed
Bug 1421826
Opened 8 years ago
Closed 7 years ago
ContainerState::ProcessDisplayItems: Unused value (UNUSED_VALUE)
Categories
(Core :: Web Painting, enhancement, P3)
Core
Web Painting
Tracking
()
RESOLVED
INVALID
People
(Reporter: Sylvestre, Unassigned, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: coverity, good-first-bug, Whiteboard: [good first bug][lang=C++][CID 1419983])
Attachments
(1 file)
|
1.05 KB,
patch
|
mattwoodrow
:
review+
Sylvestre
:
feedback+
|
Details | Diff | Splinter Review |
A trivial good first bug to understand the workflows.
returned_pointer: Assigning value from iter.GetNext() to i here, but that stored value is overwritten before it can be used.
https://dxr.mozilla.org/mozilla-central/source/layout/painting/FrameLayerBuilder.cpp?q=FrameLayerBuilder.cpp&redirect_type=direct#4018
| Reporter | ||
Updated•8 years ago
|
Mentor: sledru
| Reporter | ||
Comment 2•8 years ago
|
||
sure, please submit a patch!
| Reporter | ||
Comment 5•8 years ago
|
||
Comment on attachment 8933618 [details] [diff] [review]
not storing the value of iter.GetNext()
I am not certain about the second change but it looks good.
You should now find for an official reviewer!
Attachment #8933618 -
Flags: feedback+
Comment 6•8 years ago
|
||
Comment on attachment 8933618 [details] [diff] [review]
not storing the value of iter.GetNext()
Review of attachment 8933618 [details] [diff] [review]:
-----------------------------------------------------------------
I'm actually not a huge fan of this warning, since if anyone tries to use 'i' (even though nobody is now), leaving it as the old value will be incorrect. I guess that's my fault for not having a proper iterator instance.
Attachment #8933618 -
Flags: review+
Updated•8 years ago
|
Priority: -- → P3
Hi all,
As Matt mentioned above, currently there is no one use this variable "i", could I simply just replace the declaration in while loop condition: "nsDisplayItem* i" to "nsDisplayItem* item" on line 3989 and remove line 3990 like
> @@ -3986,8 +3986,7 @@
> int layerCount = 0;
>
> FlattenedDisplayItemIterator iter(mBuilder, aList);
> - while (nsDisplayItem* i = iter.GetNext()) {
> - nsDisplayItem* item = i;
> + while (nsDisplayItem* item = iter.GetNext()) {
> MOZ_ASSERT(item);
>
> DisplayItemType itemType = item->GetType();
I'm not sure if it will affect any flexibility for this function/module, how do you think?
Comment 9•8 years ago
|
||
(In reply to samuel from comment #7)
> Hi all,
>
> As Matt mentioned above, currently there is no one use this variable "i",
> could I simply just replace the declaration in while loop condition:
> "nsDisplayItem* i" to "nsDisplayItem* item" on line 3989 and remove line
> 3990 like
>
> > @@ -3986,8 +3986,7 @@
> > int layerCount = 0;
> >
> > FlattenedDisplayItemIterator iter(mBuilder, aList);
> > - while (nsDisplayItem* i = iter.GetNext()) {
> > - nsDisplayItem* item = i;
> > + while (nsDisplayItem* item = iter.GetNext()) {
> > MOZ_ASSERT(item);
> >
> > DisplayItemType itemType = item->GetType();
>
> I'm not sure if it will affect any flexibility for this function/module, how
> do you think?
Yes, that sounds reasonable.
Flags: needinfo?(matt.woodrow)
Comment 10•7 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:samuel.chen, could you have a look please?
Flags: needinfo?(samuel.chen)
| Reporter | ||
Comment 11•7 years ago
|
||
Unassigning
Samuel, if you are are still interested, please try to address the issue
Assignee: samuel.chen → nobody
Severity: normal → trivial
Comment 12•7 years ago
|
||
Hey i'm new here, is it okay if i pick this up?
| Reporter | ||
Comment 13•7 years ago
|
||
Sure, as nothing happened for the last 11 months, it is safe
Comment 14•7 years ago
|
||
Ahh, I'll get to it then :)
Comment 15•7 years ago
|
||
Hey it seems that the link is outdated. I can't seem to locate the code referenced here. Can you please help me out? :)
Comment 16•7 years ago
|
||
@sylvestre, is this still an outstanding issue? Looking at the commit history on the file, it appears that all the references to the iterator "i" variable have been eliminated between several diffs submitted over the past few months. Sample Diff, Current File.
Comment 17•7 years ago
|
||
Just wanted to follow up on this. Based on my findings in Comment 16, is this still an open issue?
Comment 18•7 years ago
|
||
No, this code has all been rewritten since the bug was filed sorry.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
Updated•7 years ago
|
Flags: needinfo?(samuel.chen)
You need to log in
before you can comment on or make changes to this bug.
Description
•