Closed Bug 287102 Opened 19 years ago Closed 19 years ago

fix PreFast warnings in layout/forms

Categories

(Core :: Layout: Form Controls, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bernd_mozilla, Assigned: bernd_mozilla)

References

Details

Attachments

(1 file)

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
Attached patch patchSplinter Review
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
Blocks: 283681
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: