SQLite Forum

several potential NULL pointer dereferences
Login
I think you are missing something important in your "potential" problem diagnosis. A hint as to what that something is appears in the comment header for the function you suggest might return a NULL reference:<code>
/*
** Skip over any TK_COLLATE operators and/or any unlikely()
** or likelihood() or likely() functions at the root of an
** expression.
*/
</code>
What this means is that the function is not allocating anything, and cannot return NULL due to its own (or its callees') allocation failure. It merely walks some part of an expression tree to ignore some part.  Or, if passed NULL, it returns NULL. As long as the part to be ignored did not incorporate a NULL expression (sub)tree, the sqlite3ExprSkipCollateAndLikely(pExpr) call is not going to return NULL.  Whether that can happen or not is a question of parser correctness and correctness of any parse tree manipulations that are done.

On the possibility that sqlite3ExprSkipCollateAndLikely(pExpr) is called with pExpr == NULL, I think you need to show that such an actual argument has some real prospect of occurring before issuing "should" recommendations for a bunch of additional conditionals in oft-executed code. My perusal of a subset of those calls has shown that, at the point of sqlite3ExprSkipCollateAndLikely() calls, the argument is well past the point in the code where it might be NULL.

I was curious about your assertion, "[a] similar problem has been fixed". (The referenced checkin is [here](https://www2.sqlite.org/cgi/src/info/5aeb5a2d295e10d5).) There, a subtree is passed to sqlite3ExprSkipCollateAndLikely(). I do not see that said change was a fix for a "similar problem." Whether an expression tree or subtree can contain NULL children is very dependent upon rules (or invariants) being maintained in specific parts of the code or for specific subtree types. Before I would go along with your "similar" assertion, I would need to see at least the same (or similar) subtree types being manipulated, and I would also want to know whether possible OOM events have already been handled before that code can be reached.

From what I have seen of the SQLite codebase, I believe that even if you have hit a problem with your scattershot diagnosis, the solution would be narrowly targeted adherence to existing invariants rather than a bunch of likely useless NULL checks.