Closed
Bug 287102
Opened 19 years ago
Closed 19 years ago
fix PreFast warnings in layout/forms
Categories
(Core :: Layout: Form Controls, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bernd_mozilla, Assigned: bernd_mozilla)
References
Details
Attachments
(1 file)
4.99 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
e:\moz_src\mozilla\layout\forms\nscomboboxcontrolframe.cpp (2119): warning 246: Local declaration of 'content' hides declaration of the same name in an outer scope: see previous declaration at line 2092 of e:\moz_src\mozilla\layout\forms\nscomboboxcontrolframe.cpp. FUNCTION: nsComboboxControlFrame::CreateFrameFor (2077) PATH: 2092 e:\moz_src\mozilla\layout\forms\nsformcontrolframe.cpp (611): warning 11: Dereferencing NULL pointer 'aFrame'. FUNCTION: nsFormControlFrame::RegUnRegAccessKey (566) PATH: 568 568 568 569 569 569 571 572 574 608 609 610 611 e:\moz_src\mozilla\layout\forms\nsformcontrolframe.cpp (613): warning 11: Dereferencing NULL pointer 'aFrame'. FUNCTION: nsFormControlFrame::RegUnRegAccessKey (566) PATH: 568 568 568 569 569 569 571 572 574 608 609 610 613 e:\moz_src\mozilla\layout\forms\nstextcontrolframe.cpp (2792): warning 246: Local declaration of 'rv' hides declaration of the same name in an outer scope: see previous declaration at line 2787 of e:\moz_src\mozilla\layout\forms\nstextcontrolframe.cpp. FUNCTION: nsTextControlFrame::AttributeChanged (2781) PATH: 2787
Assignee: nobody → bernd_mozilla
Status: NEW → ASSIGNED
Attachment #178143 -
Flags: superreview?(dbaron)
Attachment #178143 -
Flags: review?(dbaron)
Comment on attachment 178143 [details] [diff] [review] patch Two of these three (and a bunch in the other bugs you've filed, I think) look like warnings gcc would have shown us if we hadn't turned off -Wshadow by default (which I objected to, and I still use). >Index: nsComboboxControlFrame.cpp >- nsCOMPtr<nsIContent> content(do_QueryInterface(mDisplayContent)); >- mTextFrame->Init(aPresContext, content, mDisplayFrame, textStyleContext, nsnull); >+ nsCOMPtr<nsIContent> textContent(do_QueryInterface(mDisplayContent)); >+ mTextFrame->Init(aPresContext, textContent, mDisplayFrame, textStyleContext, nsnull); The other |content| variable is the same thing, so instead of renaming, just remove the first of the two lines you're changing. >Index: nsFormControlFrame.cpp Make the null-check of aFrame an NS_ENSURE_TRUE, at the very least, or just (preferably) remove it entirely (we know it's never null, since it would have crashed). >+ nsIContent* content = aFrame->GetContent(); >+ nsAutoString resultValue; >+ rv = content->GetAttr(kNameSpaceID_None, >+ nsHTMLAtoms::accesskey, accessKey); > if (!accessKey.IsEmpty()) { > nsIEventStateManager *stateManager = aPresContext->EventStateManager(); > if (aDoReg) { > return stateManager->RegisterAccessKey(aFrame->GetContent(), (PRUint32)accessKey.First()); > } else { > return stateManager->UnregisterAccessKey(aFrame->GetContent(), (PRUint32)accessKey.First()); Either fix these last two to use |content| as well or just use |aFrame->GetContent()| in your change (it's really only a matter of style, but better to be consistent). >Index: nsTextControlFrame.cpp >- nsresult rv = GetMaxLength(&maxLength); >+ rv = GetMaxLength(&maxLength); This is actually a change to the function. Could you rename to rv2 instead, or assign rv = NS_OK below. We probably don't want to propagate the NS_CONTENT_ATTR_* values too far. r+sr=dbaron with those changes
Attachment #178143 -
Flags: superreview?(dbaron)
Attachment #178143 -
Flags: superreview+
Attachment #178143 -
Flags: review?(dbaron)
Attachment #178143 -
Flags: review+
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•