1144625 - Useless initialization of rect in nsRegion::SimplifyOutwardByArea

Status: RESOLVED INVALID

(Core :: Graphics, defect, P3)

Description

[Quentin BOUILLAGUET]

Attachment: patch.diff

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150306140254

Steps to reproduce:

In nsRegion::SimplifyOutwardByArea (line 315), the value stored is never read.
Moreover, it is used only on the first if statement (line 335).

Comment 1

[Ryan VanderMeulen [:RyanVM]]

checkin-needed is used to signify patches that are ready to land, which comes after they've gone through review :)

Also, please see the link below and be sure to generate a patch that contains the needed commit information for landing:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Thanks for the patch!

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8579310 [details] [diff] [review]
patch.diff

Review of attachment 8579310 [details] [diff] [review]:
-----------------------------------------------------------------

How did you find this? Was it with an automated tool?

::: gfx/src/nsRegion.cpp
@@ +311,4 @@
>    tmpStorage.SetCapacity(n);
>    pixman_box32_t *tmpRect = tmpStorage.Elements();
>  
> +  pixman_box32_t *destRect = boxes; 

There's a bit of added white space here. Please remove it.

Comment 3

[Quentin BOUILLAGUET]

Attachment: patch.diff

Hi !
I found this with an automated tool.
I removed the space and followed the instructions to generate this patch !

Comment 4

[Jeff Muizelaar [:jrmuizel]]

(In reply to Quentin BOUILLAGUET from comment #3)
> Created attachment 8580247 [details] [diff] [review]
> patch.diff
> 
> Hi !
> I found this with an automated tool.
> I removed the space and followed the instructions to generate this patch !

What was the tool?

Comment 5

[Quentin BOUILLAGUET]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> (In reply to Quentin BOUILLAGUET from comment #3)
> > Created attachment 8580247 [details] [diff] [review]
> > patch.diff
> > 
> > Hi !
> > I found this with an automated tool.
> > I removed the space and followed the instructions to generate this patch !
> 
> What was the tool?


I'm currently a student.
Our professor gave us a list of some easy fixes to learn how to contribute to mozilla.
So... I don't know wich tools he used.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

I'd be very interested in knowing what tool it was.

Comment 7

[Quentin BOUILLAGUET]

(In reply to Quentin BOUILLAGUET from comment #5)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> > (In reply to Quentin BOUILLAGUET from comment #3)
> > > Created attachment 8580247 [details] [diff] [review]
> > > patch.diff
> > > 
> > > Hi !
> > > I found this with an automated tool.
> > > I removed the space and followed the instructions to generate this patch !
> > 
> > What was the tool?

Hello !
The automated tool we used was scan-build from clang.
> 
> 
> I'm currently a student.
> Our professor gave us a list of some easy fixes to learn how to contribute
> to mozilla.
> So... I don't know wich tools he used.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Jeff, I did a presentation at this university . Their teacher is a good friend and I gave him some material.
Drop me an email if you want to get access to it.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/277d8a11dc53

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Backed out for causing various Gaia python integration test failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9d5b79e51a8

https://treeherder.mozilla.org/logviewer.html#?job_id=8549742&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=8549826&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=8549846&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=8549825&repo=mozilla-inbound

Comment 11

[u279076]

Jeff, is this bug still relevant?

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

This code has since been rewritten