1172011 - nsSplitterFrame.cpp: Value stored to 'spaceLeft' is never read

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

Detected by scan-build:
https://people.mozilla.org/~sledru/reports/fx-scan-build/report-nsSplitterFrame.cpp-ResizeChildTo-13-1.html#EndPath

here
https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsSplitterFrame.cpp?from=nsSplitterFrame.cpp&case=true#1051
we should probably remove the declaration, it is useless and potentially missleading.

Comment 1

[Riley Baldin]

Attachment: removingDeclaration_diff.txt

Hi Sylvestre, I've removed the declaration in this patch.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Riley, this bug is already assigned. You should find a bug without an assigned dev.

Comment 3

[Riley Baldin]

Sorry, it just seemed trivial and I wasn't sure the assignment to a Mozilla VP was legitimate.  Moving on.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

It was ;)
Even for a VP, it is interesting to understand what is the contribution workflow.

Anyway, I have many other bugs like that. Drop me an email if you are interested.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Actually, David has another bug assigned and I can give him more if he wants.
Riley, about your patch, you should now find a reviewer (trick: look at the hg history of the file)

Comment 6

[Riley Baldin]

Thanks Sylvestre, I'll shoot you an email later today.  

Arnaud, I'm adding you in because you recently made changes in this file.  The author of this actual line was the creator of the file and I'm not sure how to find that (new to Mercurial).  I'm requesting a review of the above patch.

Comment 7

[Arnaud Sourioux [:Six]]

Hi,

Well "recently" isn't quite fair has I changed a few lines in this file more than one year ago :)
I'm not allowed to review your patch sorry ;)

You should look at the 'r=' part at the end of the commit message to find reviewers involved in this file
As this file is in the layout part, you might need to find a layout module peer reviewer or a XUL peer reviewer.
you can find module owners and peers here: https://wiki.mozilla.org/Modules/Core

Don't be afraid to post thoses kind of questions on IRC irc.mozilla.org in #developers

Thanks,

Comment 8

[Riley Baldin]

Fair enough.  "The only 2+ line change in a year" may have been a better statement.  Mike, I'm adding you in as you reviewed the last change to this file.  If you're also unable to review I'll ping the IRC chat.  I'm new to the workflow here.  Thanks all.

Comment 9

[Mike Hommey [:glandium]]

I merely reviewed file moves.

Comment 10

[Riley Baldin]

Nathan, is it possible for you to review this patch?

Comment 11

[Nathan Froyd [:froydnj]]

Comment on attachment 8624537 [details] [diff] [review]
removingDeclaration_diff.txt

I'm not a layout/XUL peer, but I feel pretty confident in saying that this patch is OK after reviewing the scan-build output.

For future reference, asking people to review patches is better done by setting the "review" flag on the patch directly:

https://bugzilla.mozilla.org/attachment.cgi?id=8624537&action=edit

on the above page, for instance, which can be reached by clicking the "details" link associated with your patch.  You can set the review flag to "?", which will open up a textbox where you can enter the reviewer; it will also provide you with a list of suggested reviewers, which tends to be fairly accurate.

I've set the review flag on this patch to "+" for you.  Thanks for the patch!

Comment 12

[Riley Baldin]

Attachment: Approved patch in hg format.

Requesting check-in.  I don't have try server access.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8625101 [details] [diff] [review]
Approved patch in hg format.

Thanks for the patch, Riley :). In the future, please use the checkin-needed bug keyword instead. It plays more nicely with our bug marking tools :)

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80005aa6f8e1

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/80005aa6f8e1