295927 - Null pointer checks in RDF container code

Status: RESOLVED INCOMPLETE

(Core Graveyard :: RDF, defect)

Description

[Neil Deakin]

datasource.GetTarget is supposed to return NS_RDF_NO_VALUE when a value isn't
present. Some implementations of nsIRDFDataSource (like JS components such as
the extension manager) just assume that returning null is ok, causing crashes in
certain situations.

Comment 1

[Neil Deakin]

Attachment: Add some null guards

Comment 2

[Axel Hecht]

Comment on attachment 184830 [details] [diff] [review]
Add some null guards

Grrr, returning null and success is verboten.
Please file bugs on the implementations?

Whatever, CompositeDataSourceImpl::GetSource has the same problem.

Could you use the wallpaper of
http://lxr.mozilla.org/seamonkey/source/rdf/base/src/nsCompositeDataSource.cpp#
245,
with a NS_WARNING? Just accepting it is bad, IMHO.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Actually, you probably want to assert when people violate the api...  And yes,
file bugs on the api implementors.

Comment 4

[Neil Deakin]

Unfortunately, success return codes in JS are so inconsistently broken that
fixing the implementors tends to make things worse. I'd file a bug, unless there
is one, but coming up with a reliable testcase is hard.

Comment 5

[Boris Zbarsky [:bzbarsky]]

I'd change the JS to return the right code, and then whenever we make XPConnect suck less it should work...

Comment 6

[Neil Deakin]

I wouldn't. I'd change the API such that returning null works. There are hundreds of consumers and extensions that assume that returning null indicates no value, and about 3, all in Mozilla code, that assume the success code is returned.

Returning a success code is a non-sensical thing anyway.