Closed Bug 932904 Opened 8 years ago Closed 8 years ago

Use of uninitialized value on the heap: created by nsNntpIncomingServerConstructor()

Categories

(MailNews Core :: Networking: NNTP, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: ishikawa, Assigned: ishikawa)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch Proposed patch: uses PR_now(). (obsolete) — Splinter Review
Use of uninitialized value on the heap: created by nsNntpIncomingServerConstructor()


Detected by memcheck/valgrind.

I see the following uninitialized value access errors during the |make
mozmill| test suite run of comm-central thunderbird (source refreshed
about 48 hours ago, and this is a DEBUG BUILD) under
valgrind/memcheck.  (I use a wrapper program to run thunderbird under
valgrind).

This did not happen when similar memcheck run on TB during |make
mozmill| test was performed about a couple of weeks ago.  So I think
the regression is fairly recent (in October?).
Or maybe we have a new test?

Noticed on 64-bit Debian GNU/Linux.

The following is the error mesasge: 

There are three error messages that appeared consecutively in the log.
I think they all point to the same issue. So I list the first error
only.

==13469== Conditional jump or move depends on uninitialised value(s)
==13469==    at 0x406BF06: dosprintf (prprf.c:867)
==13469==    by 0x406C643: PR_vsxprintf (prprf.c:1073)
==13469==    by 0x925CF50: nsACString_internal::AppendPrintf(char const*, ...) (nsTSubstring.cpp:780)
==13469==    by 0x89201DB: nsNntpIncomingServer::WriteHostInfoFile() [clone .part.33] (nsTSubstring.h:417)
==13469==    by 0x8922062: nsNntpIncomingServer::StopPopulating(nsIMsgWindow*) (nsNntpIncomingServer.cpp:795)
==13469==    by 0x891D6C1: nsNntpIncomingServer::OnStopRunningUrl(nsIURI*, tag_nsresult) (nsNntpIncomingServer.cpp:722)
==13469==    by 0x854F6AB: nsMsgMailNewsUrl::SetUrlState(bool, tag_nsresult) (nsMsgMailNewsUrl.cpp:97)
==13469==    by 0x8940464: nsNNTPProtocol::CleanupAfterRunningUrl() (nsNNTPProtocol.cpp:4735)
==13469==    by 0x89465EF: nsNNTPProtocol::ProcessProtocolState(nsIURI*, nsIInputStream*, unsigned long, unsigned int) (nsNNTPProtocol.cpp:4652)
==13469==    by 0x85554C3: nsMsgProtocol::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long, unsigned int) (nsMsgProtocol.cpp:353)
==13469==    by 0x6DF00F0: nsInputStreamPump::OnStateTransfer() (nsInputStreamPump.cpp:592)
==13469==    by 0x6DF06D7: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:433)
==13469==    by 0x9206200: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:85)
==13469==    by 0x9223FF1: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:622)
==13469==    by 0x924CA6D: NS_InvokeByIndex (xptcinvoke_x86_64_unix.cpp:164)
==13469==    by 0x8075AF4: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (XPCWrappedNative.cpp:2797)
==13469==    by 0x8082219: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (XPCWrappedNativeJSOps.cpp:1311)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BD95BC: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2499)
==13469==    by 0x9BE0308: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:419)
==13469==    by 0x9BE077A: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:481)
==13469==    by 0x9BE0FA9: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:512)
==13469==    by 0x9B24B6A: js::DirectProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:454)
==13469==    by 0x9B83919: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jswrapper.cpp:454)
==13469==    by 0x9B257D6: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:2643)
==13469==    by 0x9B258F7: proxy_Call(JSContext*, unsigned int, JS::Value*) (jsproxy.cpp:3037)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BD95BC: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2499)
==13469==    by 0x9BE0308: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:419)
==13469==    by 0x9BE077A: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:481)
==13469==    by 0x9BE0FA9: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:512)
==13469==    by 0x9B24B6A: js::DirectProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:454)
==13469==    by 0x9B83919: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jswrapper.cpp:454)
==13469==    by 0x9B257D6: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:2643)
==13469==    by 0x9B258F7: proxy_Call(JSContext*, unsigned int, JS::Value*) (jsproxy.cpp:3037)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BD95BC: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2499)
==13469==    by 0x9BE0308: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:419)
==13469==    by 0x9BE077A: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:481)
==13469==    by 0x9BE0FA9: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:512)
==13469==    by 0x9B24B6A: js::DirectProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:454)
==13469==    by 0x9B83919: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jswrapper.cpp:454)
==13469==    by 0x9B257D6: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:2643)
==13469==    by 0x9B258F7: proxy_Call(JSContext*, unsigned int, JS::Value*) (jsproxy.cpp:3037)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BD95BC: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2499)
==13469==  Uninitialised value was created by a heap allocation
==13469==    at 0x402A914: malloc (vg_replace_malloc.c:291)
==13469==    by 0x4046093: moz_xmalloc (mozalloc.cpp:54)
==13469==    by 0x8502BD3: nsNntpIncomingServerConstructor(nsISupports*, nsID const&, void**) (mozalloc.h:201)
==13469==    by 0x921AAC6: nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) (nsComponentManager.cpp:1096)
==13469==    by 0x91ABE9F: nsCreateInstanceByContractID::operator()(nsID const&, void**) const (nsComponentManagerUtils.cpp:178)
==13469==    by 0x852B62C: nsCOMPtr<nsIMsgIncomingServer>::assign_from_helper(nsCOMPtr_helper const&, nsID const&) (nsCOMPtr.h:1259)
==13469==    by 0x859031C: nsMsgAccountManager::createKeyedServer(nsACString_internal const&, nsACString_internal const&, nsACString_internal const&, nsACString_internal const&, nsIMsgIncomingServer**) (nsCOMPtr.h:637)
==13469==    by 0x85909CC: nsMsgAccountManager::CreateIncomingServer(nsACString_internal const&, nsACString_internal const&, nsACString_internal const&, nsIMsgIncomingServer**) (nsMsgAccountManager.cpp:458)
==13469==    by 0x924CA6D: NS_InvokeByIndex (xptcinvoke_x86_64_unix.cpp:164)
==13469==    by 0x8075AF4: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (XPCWrappedNative.cpp:2797)
==13469==    by 0x8082219: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (XPCWrappedNativeJSOps.cpp:1311)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BD95BC: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2499)
==13469==    by 0x9BE0308: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:419)
==13469==    by 0x9BE077A: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:481)
==13469==    by 0x9BE0FA9: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:512)
==13469==    by 0x9B24B6A: js::DirectProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:454)
==13469==    by 0x9B83919: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jswrapper.cpp:454)
==13469==    by 0x9B257D6: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:2643)
==13469==    by 0x9B258F7: proxy_Call(JSContext*, unsigned int, JS::Value*) (jsproxy.cpp:3037)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BD95BC: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2499)
==13469==    by 0x9BE0308: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:419)
==13469==    by 0x9BE077A: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:481)
==13469==    by 0x9BE0FA9: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:512)
==13469==    by 0x9B24B6A: js::DirectProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:454)
==13469==    by 0x9B83919: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jswrapper.cpp:454)
==13469==    by 0x9B257D6: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:2643)
==13469==    by 0x9B258F7: proxy_Call(JSContext*, unsigned int, JS::Value*) (jsproxy.cpp:3037)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BD95BC: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2499)
==13469==    by 0x9BE0308: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:419)
==13469==    by 0x9BE077A: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:481)
==13469==    by 0x9A91BD6: js_fun_apply(JSContext*, unsigned int, JS::Value*) (jsfun.cpp:1030)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BE0FA9: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:512)
==13469==    by 0x9B24B6A: js::DirectProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:454)
==13469==    by 0x9B83919: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jswrapper.cpp:454)
==13469==    by 0x9B257D6: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (jsproxy.cpp:2643)
==13469==    by 0x9B258F7: proxy_Call(JSContext*, unsigned int, JS::Value*) (jsproxy.cpp:3037)
==13469==    by 0x9BE32AA: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:220)
==13469==    by 0x9BE052D: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:462)
==13469==    by 0x9BD95BC: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2499)
==13469==    by 0x9BE0308: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:419)
==13469==    by 0x9BE077A: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:481)
==13469==    by 0x9BE0FA9: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:512)
==13469==

On paper, I track down the cause to the failure to initialize
|mFirstNewDate|, a member of |nsNntpIncomingServer| class.

For a new server, for which existing host inforamtion file is NOT
read, this value is not initialized at all.  Just search
|mFirstNewDate| in comm-central using MXR.  You will see what I mean.

I think it ought to be initialized to some value.

Question is, what value should it be?

I suspect that 

  uint32_t(PR_Now() / PR_USEC_PER_SEC);

should be it, but I am not familiar with the code.
Another choice would be 0: this means POSIX epoch (1970 Jan 1).

Since TB currently writes out uninitialized bogus value (that can be a
future date!) to the external file and seems to use that value
forever, I think whichever value is better than the current
undeterministic behavior: I prefer PR_now variant.

TIA
I have no idea who the reviewer should be since the code in question has not been touched for quite some time. 

Also, I noticed that bug entry for product category, thunderbird, does not show "News" as component. I think this is wrong. "News" should be a valid entry (?)

TIA
Comment on attachment 824770 [details] [diff] [review]
Proposed patch: uses PR_now().

Back in 809060, it was Mark Banner who reviewed the patch, and so I am setting the reviewer to him.

Still a mystery why this is caught now. Maybe a new test.
Attachment #824770 - Flags: review?(mbanner)
Comment on attachment 824770 [details] [diff] [review]
Proposed patch: uses PR_now().

So unless I'm mistaken, mFirstNewDate isn't actually used except when writing the file and is only filled in when reading the file. So really, it seems like it is redundant.

Asking Joshua for a second opinion...
Attachment #824770 - Flags: review?(mbanner) → review?(Pidgeot18)
(In reply to Mark Banner (:standard8) from comment #3)
> Comment on attachment 824770 [details] [diff] [review]
> Proposed patch: uses PR_now().
> 
> So unless I'm mistaken, mFirstNewDate isn't actually used except when
> writing the file and is only filled in when reading the file. So really, it
> seems like it is redundant.

It looks like so unless JavaScript somehow access this and modifies it with XPCOM magic.



> Asking Joshua for a second opinion...

Appreciated.


> On paper, I track down the cause to the failure to initialize
> |mFirstNewDate|, a member of |nsNntpIncomingServer| class.

With the patch posted here, I confirmed by |make mozill| run under valgrind/memcheck, that 
I can eliminate the error of using uninitialized value.
So if JavaScript or some external entity tries to use the file and access the data,
something like the posted patch should help.

But now this is gone, another bug stands out:
Bug 932851 - Use of uninitialized value created on stack: : js::Discard() (StructuredClone.cpp:351) (edit) 


TIA
Assignee: nobody → ishikawa
(In reply to ISHIKAWA, Chiaki from comment #1)
> I have no idea who the reviewer should be since the code in question has not
> been touched for quite some time. 
> 
> Also, I noticed that bug entry for product category, thunderbird, does not
> show "News" as component. I think this is wrong. "News" should be a valid
> entry (?)
> 

I think the Product should be "MailNews Core" and the Component "Networking: NNTP".
(In reply to Jens Müller (:tessarakt) from comment #5)
> (In reply to ISHIKAWA, Chiaki from comment #1)
> > I have no idea who the reviewer should be since the code in question has not
> > been touched for quite some time. 
> > 
> > Also, I noticed that bug entry for product category, thunderbird, does not
> > show "News" as component. I think this is wrong. "News" should be a valid
> > entry (?)
> > 
> 
> I think the Product should be "MailNews Core" and the Component "Networking:
> NNTP".

I tried to set the Product to MailNews Core, but then
the component of "Networking: NNTP is now shown".
I am afraid that the combination is not allowed by bugzilla?
Hmm...
Component: Untriaged → Networking: NNTP
Product: Thunderbird → MailNews Core
I see. Once I change the Product to MailNews Core and tried to save the change,
a new selection for component, version, etc. appeared!
I wish the new component (MailNews Core) 
would have been shown when I changed the entry to MailNews Core even before saving it.

Thanks for the product/component suggestion.
Comment on attachment 824770 [details] [diff] [review]
Proposed patch: uses PR_now().

Review of attachment 824770 [details] [diff] [review]:
-----------------------------------------------------------------

mFirstNewDate is otherwise unused, so it's better to kill it altogether instead of initializing it to a bad value. I believe this is meant to interoperate with NS 4.x hostinfo.dat files... but it's safe to say that no one cares about that interoperability anymore. :-)
Attachment #824770 - Flags: review?(Pidgeot18) → review-
(In reply to Joshua Cranmer [:jcranmer] from comment #8)
> Comment on attachment 824770 [details] [diff] [review]
> Proposed patch: uses PR_now().
> 
> Review of attachment 824770 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> mFirstNewDate is otherwise unused, so it's better to kill it altogether
> instead of initializing it to a bad value. I believe this is meant to
> interoperate with NS 4.x hostinfo.dat files... but it's safe to say that no
> one cares about that interoperability anymore. :-)

Thank you. I will create a patch that removes it completely.
(I am still not sure why this showed up in my valgrind/memcheck lately. Maybe a new test triggered this code path???. But I digress.)

TIA
Attached patch A patch to eliminate mFirstDate. (obsolete) — Splinter Review
This is a patch to eliminate the use of mFirstDate.
I have not completely eradicate them, but instead turned the usage into comment.
This is because there may be a chance that some developers notice the no-longer used "firstnewdate" line in existing external files (that are not used recently) and 
they may want to find out what is behind for that particular entry.
At least, by leaving the comment lines, the developers would be able to
find out "firstnewdate".

TIA
Attachment #824770 - Attachment is obsolete: true
Attachment #831404 - Flags: review?(Pidgeot18)
Ping?
Flags: needinfo?(Pidgeot18)
It helps when asking review to ask from the right email address. :-)
Flags: needinfo?(Pidgeot18)
Comment on attachment 831404 [details] [diff] [review]
A patch to eliminate mFirstDate.

Review of attachment 831404 [details] [diff] [review]:
-----------------------------------------------------------------

Don't comment it out, delete it entirely.
Attachment #831404 - Flags: review?(Pidgeot18) → review-
(In reply to Joshua Cranmer [:jcranmer] from comment #12)
> It helps when asking review to ask from the right email address. :-)

Sorry, it still puzzles me sometimes to figure out what is one's correct e-mail address, 
and what is nickname (used in bugzilla).

(In reply to Joshua Cranmer [:jcranmer] from comment #13)
> Comment on attachment 831404 [details] [diff] [review]
> A patch to eliminate mFirstDate.
> 
> Review of attachment 831404 [details] [diff] [review]:
> -----------------------------------------------------------------

> 
> Don't comment it out, delete it entirely.

I am uploading a new patch here.

TIA
Attachment #831404 - Attachment is obsolete: true
Attachment #8358940 - Flags: review?(Pidgeot18)
Comment on attachment 8358940 [details] [diff] [review]
uninitialized-mFirstNewDate-Rev02.patch

Review of attachment 8358940 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/news/src/nsNntpIncomingServer.h
@@ +125,2 @@
>      uint32_t mLastGroupDate;
> +

Nit: delete this blank line.
Attachment #8358940 - Flags: review?(Pidgeot18) → review+
(In reply to Joshua Cranmer [:jcranmer] from comment #15)
> Comment on attachment 8358940 [details] [diff] [review]
> uninitialized-mFirstNewDate-Rev02.patch
> 
> Review of attachment 8358940 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mailnews/news/src/nsNntpIncomingServer.h
> @@ +125,2 @@
> >      uint32_t mLastGroupDate;
> > +
> 
> Nit: delete this blank line.

Thank you for the review.
I am uploading the final patch with the above whitespace line removed.

I will put checkin-needed keyword.
Attachment #8358940 - Attachment is obsolete: true
Attachment #8362184 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/59023e39e8a0
Status: NEW → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in before you can comment on or make changes to this bug.