[SERVER-14502] dereference of canonical query before NULL check in getExecutorIDHack() Created: 08/Jul/14  Updated: 26/Jul/14  Resolved: 09/Jul/14

Status: Closed
Project: Core Server
Component/s: Querying
Affects Version/s: None
Fix Version/s: 2.7.4

Type: Bug Priority: Major - P3
Reporter: Coverity Collector User Assignee: David Storch
Resolution: Done Votes: 0
Labels: coverity
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Operating System: ALL
Participants:

 Description   

A new defect has been detected and assigned to david.storch in Coverity Connect.
http://coverity.mongodb.com//sourcebrowser.htm?projectId=10001#mergedDefectId=25650
The defect was flagged by checker REVERSE_INULL in
file /src/mongo/db/query/get_executor.cpp
function mongo::getExecutorIDHack(mongo::Collection *, mongo::CanonicalQuery *, const mongo::QueryPlannerParams &, mongo::PlanExecutor **)
and this ticket was created by asya



 Comments   
Comment by Githook User [ 09/Jul/14 ]

Author:

{u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}

Message: SERVER-14502 remove unnecessary check for NULL canonical query
Branch: master
https://github.com/mongodb/mongo/commit/ac6619a626d96de740b196340e3aaeb99c6caadd

Comment by David Storch [ 09/Jul/14 ]

Ah, you're right, my fault. I missed the code that dereferences 'query' in getExecutorIDHack(). The 'query' cannot be NULL as things stand today, but it will be NULL soon. Simple fix coming up.

Comment by Asya Kamsky [ 09/Jul/14 ]

That's not what the complaint it. The complaint is that you are checking for NULL when you've already dereferenced the value.

That's inconsistent. If the query cannot be NULL, then why is the check there. If the query might be NULL at some point in the future, you need to protect the earlier dereference that has already happened before you get to the NULL check.

The reason this is a new defect is that the NULL check was already there, but new IDHackStage code was recently added and it assumes the value is not NULL (inside the call dereferences query unconditionally).

Comment by David Storch [ 08/Jul/14 ]

Coverity is complaining that we are checking for NULL even though there is no possibility of 'query' being NULL. This is true in the code today, but in the future it will not be. We may call getExecutorIDHack(...) in the future with a NULL canonical query so that the update system can use the IDHackStage without canonicalizing. Closing as "Won't Fix".

Generated at Thu Feb 08 03:35:04 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.