Closed
Bug 818657
Opened 12 years ago
Closed 12 years ago
Fix build failures due to prmem include removals
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 20.0
People
(Reporter: mcsmurf, Assigned: aceman)
References
Details
Attachments
(1 file, 1 obsolete file)
13.58 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
Bug 801466 removed a few prmem includes from files in mozilla-central. Some files in mailnews/ relied on this and so this now broke. Example build failures: e:\builds\moz2_slave\tb-c-cen-w32-dbg\build\config\rules.mk:1113:0$ e:/builds/moz2_slave/tb-c-cen-w32-dbg/build/objdir-tb/_virtualenv/Scripts/python.exe -O e:/builds/moz2_slave/tb-c-cen-w32-dbg/build/mozilla/build/cl.py cl -FonsMsgUtils.obj -c -D_HAS_EXCEPTIONS=0 -I../../../mozilla/dist/stl_wrappers -D_IMPL_NS_MSG_BASE -DZLIB_DLL -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES -DZLIB_INTERNAL -DMOZ_THUNDERBIRD=1 -DOSTYPE=\"WINNT6.1\" -DOSARCH=WINNT -Ie:/builds/moz2_slave/tb-c-cen-w32-dbg/build/mailnews/base/util -I. -I../../../mozilla/dist/include -I../../../mozilla/dist/include/nsprpub -Ie:/builds/moz2_slave/tb-c-cen-w32-dbg/build/objdir-tb/mozilla/dist/include/nspr -Ie:/builds/moz2_slave/tb-c-cen-w32-dbg/build/objdir-tb/mozilla/dist/include/nss -TP -nologo -W3 -Gy -Fdgenerated.pdb -wd4345 -wd4351 -wd4800 -we4553 -GR- -DDEBUG -D_DEBUG -DTRACING -Zi -O1 -Oy- -MDd -FI ../../../comm-config.h -DMOZILLA_CLIENT e:/builds/moz2_slave/tb-c-cen-w32-dbg/build/mailnews/base/util/nsMsgUtils.cpp nsMsgDBFolder.cpp e:/builds/moz2_slave/tb-c-cen-w32-dbg/build/mailnews/base/util/nsMsgDBFolder.cpp(1733) : warning C4244: 'argument' : conversion from 'int64_t' to 'uint32_t', possible loss of data e:/builds/moz2_slave/tb-c-cen-w32-dbg/build/mailnews/base/util/nsMsgDBFolder.cpp(2762) : error C3861: 'PR_MALLOC': identifier not found e:/builds/moz2_slave/tb-c-cen-w32-dbg/build/mailnews/base/util/nsMsgDBFolder.cpp(2779) : error C3861: 'PR_Free': identifier not found [...]
Attachment #688955 -
Flags: review?(mbanner)
Status: NEW → ASSIGNED
OS: Windows 7 → All
Hardware: x86_64 → All
Comment 2•12 years ago
|
||
Comment on attachment 688955 [details] [diff] [review] patch Review of attachment 688955 [details] [diff] [review]: ----------------------------------------------------------------- I'm not quite sure why we need to dynamically allocate lineBuffer, but for now, we'll keep it as it is. Just some changes need applying in most of the cases. ::: mailnews/base/util/nsMsgDBFolder.cpp @@ +5429,5 @@ > 4. multipart/mixed - scan past boundary, treat next part as body. > */ > > + nsAutoPtr<nsLineBuffer<char> > lineBuffer; > + lineBuffer = new nsLineBuffer<char>; nit: We can make this all on one line. We should also be checking for if (!lineBuffer) return NS_ERROR_OUT_OF_MEMORY;
Attachment #688955 -
Flags: review?(mbanner) → review-
OK, done.
Attachment #688955 -
Attachment is obsolete: true
Attachment #688969 -
Flags: review?(mbanner)
Comment 4•12 years ago
|
||
Comment on attachment 688969 [details] [diff] [review] patch v2 Review of attachment 688969 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks.
Attachment #688969 -
Flags: review?(mbanner) → review+
Comment 5•12 years ago
|
||
Checked in: https://hg.mozilla.org/comm-central/rev/f6aa8845e67b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 20.0
Comment 6•12 years ago
|
||
nsLineBuffer<char> is 4108/4120 bytes, so possibly unsuitable for stack allocation?
Comment 7•12 years ago
|
||
> We should also be checking for
>
> if (!lineBuffer)
> return NS_ERROR_OUT_OF_MEMORY;
why? isn't new infalible in com-central?
Comment 8•12 years ago
|
||
Comment on attachment 688969 [details] [diff] [review] patch v2 >@@ -5582,17 +5584,17 @@ NS_IMETHODIMP nsMsgDBFolder::GetMsgTextF > msgText.Append(NS_LITERAL_CSTRING("\r\n")); > } > > bytesRead += curLine.Length(); > if (bytesRead > bytesToRead) > break; > } > } >- PR_Free(lineBuffer); >+ lineBuffer = nullptr; is there some reason the memory needs to be freed then as opposed to when the variable goes out of scope? (same comment applies a bunch of places below)
Comment 9•12 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #6) > nsLineBuffer<char> is 4108/4120 bytes, so possibly unsuitable for stack > allocation? That would probably be it. (In reply to Trevor Saunders (:tbsaunde) from comment #7) > > We should also be checking for > > > > if (!lineBuffer) > > return NS_ERROR_OUT_OF_MEMORY; > > why? isn't new infalible in com-central? Well, for one, last I knew, new isn't infallible - I know it was being worked on, but I cannot find any announcements on mozilla.dev.planning about it, nor have I seen a change in status mentioned anywhere else (and I was expecting this to have been shouted about quite a bit). I know strings are infallible, but not allocators.
You need to log in
before you can comment on or make changes to this bug.
Description
•