Conditional jump or move depends on uninitialised value at nsNNTPProtocol::ParseURL()

RESOLVED FIXED in Thunderbird 3.0rc1

Status

MailNews Core
Networking: NNTP
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Martin Stránský, Assigned: Jan Horak)

Tracking

({fixed-seamonkey2.0.1})

Trunk
Thunderbird 3.0rc1
fixed-seamonkey2.0.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
m_newsAction variable is not set in constructor so it can be used uninitialized:

Valgrind output:

==22747== Conditional jump or move depends on uninitialised value(s)
==22747==    at 0x5812916: nsNNTPProtocol::ParseURL(nsIURI*, char**, char**, char**) (nsNNTPProtocol.cpp:1385)
==22747==    by 0x5814605: nsNNTPProtocol::Initialize(nsIURI*, nsIMsgWindow*) (nsNNTPProtocol.cpp:456)
==22747==    by 0x582712A: nsNntpIncomingServer::GetNntpConnection(nsIURI*, nsIMsgWindow*, nsINNTPProtocol**) (nsNntpIncomingServer.cpp:583)
==22747==    by 0x5827190: nsNntpIncomingServer::LoadNewsUrl(nsIURI*, nsIMsgWindow*, nsISupports*) (nsNntpIncomingServer.cpp:614)
==22747==    by 0x581AD75: nsNntpService::RunNewsUrl(nsIURI*, nsIMsgWindow*, nsISupports*) (nsNntpService.cpp:1267)
==22747==    by 0x581B381: nsNntpService::GetNewNews(nsINntpIncomingServer*, char const*, int, nsIUrlListener*, nsIMsgWindow*, nsIURI**) (nsNntpService.cpp:1298)
==22747==    by 0x581DBF3: nsMsgNewsFolder::GetNewsMessages(nsIMsgWindow*, int, nsIUrlListener*) (nsNewsFolder.cpp:895)
==22747==    by 0x581DCE3: nsMsgNewsFolder::GetNewMessages(nsIMsgWindow*, nsIUrlListener*) (nsNewsFolder.cpp:866)
==22747==    by 0x5821C4F: nsMsgNewsFolder::UpdateFolder(nsIMsgWindow*) (nsNewsFolder.cpp:350)
==22747==    by 0x41D285E: NS_InvokeByIndex_P (in /usr/lib/thunderbird-3.0b3pre/libxpcom_core.so)
==22747==    by 0x5D20DB9: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (xpcwrappednative.cpp:2450)
==22747==    by 0x5D28DFB: XPC_WN_CallMethod(JSContext*, JSObject*, unsigned, int*, int*) (xpcwrappednativejsops.cpp:1583)
Component: General → Networking: News
Product: Thunderbird → MailNews Core
QA Contact: general → networking.news
(Reporter)

Comment 1

8 years ago
Created attachment 379554 [details] [diff] [review]
patch

Initializes m_newsAction in nsNNTPProtocol::nsNNTPProtocol()
(Reporter)

Comment 2

8 years ago
Comment on attachment 379554 [details] [diff] [review]
patch

Can you check it please?
Attachment #379554 - Flags: review?(dmose)
Assigning to Stransky; since he was kind enough to provide a patch.
Assignee: nobody → stransky

Updated

8 years ago
Attachment #379554 - Flags: review?(dmose) → review?(Pidgeot18)
Comment on attachment 379554 [details] [diff] [review]
patch

Switching reviewer to jcranmer, as he's the News module owner...
(In reply to comment #4)
> (From update of attachment 379554 [details] [diff] [review])
> Switching reviewer to jcranmer, as he's the News module owner...

Dan where can this bit of information be found ?
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 379554 [details] [diff] [review] [details])
> > Switching reviewer to jcranmer, as he's the News module owner...
> 
> Dan where can this bit of information be found ?

I've just added a link set here: https://developer.mozilla.org/en/comm-central#Requirements - mailnews specific rules apply to this patch.
Attachment #379554 - Flags: review?(Pidgeot18) → review-
Comment on attachment 379554 [details] [diff] [review]
patch

I don't think this is the right way to go--protocol objects can be reused, so if the value is not getting initialized, it will have the wrong value on the next URL being run.

A better option would be to move the initialization of m_newsAction (the proper one, m_runningUrl->GetNewsAction) before the call to ParseURL in Initialize.
(Reporter)

Updated

8 years ago
Assignee: stransky → jhorak
(Assignee)

Comment 8

8 years ago
Created attachment 404820 [details] [diff] [review]
Patch v2: moved to Initialize method

Moving m_newsAction initialization into Initialize method. Please could you look into it?
Attachment #404820 - Flags: review?
(Assignee)

Updated

8 years ago
Attachment #404820 - Flags: review? → review?(Pidgeot18)
Attachment #404820 - Flags: review?(Pidgeot18) → review+
Comment on attachment 404820 [details] [diff] [review]
Patch v2: moved to Initialize method


> https://developer.mozilla.org/en/comm-central#Requirements - mailnews specific
> rules apply to this patch.

sr is required - so requesting it.
Attachment #404820 - Flags: superreview?(bugzilla)
Comment on attachment 404820 [details] [diff] [review]
Patch v2: moved to Initialize method

Sorry for the delay.

In future it is useful if you could include -p in the diff options when creating the patch to submit - it gives an idea of which functions code is in, which makes it easier to review the patch.

sr=Standard8. Also a=Standard8 for checkin to the code base during the TB 3 lockdown period.

I'm assuming you don't have checkin privs, so I'll do this for you in a while.
Attachment #404820 - Flags: superreview?(bugzilla)
Attachment #404820 - Flags: superreview+
Attachment #404820 - Flags: approval-thunderbird3+
Attachment #379554 - Attachment is obsolete: true
Checked in: http://hg.mozilla.org/comm-central/rev/3edac5d7d939
Status: NEW → RESOLVED
Last Resolved: 8 years ago
OS: Linux → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0rc1

Updated

8 years ago
Keywords: fixed-seamonkey2.0.1
You need to log in before you can comment on or make changes to this bug.