If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

redundant GetMessageKey() call

RESOLVED FIXED in Thunderbird 3.3a2

Status

MailNews Core
Database
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Boying Lu, Assigned: Boying Lu)

Tracking

Trunk
Thunderbird 3.3a2
x86
Solaris

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
The GetMessageKey() call is redundant and should be removed.
(Assignee)

Comment 1

7 years ago
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.
(Assignee)

Comment 2

7 years ago
Created attachment 489124 [details] [diff] [review]
patch
Attachment #489124 - Flags: review?
(Assignee)

Updated

7 years ago
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)

Comment 4

7 years ago
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.
(Assignee)

Comment 5

7 years ago
Created attachment 489418 [details] [diff] [review]
move the decl of newRoot before its first reference
Attachment #489124 - Attachment is obsolete: true
Attachment #489418 - Flags: review?(bienvenu)
Attachment #489124 - Flags: review?(bienvenu)

Comment 6

7 years ago
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+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/9a268939f29c
Status: NEW → RESOLVED
Last Resolved: 7 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.