Closed Bug 185010 Opened 17 years ago Closed 16 years ago

nsGenericHTMLElement::GetAttr is messy and causes compiler warnings

Categories

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

defect
Not set
trivial

Tracking

()

RESOLVED FIXED

People

(Reporter: bratell, Assigned: sgautherie)

References

(Blocks 1 open bug)

Details

Attachments

(1 obsolete file)

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);
  }
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
Note that gcc 3.2 does not warn.
Blocks: buildwarning
Mass-reassigning bugs.
Assignee: jst → dom_bugs
Whiteboard: [good first bug]
Attached patch (Av1) <nsGenericHTMLElement.cpp> (obsolete) — Splinter Review
Comment on attachment 137934 [details] [diff] [review]
(Av1) <nsGenericHTMLElement.cpp>

I have no compiler: Could you compile/test/review it ? Thanks.
Attachment #137934 - Flags: review?(caillon)
Reassigning from 'general#dom.bugs' to me :->
Assignee: general → gautheri
Severity: normal → trivial
OS: Windows 2000 → All
Hardware: PC → All
Whiteboard: [good first bug]
Target Milestone: --- → mozilla1.7alpha
Status: NEW → ASSIGNED
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.
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
Depends on: attrs
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.
Attachment #137934 - Flags: review?(caillon)
this was fixed by attachment 139913 [details] [diff] [review] in bug 195350
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment on attachment 137934 [details] [diff] [review]
(Av1) <nsGenericHTMLElement.cpp>

Obsoleting, per comment 10.
Attachment #137934 - Attachment description: (Av1) → (Av1) <nsGenericHTMLElement.cpp>
Attachment #137934 - Attachment is obsolete: true
Target Milestone: mozilla1.7alpha → ---
Component: DOM: HTML → DOM: Core & HTML
QA Contact: stummala → general
You need to log in before you can comment on or make changes to this bug.