[sqlite-dev] PVS-Studio Testing on SQLite Code and Related Issues
Andrey Karpov
Hello. I am one of the PVS-Studio analyzer developers. We are currently testing some of our latest diagnostics on various projects, including the SQLite code. Some new diagnostics issue warnings, but we aren't sure that these are errors. First of all, SQLite is a small, high-quality project. And secondly, the code is sometimes very complex and confusing for the analyzer (and a third-party person as well ;). That's why I don't want to jump to conclusions. It's interesting to know the opinion of SQLite developers.
I'll describe two cases below. Please, tell me, in which case it's a false positive and which one is relevant (contains an error).
Fragment N1. PVS-Studio warning: V1040 Possible typo in the spelling of a pre-defined macro name. The '__minux' macro is similar to '__linux'. shell.c 242
Код:
#if !defined(_WIN32) && !defined(WIN32) && !defined(__minux)
What's this magic "__minux"? May be authors wanted to write "__linux__"? Or it is correct and it's a special feature, isn't it?
Fragment N2. PVS-Studio warning: V1044 The diagnostic isn't named yet. sqlite3.c 49362
The code (I removed asserts not to distract from the main point)
static void pcache1EnforceMaxPage(PCache1 *pCache){
PGroup *pGroup = pCache->pGroup;
PgHdr1 *p;
while( pGroup->nPurgeable>pGroup->nMaxPage
&& (p=pGroup->lru.pLruPrev)->isAnchor==0
){
pcache1PinPage(p);
pcache1RemoveFromHash(p, 1);
}
if( pCache->nPage==0 && pCache->pBulk ){
sqlite3_free(pCache->pBulk);
pCache->pBulk = pCache->pFree = 0;
}
}
The analyzer suspects that there may be an infinity loop. Is that true? Reviewing this code, I tend to think that the analyzer is right, but I'm not sure, because the code is quite complex.
In the loop, only members of the 'PgHdr1' class, which the 'p' pointer points to, can change. The variable pGroup->nPurgeable>pGroup->nMaxPage in the loop cannot change, as there is no access to this variable via the 'p' pointer.
The only thing that can change is 'isAnchor'. But when I look into the called functions, I don't see the 'isAnchor' change anywhere. Is it really an error or do we (I and the analyzer) miss something?
----
Best regards,
Andrey Karpov, Microsoft MVP,
Ph.D. in Mathematics, CTO
"Program Verification Systems" Co Ltd.
URL: www.viva64.com
Bob Friesenhahn
>
> #if !defined(_WIN32) && !defined(WIN32) && !defined(__minux)
>
> What's this magic "__minux"? May be authors wanted to write "__linux__"? Or
> it is correct and it's a special feature, isn't it?
Minux is a famous operating system which continues to this day. See
https://en.wikipedia.org/wiki/MINIX and http://www.minix3.org/.
Bob
--
Bob Friesenhahn
bfri...@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer, http://www.GraphicsMagick.org/
Public Key, http://www.simplesystems.org/users/bfriesen/public-key.txt
_______________________________________________
sqlite-dev mailing list
sqlit...@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-dev
Dan Kennedy
pGroup is accessible via pointer p. See the first removed assert() for proof:
assert( p->pCache->pGroup==pGroup );
Dan.