header *can't* be null, as XPT_DoHeader will return false when it sets header to null which means we take the early exit.

Comment on attachment 214401 [details] [diff] [review] try to quiet coverity by not null checking something that won't be null sr=bzbarsky, but I really don't know this code. Shouldn't shaver be in on this?

shaver didn't want to deal w/ the noise last night, so i've been a bit shy.

I have to ask, what is Coverty and why doesn't it like null checks? How about changing it to an assert to enforce the fact that it can't/shouldn't be null. If Coverty doesn't like that, smack it a good one for me.

http://scan.coverity.com it's a static analyzer. you can browse through bugzilla for bugs w/ the coverity keyword to see other examples of things it finds. in general, coverity is at the whim of the build style you choose, so if you try building disable-debug and asserts are replaced by empty, then coverity will ignore them. (whereas if you build w/ enable-debug, coverity would understand them....) i have no idea how this public coverity was configured (and i've never actually watched someone configure coverity, so this is just an impression), my guess is that it'd have been configured disable-debug so while we can assert, i'd imagine it'd ignore us. in this case, i don't see the point of asserting, it's just the way the functions work, no mystery. compare that to some other bugs where there are very long call chains with fairly disconnected assumptions.

I'm just going on the fact that the original author thought it was worth the effort to put in the check. I imagine nisheeth was just be cautious. I'd be curious to know why Coverty thinks this null check is bad. Is it the subsequent use of header without the null check that it's really concerned with? The correct solution is for XPT_DoHeader to assert the post condition. If we're going to put effort into this, wouldn't it be nice to get a little more out of it than just quieting some noise?

Comment on attachment 214401 [details] [diff] [review] try to quiet coverity by not null checking something that won't be null Yeah, this is right.

Comment on attachment 214401 [details] [diff] [review] try to quiet coverity by not null checking something that won't be null mozilla/xpcom/typelib/xpt/tools/xpt_link.c 1.34 yeah, coverity complains about the inconsistency. the decission about which is right is something that we get to figure out.