210541 - Bookmarks now crash if nsIProfile impl is not present.

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Conrad Carlen (not reading bugmail)]

Notice that here:
http://lxr.mozilla.org/mozilla/source/xpfe/components/bookmarks/src/nsBookmarksService.cpp#1786,
there's no null-check. If the profile mgr is not present (and it's not in many
embedding apps) this will crash.

Comment 1

[Alec Flett]

is there a patch coming? I'll review :)

Comment 2

[Conrad Carlen (not reading bugmail)]

Attachment: patch

The patch is -w, since I had to indent a sizable block.

Comment 3

[Conrad Carlen (not reading bugmail)]

Comment on attachment 126389 [details] [diff] [review]
patch

Eek! the 1.4 branch has this problem. Requesting a=.

Comment 4

[Alec Flett]

Comment on attachment 126389 [details] [diff] [review]
patch

I'm sure you've already confirmed this just by the nature of this patch, but I
just want to double-check that
1) rv isn't uninitialized  now, or has some bogus value after the first hunk of
the patch
2) that the other people who use useProfile aren't getting it in some wierd
success or error condition now

r/sr=alecf assuming that's all cool - this is a null (success) check, mostly
low risk.. don't delay checking this into 1.4 in case you can't get a review
from Jan.

Comment 5

[timeless]

if you want r=timeless you can have it, i talked someone through this problem
when they foolishly built mozilla w/ the noprofiles flag.

Comment 6

[Pierre Chanial]

I am guilty of this left-over (bug 36339)
conrad's patch is ok with alec's comments: I simply missed the check for rv, and
this nsIprofile service result is not used (and is overwritten after).
In addition, useprofile is not used after the statement that simply sets the
bookmarks top root name as 'Bookmarks'.

thanks!
r=pch

Comment 7

[Conrad Carlen (not reading bugmail)]

Fixed on trunk.

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 126389 [details] [diff] [review]
patch

moving approval request forward.

Comment 9

[Mike Kaply [:mkaply]]

Comment on attachment 126389 [details] [diff] [review]
patch

a=mkaply

Comment 10

[Mike Kaply [:mkaply]]

Please add the fixed1.4.1 keyword when this is checked in.

Comment 11

[Jan Varga [:janv]]

checked into the 1.4 branch