133711 - Unreachable statement (duplicate return) in nsXULAttribute and nsSVGAttributes (Bugs found via linting....)

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[John Carter]

This is a list of bugs found by antic (http://artho.com/jlint/)

The vast majority of false positives have been removed leaving only
definite bugs and highly suspicious cases.

======================================================================
Same one as in svg
The "return NS_OK" is unreachable, probably a bug.

~/builds/mozilla/content/xul/content/src/nsXULAttributes.cpp:600:9: Unreachable
statement

nsXULAttributes::RemoveNamedItem(const nsAReadableString& aName,
                                 nsIDOMNode** aReturn)
{
    nsCOMPtr<nsIDOMElement> element( do_QueryInterface(mContent) );
    if (element) {
        return element->RemoveAttribute(aName);
        *aReturn = nsnull; // XXX should be the element we just removed
        return NS_OK;
    }
    else {
        return NS_ERROR_FAILURE;
    }
}

Comment 1

[Christopher Hoess (gone)]

That would be "XP Toolkit/Widgets: XUL".

Comment 2

[John Morrison]

cvsblame sez waterson.

bbaetz, this was also copy-pasted into SVG: nsSVGAttributes.cpp, line 931.

Comment 3

[Bradley Baetz (:bbaetz)]

SVG case is bug 133710; feel free to fix both of these at the same time.

Comment 4

[Alex Fritze]

*** Bug 133710 has been marked as a duplicate of this bug. ***

Comment 5

[Alex Fritze]

Attachment: patch fixing ns{XUL,SVG}Attributes

Comment 6

[Chris Waterson]

How embarrassing. While we're at it, maybe we should remove the
else-after-return, too?

Comment 7

[Tim]

Attachment: The previous patch without the else-after-return

Comment 8

[timeless]

Jul 11 15:43 Out with the old, in with the new.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 92147 [details] [diff] [review]
The previous patch without the else-after-return

This leaves aReturn un-initialized.  Unacceptable (even if equivalent to the
exisiting code).

Comment 10

[Boris Zbarsky [:bzbarsky]]

This is not mine; please mail me when you need review instead of assigning bugs
to me.  To patch author, to fix the patch.  ;)

Comment 11

[Tim]

Attachment: Initialize out param, too

I also pre-emptively fixed the grammar of the comment, just to avoid another
round =)

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 92193 [details] [diff] [review]
Initialize out param, too

sr=bzbarsky; please file bugs on fixing those aReturn values if there aren't
such bugs already.

Comment 13

[Tim]

This has had R+SR for a while. Has it bitrotted yet? I don't have a tree handy
(or the capacity for a tree on this machine). I can check its status in a week
or so.  If it hasn't bitrotted (miraculously), is there something holding it
from being checked in?

Comment 14

[Boris Zbarsky [:bzbarsky]]

This has not bitrotted, and even builds.  I'll try to check it in early next
week, but if someone has time before then that would be great.

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

03/04/2003 18:35	timeless%mozdev.org 	mozilla/ content/ xul/ content/ src/
nsXULAttributes.cpp 	1.62 	3/5  	Bug 133711 Unreachable statement (duplicate
return) in nsXULAttribute and nsSVGAttributes (Bugs found via linting....)
patch by [e-mail] r=timeless sr=bz

therefore, marking FIXED.