185010 - nsGenericHTMLElement::GetAttr is messy and causes compiler warnings

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Bratell]

For instance is aResult truncated twice (just to be sure?).

Now it starts: 

nsresult
nsGenericHTMLElement::GetAttr(PRInt32 aNameSpaceID, nsIAtom *aAttribute,
                              nsIAtom*& aPrefix, nsAString& aResult) const
{
  nsresult rv;
  aResult.Truncate();   // First truncate

  aPrefix = nsnull;
  const nsHTMLValue* value;
  if (aNameSpaceID != kNameSpaceID_None) {
    rv = mAttributes ? mAttributes->GetAttribute(aAttribute, aNameSpaceID,
                                                 aPrefix, &value) :
                       NS_CONTENT_ATTR_NOT_THERE;
  } else {
    aNameSpaceID = kNameSpaceID_None;
    rv = mAttributes ? mAttributes->GetAttribute(aAttribute, &value) :
                       NS_CONTENT_ATTR_NOT_THERE;
  }

  aResult.Truncate(); // <-- Second truncate




it would be better if it started 


nsresult
nsGenericHTMLElement::GetAttr(PRInt32 aNameSpaceID, nsIAtom *aAttribute,
                              nsIAtom*& aPrefix, nsAString& aResult) const
{
  nsresult rv;
  aResult.Truncate();
  aPrefix = nsnull;

  if (!mAttributes)
    return NS_CONTENT_ATTR_NOT_THERE;

  const nsHTMLValue* value;
  if (aNameSpaceID == kNameSpaceID_None) {
    rv = mAttributes->GetAttribute(aAttribute, &value);
  } else {
    rv = mAttributes->GetAttribute(aAttribute, aNameSpaceID, aPrefix, &value);
  }

Comment 1

[Daniel Bratell]

The compiler warning was bogus but I can understand that the compiler had
difficulty seeing that. The warning was:
j:\moz\rmp\mozilla\content\html\content\src\nsgenerichtmlelement.cpp(2174) :
warning C4701: local variable 'value' may be used without having been initialized

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

Note that gcc 3.2 does not warn.

Comment 3

[Johnny Stenback (:jst)]

Mass-reassigning bugs.

Comment 4

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) <nsGenericHTMLElement.cpp>

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 137934 [details] [diff] [review]
(Av1) <nsGenericHTMLElement.cpp>

I have no compiler: Could you compile/test/review it ? Thanks.

Comment 6

[Serge Gautherie (:sgautherie)]

Reassigning from 'general#dom.bugs' to me :->

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Unless someone really wants us to land this i'd rather that we didn't. I'm
rewriting this code in bug 195350 which will take care of this problem and oh so
much more.

Comment 8

[Serge Gautherie (:sgautherie)]

I'd like to get rid of the warning as soon as possible;

but if your patch(s) can be cheched in in the near future,
I fully agree to stand by in the meantime.

Updating:
+(d.on) 195350

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 137934 [details] [diff] [review]
(Av1) <nsGenericHTMLElement.cpp>

Agreed.  Anyway, as already mention, the warning is bogus (caused by an old,
buggy gcc version).  New gcc versions do not warn here.  If you really want to
get rid of the warning, update your compiler.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

this was fixed by attachment 139913 [details] [diff] [review] in bug 195350

Comment 11

[Serge Gautherie (:sgautherie)]

Comment on attachment 137934 [details] [diff] [review]
(Av1) <nsGenericHTMLElement.cpp>

Obsoleting, per comment 10.