Closed Bug 610611 Opened 11 years ago Closed 11 years ago

redundant GetMessageKey() call

Categories

(MailNews Core :: Database, defect)

x86
Solaris
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a2

People

(Reporter: eagle.lu, Assigned: eagle.lu)

Details

Attachments

(1 file, 1 obsolete file)

The GetMessageKey() call is redundant and should be removed.
Sorry, for clicking submit button too quickly.
I mean the call at http://mxr.mozilla.org/comm-central/source/mailnews/db/msgdb/src/nsMsgThread.cpp#209 is redundant.
Attached patch patch (obsolete) — Splinter Review
Attachment #489124 - Flags: review?
Attachment #489124 - Flags: review? → review?(bugzilla)
Component: General → Database
Product: Thunderbird → MailNews Core
QA Contact: general → database
Comment on attachment 489124 [details] [diff] [review]
patch

>   nsCOMPtr <nsIMsgDBHdr> ancestorHdr = newParentOfOldRoot;
>   nsMsgKey newRoot;
>-  newParentOfOldRoot->GetMessageKey(&newRoot);
>   mdb_pos outPos;
> 
>   nsMsgKey newHdrAncestor;
>   ancestorHdr->GetMessageKey(&newRoot);

This seems wrong, looking further below newHdrAncestor is not initialised and used uninitialised.

However, these newRoot and newHdrAncestor would then end up the same. That might be right, but then we could just assign rather than getting the message key twice.

Although it also could be that just getting the newHdrAncestor is redundant as you've got the patch. In which case I would tend to reorder the variables a bit.

So I'm not quite sure which shape this function is meant to be in. So I'm going to pass the review to David and let him have a look.
Attachment #489124 - Flags: review?(bugzilla) → review?(bienvenu)
newHdrAncestor is initialized here:
    ancestorHdr->GetThreadParent(&newHdrAncestor);

ThreadParent is not the same as MessageKey, so I don't think newHdrAncestor and newRoot are the same.

So this patch looks OK, other than perhaps moving the decl of newRoot to just before it's used.
Attachment #489124 - Attachment is obsolete: true
Attachment #489418 - Flags: review?(bienvenu)
Attachment #489124 - Flags: review?(bienvenu)
Comment on attachment 489418 [details] [diff] [review]
move the decl of newRoot before its first reference

thx for the patch.
Attachment #489418 - Flags: review?(bienvenu) → review+
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/9a268939f29c
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a1
Target Milestone: Thunderbird 3.3a1 → Thunderbird 3.3a2
You need to log in before you can comment on or make changes to this bug.