test-and-dereference: a common problem?
Paul Millar
paul at astro.gla.ac.uk
Mon May 16 04:51:34 CDT 2005
OK, sometimes I feel like I'm living on the edge of things here. Am I
the only one spotting these things? :^)
Anyway, I noticed that the dlls/advapi32/crypt.c tests were bombing out
for me. This turned out to be due to dlls/advapi32/crypt.c:1649
> if (!key || !pbData || !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
> CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
with the problem being triggered by dlls/advapi32/tests/crypt.c:243,
the relevant part being:
[...]
> result = pCryptDestroyKey(hKey2);
> ok (result, "%ld\n", GetLastError());
>
> dwTemp = CRYPT_MODE_ECB;
> result = pCryptSetKeyParam(hKey2, KP_MODE, (BYTE*)&dwTemp, sizeof(DWORD));
> ok (!result && GetLastError() == ERROR_INVALID_PARAMETER, "%ld\n", GetLastError());
So, this makes sense. Its that old chestnut of compiler being free to
evaluate if clauses in any order it chooses (for optimisation).
So, simple mistake, easy to fix. BUT, I decided to investigate a
little further. The following is a one-liner bash script that looks
for variables that are tested, then dereferenced (as with both key and
key->pProvider above). (I've split it over a few lines, hoping to
make it easier to read)
find . -name \*.[ch] |
while read f; do
egrep -H 'if *\( *!([a-zA-Z0-9_>-]+) *\|\|.*(\*\1[^a-zA-Z0-9_]|\1->)' $f;
done
Guess what, there are some 155 such test-and-dereference "if"s
currently. This problem is somewhat endemic!
For the most part, gcc seems to "do the right thing", as its
cheaper to evaluate !a than (a->prop == BAD_VAL). For the crypt.c
case, its somehow cheaper to dereference key->pProvider first before
doing the !key test; perhaps, due to the final key->pProvider->dwMagic
test and/or the value of key being cached in processor L1 cache, or
somethin' ...
But, I think we're leaving ourselves vulnerable to some weird errors
in the future. I've started patching these (see patch at end of
email), but stopped to do a reality check here. I'm still surprised
that this has slipped under the net, so to speak.
For the most part this isn't difficult: routines tend to simply return
an error code when a pointer is NULL. So I've explicitly separated
the if clauses. Using the example above:
if (!key || !pbData)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
if (!key->pProvider)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
if (key->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
Ugly, but fairly obvious what's going on.
Sometimes (e.g. due to critical sections), its desirable to just set a
return variable. In these cases I've used the tertiary ? operator. I
like it, but I know its rather fallen from favour with some people, so
does anyone have strong opinions about its use?
For more complex cases, I've added a variable (an int, as it happens)
to carry the boolean value. For example:
int bad_ptr;
if( !(bad_ptr = !ptr))
bad_ptr = (ptr->pVal == BAD_VALUE);
if( bad_ptr) {
... code here ...
}
This, I hope, is brief without loosing its clarity overly.
I was thinking about making this if( !(...)) into a macro as its used
fairly often. Something like:
#define TEST_BAD_PTR(A,B,C) if( !(A = B)) A = (C);
TEST_BAD_PTR( is_bad, !key || !other_ptr, !key->something)
if( is_bad) {
/* do stuff here */
return -1;
}
Does anyone have strong opinions about this?
So, without further ado, here's the patch. Its one big one, but I can
split it on a per-DLL basis, if that helps any.
Cheers,
Paul.
ChangeLog:
Fix some test-and-dereference if statements.
Index: dlls/advapi32/crypt.c
===================================================================
RCS file: /home/wine/wine/dlls/advapi32/crypt.c,v
retrieving revision 1.64
diff -u -r1.64 crypt.c
--- dlls/advapi32/crypt.c 20 Apr 2005 15:18:43 -0000 1.64
+++ dlls/advapi32/crypt.c 16 May 2005 09:38:23 -0000
@@ -258,6 +258,7 @@
PSTR imagepath = NULL, keyname = NULL, provname = NULL, temp = NULL;
DWORD keytype, type, len;
ULONG r;
+ int no_csp_name;
TRACE("(%p, %s, %s, %ld, %08lx)\n", phProv, pszContainer,
pszProvider, dwProvType, dwFlags);
@@ -274,7 +275,10 @@
return FALSE;
}
- if (!pszProvider || !*pszProvider)
+ if( !(no_csp_name = !pszProvider))
+ no_csp_name = !*pszProvider;
+
+ if (no_csp_name)
{
/* No CSP name specified so try the user default CSP first
* then try the machine default CSP
@@ -668,7 +672,13 @@
TRACE("(0x%lx, 0x%lx, %d, %08lx, %p, %p)\n", hKey, hHash, Final, dwFlags, pbData, pdwDataLen);
- if (!key || !pbData || !pdwDataLen || !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if( !key || !pbData || !pdwDataLen)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (!key->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (key->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = key->pProvider;
@@ -744,7 +754,10 @@
if (!hash)
CRYPT_ReturnLastError(ERROR_INVALID_HANDLE);
- if (!hash->pProvider || hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!hash->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = hash->pProvider;
@@ -776,7 +789,10 @@
if (!key)
CRYPT_ReturnLastError(ERROR_INVALID_HANDLE);
- if (!key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!key->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (key->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = key->pProvider;
@@ -809,8 +825,12 @@
TRACE("(0x%lx, %p, %08ld, %p)\n", hHash, pdwReserved, dwFlags, phHash);
orghash = (PCRYPTHASH)hHash;
- if (!orghash || pdwReserved || !phHash || !orghash->pProvider ||
- orghash->pProvider->dwMagic != MAGIC_CRYPTPROV)
+
+ if( !orghash || !phHash || pdwReserved )
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if ( !orghash->pProvider ||
+ orghash->pProvider->dwMagic != MAGIC_CRYPTPROV)
{
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
}
@@ -855,7 +875,11 @@
TRACE("(0x%lx, %p, %08ld, %p)\n", hKey, pdwReserved, dwFlags, phKey);
orgkey = (PCRYPTKEY)hKey;
- if (!orgkey || pdwReserved || !phKey || !orgkey->pProvider ||
+
+ if( !orgkey || pdwReserved || !phKey)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if ( !orgkey->pProvider ||
orgkey->pProvider->dwMagic != MAGIC_CRYPTPROV)
{
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
@@ -910,7 +934,13 @@
TRACE("(0x%lx, 0x%lx, %d, %08ld, %p, %p, %ld)\n", hKey, hHash, Final, dwFlags, pbData, pdwDataLen, dwBufLen);
- if (!key || !pdwDataLen || !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!key || !pdwDataLen)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (!key->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (key->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = key->pProvider;
@@ -1152,7 +1182,10 @@
TRACE("(0x%lx, 0x%lx, %ld, %08ld, %p, %p)\n", hKey, hExpKey, dwBlobType, dwFlags, pbData, pdwDataLen);
- if (!key || !pdwDataLen || !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!key || !pdwDataLen)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (!key->pProvider)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = key->pProvider;
@@ -1182,9 +1215,9 @@
TRACE("(0x%lx, %d, %08ld, %p)\n", hProv, Algid, dwFlags, phKey);
- if (!prov)
+ if (!phKey || !prov)
CRYPT_ReturnLastError(ERROR_INVALID_HANDLE);
- if (!phKey || !prov || prov->dwMagic != MAGIC_CRYPTPROV)
+ if (prov->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
if ( !(key = CRYPT_Alloc(sizeof(CRYPTKEY))) )
CRYPT_ReturnLastError(ERROR_NOT_ENOUGH_MEMORY);
@@ -1313,7 +1346,13 @@
TRACE("(0x%lx, %ld, %p, %p, %08ld)\n", hHash, dwParam, pbData, pdwDataLen, dwFlags);
- if (!hash || !pdwDataLen || !hash->pProvider || hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!hash || !pdwDataLen)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (!hash->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = hash->pProvider;
@@ -1348,7 +1387,13 @@
TRACE("(0x%lx, %ld, %p, %p, %08ld)\n", hKey, dwParam, pbData, pdwDataLen, dwFlags);
- if (!key || !pdwDataLen || !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!key || !pdwDataLen)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (!key->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (key->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = key->pProvider;
@@ -1382,7 +1427,10 @@
TRACE("(0x%lx, %ld, %p, %p, %08ld)\n", hProv, dwParam, pbData, pdwDataLen, dwFlags);
- if (!prov || prov->dwMagic != MAGIC_CRYPTPROV)
+ if (!prov)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (prov->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
return prov->pFuncs->pCPGetProvParam(prov->hPrivate, dwParam, pbData, pdwDataLen, dwFlags);
@@ -1452,9 +1500,11 @@
TRACE("(0x%lx, %p, %ld, %08ld)\n", hHash, pbData, dwDataLen, dwFlags);
- if (!hash)
+ if (!hash || !pbData || !dwDataLen)
CRYPT_ReturnLastError(ERROR_INVALID_HANDLE);
- if (!pbData || !dwDataLen || !hash->pProvider || hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!hash->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+ if (hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = hash->pProvider;
@@ -1484,7 +1534,10 @@
if (!hash || !key)
CRYPT_ReturnLastError(ERROR_INVALID_HANDLE);
- if (!hash->pProvider || hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!hash->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = hash->pProvider;
@@ -1514,7 +1567,10 @@
TRACE("(0x%lx, %p, %ld, 0x%lx, %08ld, %p)\n", hProv, pbData, dwDataLen, hPubKey, dwFlags, phKey);
- if (!prov || !pbData || !dwDataLen || !phKey || prov->dwMagic != MAGIC_CRYPTPROV)
+ if (!prov || !pbData || !dwDataLen || !phKey)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (prov->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
if ( !(importkey = CRYPT_Alloc(sizeof(CRYPTKEY))) )
@@ -1563,9 +1619,11 @@
TRACE("(0x%lx, %ld, %s, %08ld, %p, %p)\n",
hHash, dwKeySpec, debugstr_w(sDescription), dwFlags, pbSignature, pdwSigLen);
- if (!hash)
+ if (!hash || !pdwSigLen)
CRYPT_ReturnLastError(ERROR_INVALID_HANDLE);
- if (!pdwSigLen || !hash->pProvider || hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!hash->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+ if (hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = hash->pProvider;
@@ -1616,7 +1674,13 @@
TRACE("(0x%lx, %ld, %p, %08ld)\n", hHash, dwParam, pbData, dwFlags);
- if (!hash || !pbData || !hash->pProvider || hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!hash || !pbData)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (!hash->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (hash->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = hash->pProvider;
@@ -1646,7 +1710,13 @@
TRACE("(0x%lx, %ld, %p, %08ld)\n", hKey, dwParam, pbData, dwFlags);
- if (!key || !pbData || !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!key || !pbData)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (!key->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+
+ if (key->pProvider->dwMagic != MAGIC_CRYPTPROV)
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
prov = key->pProvider;
@@ -1856,11 +1926,12 @@
TRACE("(0x%lx, %p, %ld, 0x%lx, %s, %08ld)\n", hHash, pbSignature,
dwSigLen, hPubKey, debugstr_w(sDescription), dwFlags);
- if (!hash || !key)
+ if (!hash || !key || !pbSignature || !dwSigLen)
CRYPT_ReturnLastError(ERROR_INVALID_HANDLE);
- if (!pbSignature || !dwSigLen ||
- !hash->pProvider || hash->pProvider->dwMagic != MAGIC_CRYPTPROV ||
- !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
+ if (!hash->pProvider || !key->pProvider)
+ CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
+ if ( hash->pProvider->dwMagic != MAGIC_CRYPTPROV ||
+ key->pProvider->dwMagic != MAGIC_CRYPTPROV)
{
CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
}
Index: dlls/comctl32/comctl32undoc.c
===================================================================
RCS file: /home/wine/wine/dlls/comctl32/comctl32undoc.c,v
retrieving revision 1.99
diff -u -r1.99 comctl32undoc.c
--- dlls/comctl32/comctl32undoc.c 6 May 2005 15:44:32 -0000 1.99
+++ dlls/comctl32/comctl32undoc.c 16 May 2005 09:38:32 -0000
@@ -1909,7 +1909,10 @@
{
INT i;
- if (!hdpa || !hdpa->ptrs)
+ if (!hdpa)
+ return -1;
+
+ if (!hdpa->ptrs)
return -1;
for (i = 0; i < hdpa->nItemCount; i++) {
Index: dlls/comctl32/imagelist.c
===================================================================
RCS file: /home/wine/wine/dlls/comctl32/imagelist.c,v
retrieving revision 1.98
diff -u -r1.98 imagelist.c
--- dlls/comctl32/imagelist.c 6 May 2005 15:44:32 -0000 1.98
+++ dlls/comctl32/imagelist.c 16 May 2005 09:38:43 -0000
@@ -1064,7 +1064,10 @@
BOOL bIsTransparent, bBlend, bResult = FALSE, bMask;
HIMAGELIST himl;
- if (!pimldp || !(himl = pimldp->himl)) return FALSE;
+ if( !pimldp)
+ return FALSE;
+
+ if (!(himl = pimldp->himl)) return FALSE;
if (!is_valid(himl)) return FALSE;
if ((pimldp->i < 0) || (pimldp->i >= himl->cCurImage)) return FALSE;
Index: dlls/comctl32/listview.c
===================================================================
RCS file: /home/wine/wine/dlls/comctl32/listview.c,v
retrieving revision 1.409
diff -u -r1.409 listview.c
--- dlls/comctl32/listview.c 6 May 2005 15:44:32 -0000 1.409
+++ dlls/comctl32/listview.c 16 May 2005 09:39:19 -0000
@@ -3455,7 +3455,12 @@
TRACE("(lpLVItem=%s, isW=%d)\n", debuglvitem_t(lpLVItem, isW), isW);
- if (!lpLVItem || lpLVItem->iItem < 0 || lpLVItem->iItem >= infoPtr->nItemCount)
+ assert( infoPtr);
+
+ if( !lpLVItem)
+ return FALSE;
+
+ if (lpLVItem->iItem < 0 || lpLVItem->iItem >= infoPtr->nItemCount)
return FALSE;
/* For efficiency, we transform the lpLVItem->pszText to Unicode here */
@@ -5026,7 +5031,12 @@
TRACE("(lpLVItem=%s, isW=%d)\n", debuglvitem_t(lpLVItem, isW), isW);
- if (!lpLVItem || lpLVItem->iItem < 0 || lpLVItem->iItem >= infoPtr->nItemCount)
+ assert( infoPtr);
+
+ if( !lpLVItem)
+ return FALSE;
+
+ if (lpLVItem->iItem < 0 || lpLVItem->iItem >= infoPtr->nItemCount)
return FALSE;
if (lpLVItem->mask == 0) return TRUE;
@@ -5264,7 +5274,12 @@
LPWSTR pszText;
BOOL bResult;
- if (!lpLVItem || lpLVItem->iItem < 0 || lpLVItem->iItem >= infoPtr->nItemCount)
+ assert( infoPtr);
+
+ if( !lpLVItem)
+ return FALSE;
+
+ if (lpLVItem->iItem < 0 || lpLVItem->iItem >= infoPtr->nItemCount)
return FALSE;
pszText = lpLVItem->pszText;
@@ -5613,6 +5628,8 @@
*/
static INT LISTVIEW_GetItemTextT(LISTVIEW_INFO *infoPtr, INT nItem, LPLVITEMW lpLVItem, BOOL isW)
{
+ assert( infoPtr);
+
if (!lpLVItem || nItem < 0 || nItem >= infoPtr->nItemCount) return 0;
lpLVItem->mask = LVIF_TEXT;
@@ -6086,8 +6103,11 @@
if (infoPtr->dwStyle & LVS_OWNERDATA) return infoPtr->nItemCount++;
+
/* make sure it's an item, and not a subitem; cannot insert a subitem */
- if (!lpLVItem || lpLVItem->iItem < 0 || lpLVItem->iSubItem) return -1;
+ if( !lpLVItem)
+ return -1;
+ if (lpLVItem->iItem < 0 || lpLVItem->iSubItem) return -1;
if (!is_assignable_item(lpLVItem, infoPtr->dwStyle)) return -1;
@@ -8224,10 +8244,13 @@
static LRESULT LISTVIEW_HeaderNotification(LISTVIEW_INFO *infoPtr, const NMHEADERW *lpnmh)
{
UINT uView = infoPtr->dwStyle & LVS_TYPEMASK;
+ int truth; /* TODO: what should this be called? */
TRACE("(lpnmh=%p)\n", lpnmh);
- if (!lpnmh || lpnmh->iItem < 0 || lpnmh->iItem >= DPA_GetPtrCount(infoPtr->hdpaColumns)) return 0;
+ if( !lpnmh)
+ return 0;
+ if (lpnmh->iItem < 0 || lpnmh->iItem >= DPA_GetPtrCount(infoPtr->hdpaColumns)) return 0;
switch (lpnmh->hdr.code)
{
@@ -8239,7 +8262,9 @@
COLUMN_INFO *lpColumnInfo;
INT dx, cxy;
- if (!lpnmh->pitem || !(lpnmh->pitem->mask & HDI_WIDTH))
+ if( !(truth = !lpnmh->pitem))
+ truth = !(lpnmh->pitem->mask & HDI_WIDTH);
+ if (truth)
{
HDITEMW hdi;
Index: dlls/comctl32/tab.c
===================================================================
RCS file: /home/wine/wine/dlls/comctl32/tab.c,v
retrieving revision 1.109
diff -u -r1.109 tab.c
--- dlls/comctl32/tab.c 12 May 2005 09:57:10 -0000 1.109
+++ dlls/comctl32/tab.c 16 May 2005 09:39:31 -0000
@@ -2573,7 +2573,10 @@
TRACE("(%p,%d,%s)\n", infoPtr, iItem, fHighlight ? "true" : "false");
- if (!infoPtr || iItem < 0 || iItem >= infoPtr->uNumItem)
+ if( !infoPtr)
+ return FALSE;
+
+ if (iItem < 0 || iItem >= infoPtr->uNumItem)
return FALSE;
lpState = &TAB_GetItem(infoPtr, iItem)->dwState;
Index: dlls/dbghelp/msc.c
===================================================================
RCS file: /home/wine/wine/dlls/dbghelp/msc.c,v
retrieving revision 1.13
diff -u -r1.13 msc.c
--- dlls/dbghelp/msc.c 29 Mar 2005 11:30:57 -0000 1.13
+++ dlls/dbghelp/msc.c 16 May 2005 09:39:44 -0000
@@ -496,7 +496,7 @@
{
struct symt_udt* symt;
const unsigned char* ptr = list;
- int value, leaf_len;
+ int value, leaf_len, truth;
const struct p_string* p_name;
const char* c_name;
struct symt* subtype;
@@ -565,7 +565,11 @@
p_name = (const struct p_string*)((const char*)&type->member_v1.offset + leaf_len);
subtype = codeview_get_type(type->member_v1.type, TRUE);
- if (!subtype || subtype->tag != SymTagCVBitField)
+ /* Avoid dereferencing subtype if NULL. TODO: what should "truth" be? */
+ if( !( truth = !subtype))
+ truth = subtype->tag != SymTagCVBitField;
+
+ if ( truth)
{
DWORD elem_size = 0;
if (subtype) symt_get_info(subtype, TI_GET_LENGTH, &elem_size);
@@ -589,7 +593,11 @@
p_name = (const struct p_string*)((const unsigned char*)&type->member_v2.offset + leaf_len);
subtype = codeview_get_type(type->member_v2.type, TRUE);
- if (!subtype || subtype->tag != SymTagCVBitField)
+ /* Avoid dereferencing subtype if NULL */
+ if( !( truth = !subtype))
+ truth = subtype->tag != SymTagCVBitField;
+
+ if (truth)
{
DWORD elem_size = 0;
if (subtype) symt_get_info(subtype, TI_GET_LENGTH, &elem_size);
@@ -612,7 +620,11 @@
c_name = (const char*)&type->member_v3.offset + leaf_len;
subtype = codeview_get_type(type->member_v3.type, TRUE);
- if (!subtype || subtype->tag != SymTagCVBitField)
+ /* Avoid dereferencing subtype if NULL */
+ if( !( truth = !subtype))
+ truth = subtype->tag != SymTagCVBitField;
+
+ if (truth)
{
DWORD elem_size = 0;
if (subtype) symt_get_info(subtype, TI_GET_LENGTH, &elem_size);
@@ -1614,7 +1626,8 @@
const WORD* block_list;
DWORD i;
- if (!toc || file_nr >= toc->num_files) return NULL;
+ if (!toc) return NULL;
+ if (file_nr >= toc->num_files) return NULL;
block_list = (const WORD*) &toc->file[toc->num_files];
for (i = 0; i < file_nr; i++)
@@ -1629,7 +1642,8 @@
const DWORD* block_list;
DWORD i;
- if (!toc || file_nr >= toc->num_files) return NULL;
+ if (!toc) return NULL;
+ if (file_nr >= toc->num_files) return NULL;
if (toc->file_size[file_nr] == 0 || toc->file_size[file_nr] == 0xFFFFFFFF)
{
Index: dlls/ddraw/dsurface/main.c
===================================================================
RCS file: /home/wine/wine/dlls/ddraw/dsurface/main.c,v
retrieving revision 1.64
diff -u -r1.64 main.c
--- dlls/ddraw/dsurface/main.c 7 Mar 2005 12:23:34 -0000 1.64
+++ dlls/ddraw/dsurface/main.c 16 May 2005 09:39:50 -0000
@@ -429,7 +429,9 @@
TRACE("(%p)->(%08lx,%p)\n",This,dwFlags,pAttach);
- if (!surf || (surf->surface_owner != This))
+ if (!surf)
+ return DDERR_SURFACENOTATTACHED; /* unchecked */
+ if (surf->surface_owner != This)
return DDERR_SURFACENOTATTACHED; /* unchecked */
surf->detach(surf);
Index: dlls/gdi/bitmap.c
===================================================================
RCS file: /home/wine/wine/dlls/gdi/bitmap.c,v
retrieving revision 1.7
diff -u -r1.7 bitmap.c
--- dlls/gdi/bitmap.c 13 Apr 2005 16:11:18 -0000 1.7
+++ dlls/gdi/bitmap.c 16 May 2005 09:39:55 -0000
@@ -234,9 +234,15 @@
BITMAPOBJ *bmpobj;
HBITMAP hbitmap;
- if (!bmp || bmp->bmType || bmp->bmPlanes != 1)
+ if( !bmp) {
+ SetLastError( ERROR_INVALID_PARAMETER );
+ return NULL;
+ }
+
+
+ if( bmp->bmType || bmp->bmPlanes != 1)
{
- if (bmp && bmp->bmPlanes != 1)
+ if( bmp->bmPlanes != 1)
FIXME("planes = %d\n", bmp->bmPlanes);
SetLastError( ERROR_INVALID_PARAMETER );
return NULL;
Index: dlls/gdi/freetype.c
===================================================================
RCS file: /home/wine/wine/dlls/gdi/freetype.c,v
retrieving revision 1.85
diff -u -r1.85 freetype.c
--- dlls/gdi/freetype.c 6 May 2005 16:22:55 -0000 1.85
+++ dlls/gdi/freetype.c 16 May 2005 09:40:09 -0000
@@ -418,7 +418,7 @@
#ifdef HAVE_FREETYPE_FTWINFNT_H
FT_WinFNT_HeaderRec winfnt_header;
#endif
- int i, bitmap_num;
+ int i, bitmap_num, not_newer;
do {
char *family_name = fake_family;
@@ -505,19 +505,24 @@
pFT_Done_Face(ft_face);
return FALSE;
}
- if(!pHeader || pHeader->Font_Revision <= face->font_version) {
+
+ /* Avoid dereferencing pHeader if NULL */
+ if( !(not_newer = !pHeader))
+ not_newer=(pHeader->Font_Revision <= face->font_version);
+
+ if( not_newer) {
TRACE("Original font is newer so skipping this one\n");
HeapFree(GetProcessHeap(), 0, StyleW);
pFT_Done_Face(ft_face);
return FALSE;
- } else {
- TRACE("Replacing original with this one\n");
- list_remove(&face->entry);
- HeapFree(GetProcessHeap(), 0, face->file);
- HeapFree(GetProcessHeap(), 0, face->StyleName);
- HeapFree(GetProcessHeap(), 0, face);
- break;
- }
+ }
+
+ TRACE("Replacing original with this one\n");
+ list_remove(&face->entry);
+ HeapFree(GetProcessHeap(), 0, face->file);
+ HeapFree(GetProcessHeap(), 0, face->StyleName);
+ HeapFree(GetProcessHeap(), 0, face);
+ break;
}
}
face = HeapAlloc(GetProcessHeap(), 0, sizeof(*face));
Index: dlls/kernel/lcformat.c
===================================================================
RCS file: /home/wine/wine/dlls/kernel/lcformat.c,v
retrieving revision 1.12
diff -u -r1.12 lcformat.c
--- dlls/kernel/lcformat.c 27 Jan 2005 10:41:40 -0000 1.12
+++ dlls/kernel/lcformat.c 16 May 2005 09:40:19 -0000
@@ -170,6 +170,7 @@
};
static NLS_FORMAT_NODE *NLS_CachedFormats = NULL;
NLS_FORMAT_NODE *node = NLS_CachedFormats;
+ int not_cached;
dwFlags &= LOCALE_NOUSEROVERRIDE;
@@ -179,7 +180,11 @@
while (node && (node->lcid != lcid || node->dwFlags != dwFlags) && node->next)
node = node->next;
- if (!node || node->lcid != lcid || node->dwFlags != dwFlags)
+ /* Avoid dereferencing node if NULL */
+ if( !(not_cached = !node))
+ not_cached = (node->lcid != lcid || node->dwFlags != dwFlags);
+
+ if( not_cached)
{
NLS_FORMAT_NODE *new_node;
DWORD i;
Index: dlls/kernel/local16.c
===================================================================
RCS file: /home/wine/wine/dlls/kernel/local16.c,v
retrieving revision 1.11
diff -u -r1.11 local16.c
--- dlls/kernel/local16.c 14 May 2005 12:18:15 -0000 1.11
+++ dlls/kernel/local16.c 16 May 2005 09:40:28 -0000
@@ -200,7 +200,8 @@
LOCALHEAPINFO *pInfo;
INSTANCEDATA *ptr = MapSL( MAKESEGPTR( ds, 0 ));
TRACE("Heap at %p, %04x\n", ptr, (ptr != NULL ? ptr->heap : 0xFFFF));
- if (!ptr || !ptr->heap) return NULL;
+ if (!ptr) return NULL;
+ if (!ptr->heap) return NULL;
if (IsBadReadPtr16( (SEGPTR)MAKELONG(ptr->heap,ds), sizeof(LOCALHEAPINFO)))
{
WARN("Bad pointer\n");
@@ -2196,7 +2197,10 @@
LOCAL32HEADER *header = Local32_GetHeap( handle );
if ( !header ) return FALSE;
- if ( !pLocal32Info || pLocal32Info->dwSize < sizeof(LOCAL32INFO) )
+ if( !pLocal32Info)
+ return FALSE;
+
+ if ( pLocal32Info->dwSize < sizeof(LOCAL32INFO) )
return FALSE;
pLocal32Info->dwMemReserved = 0;
Index: dlls/kernel/locale.c
===================================================================
RCS file: /home/wine/wine/dlls/kernel/locale.c,v
retrieving revision 1.60
diff -u -r1.60 locale.c
--- dlls/kernel/locale.c 11 May 2005 12:57:50 -0000 1.60
+++ dlls/kernel/locale.c 16 May 2005 09:40:41 -0000
@@ -2956,8 +2956,15 @@
BOOL bContinue = TRUE, bAlternate = FALSE;
LGRPID lgrpid;
ULONG ulIndex = 1; /* Ignore default entry of 1st key */
+ int invalid;
+
+ if (!lpProcs)
+ {
+ SetLastError(ERROR_INVALID_PARAMETER);
+ return FALSE;
+ }
- if (!lpProcs || !lpProcs->lgrpid || lpProcs->lgrpid > LGRPID_ARMENIAN)
+ if (!lpProcs->lgrpid || lpProcs->lgrpid > LGRPID_ARMENIAN)
{
SetLastError(ERROR_INVALID_PARAMETER);
return FALSE;
Index: dlls/kernel/resource16.c
===================================================================
RCS file: /home/wine/wine/dlls/kernel/resource16.c,v
retrieving revision 1.8
diff -u -r1.8 resource16.c
--- dlls/kernel/resource16.c 24 Mar 2005 21:01:38 -0000 1.8
+++ dlls/kernel/resource16.c 16 May 2005 09:40:46 -0000
@@ -126,7 +126,9 @@
static HRSRC MapHRsrc16To32( NE_MODULE *pModule, HRSRC16 hRsrc16 )
{
HRSRC_MAP *map = (HRSRC_MAP *)pModule->hRsrcMap;
- if ( !map || !hRsrc16 || hRsrc16 > map->nUsed ) return 0;
+
+ if ( !map || !hRsrc16) return 0;
+ if ( hRsrc16 > map->nUsed ) return 0;
return map->elem[hRsrc16-1].hRsrc;
}
@@ -137,7 +139,8 @@
static WORD MapHRsrc16ToType( NE_MODULE *pModule, HRSRC16 hRsrc16 )
{
HRSRC_MAP *map = (HRSRC_MAP *)pModule->hRsrcMap;
- if ( !map || !hRsrc16 || hRsrc16 > map->nUsed ) return 0;
+ if ( !map || !hRsrc16) return 0;
+ if ( hRsrc16 > map->nUsed ) return 0;
return map->elem[hRsrc16-1].type;
}
@@ -390,7 +393,8 @@
FARPROC16 prevHandler = NULL;
NE_MODULE *pModule = NE_GetPtr( hModule );
- if (!pModule || !pModule->res_table) return NULL;
+ if (!pModule) return NULL;
+ if (!pModule->res_table) return NULL;
pResTab = (LPBYTE)pModule + pModule->res_table;
pTypeInfo = (NE_TYPEINFO *)(pResTab + 2);
@@ -892,7 +896,8 @@
HGLOBAL16 ret;
NE_MODULE *pModule = NE_GetPtr( hModule );
- if (!pModule || !pModule->res_table || !hRsrc) return 0;
+ if (!pModule || !hRsrc) return 0;
+ if (!pModule->res_table) return 0;
TRACE("module=%04x res=%04x size=%ld\n", hModule, hRsrc, size );
@@ -934,7 +939,8 @@
HFILE16 fd;
NE_MODULE *pModule = NE_GetPtr( hModule );
- if (!pModule || !pModule->res_table || !hRsrc) return -1;
+ if (!pModule || !hRsrc) return -1;
+ if (!pModule->res_table) return -1;
TRACE("module=%04x res=%04x\n", pModule->self, hRsrc );
Index: dlls/kernel/task.c
===================================================================
RCS file: /home/wine/wine/dlls/kernel/task.c,v
retrieving revision 1.22
diff -u -r1.22 task.c
--- dlls/kernel/task.c 14 May 2005 12:16:46 -0000 1.22
+++ dlls/kernel/task.c 16 May 2005 09:40:52 -0000
@@ -515,7 +515,10 @@
WORD args[5];
TDB *pTask = TASK_GetCurrent();
- if ( !pTask || !pTask->userhandler ) return;
+ if( !pTask)
+ return;
+
+ if ( !pTask->userhandler ) return;
args[4] = hTaskOrModule;
args[3] = uCode;
@@ -1463,7 +1466,8 @@
/* make sure that task and hInstance are valid (skip initial Wine task !) */
while (1) {
pTask = TASK_GetPtr( lpte->hNext );
- if (!pTask || pTask->magic != TDB_MAGIC) return FALSE;
+ if (!pTask) return FALSE;
+ if (pTask->magic != TDB_MAGIC) return FALSE;
if (pTask->hInstance)
break;
lpte->hNext = pTask->hNext;
@@ -1505,8 +1509,14 @@
void WINAPI FatalAppExit16( UINT16 action, LPCSTR str )
{
TDB *pTask = TASK_GetCurrent();
+ int have_not_dll;
+
+ /* Avoid dereferencing NULL */
+ if( ! (have_not_dll = !pTask))
+ have_not_dll = !(pTask->error_mode & SEM_NOGPFAULTERRORBOX);
+
- if (!pTask || !(pTask->error_mode & SEM_NOGPFAULTERRORBOX))
+ if ( have_not_dll)
{
HMODULE mod = GetModuleHandleA( "user32.dll" );
if (mod)
Index: dlls/mapi32/prop.c
===================================================================
RCS file: /home/wine/wine/dlls/mapi32/prop.c,v
retrieving revision 1.9
diff -u -r1.9 prop.c
--- dlls/mapi32/prop.c 23 Mar 2005 13:15:19 -0000 1.9
+++ dlls/mapi32/prop.c 16 May 2005 09:41:04 -0000
@@ -1190,7 +1190,10 @@
ULONG i;
TRACE("(%p)\n", lpRowSet);
- if (!lpRowSet || IsBadReadPtr(lpRowSet, CbSRowSet(lpRowSet)))
+ if (!lpRowSet)
+ return TRUE;
+
+ if (IsBadReadPtr(lpRowSet, CbSRowSet(lpRowSet)))
return TRUE;
for (i = 0; i < lpRowSet->cRows; i++)
@@ -1256,7 +1259,9 @@
ULONG i;
TRACE("(%p)\n", lpRow);
- if (!lpRow || IsBadReadPtr(lpRow, sizeof(SRow)) || !lpRow->lpProps ||
+ if (!lpRow) return TRUE;
+ if (!lpRow->lpProps) return TRUE;
+ if (IsBadReadPtr(lpRow, sizeof(SRow)) ||
IsBadReadPtr(lpRow->lpProps, lpRow->cValues * sizeof(SPropValue)))
return TRUE;
Index: dlls/mpr/wnet.c
===================================================================
RCS file: /home/wine/wine/dlls/mpr/wnet.c,v
retrieving revision 1.19
diff -u -r1.19 wnet.c
--- dlls/mpr/wnet.c 24 Mar 2005 21:01:38 -0000 1.19
+++ dlls/mpr/wnet.c 16 May 2005 09:41:14 -0000
@@ -320,7 +320,10 @@
{
DWORD ret = BAD_PROVIDER_INDEX;
- if (providerTable && providerTable->numProviders)
+ if( !providerTable)
+ return BAD_PROVIDER_INDEX;
+
+ if (providerTable->numProviders)
{
DWORD i;
@@ -404,7 +407,10 @@
{
PWNetEnumerator ret;
- if (!providerTable || index >= providerTable->numProviders)
+ if( !providerTable)
+ return NULL;
+
+ if (index >= providerTable->numProviders)
ret = NULL;
else
{
@@ -623,7 +629,9 @@
if (!lphEnum)
ret = WN_BAD_POINTER;
- else if (!providerTable || providerTable->numProviders == 0)
+ else if (!providerTable)
+ ret = WN_NO_NETWORK;
+ else if (providerTable->numProviders == 0)
ret = WN_NO_NETWORK;
else
{
@@ -712,7 +720,9 @@
if (!lphEnum)
ret = WN_BAD_POINTER;
- else if (!providerTable || providerTable->numProviders == 0)
+ else if (!providerTable) /* avoid dereferencing */
+ ret = WN_NO_NETWORK;
+ else if (providerTable->numProviders == 0)
ret = WN_NO_NETWORK;
else
{
Index: dlls/msacm/stream.c
===================================================================
RCS file: /home/wine/wine/dlls/msacm/stream.c,v
retrieving revision 1.17
diff -u -r1.17 stream.c
--- dlls/msacm/stream.c 11 Aug 2004 19:35:34 -0000 1.17
+++ dlls/msacm/stream.c 16 May 2005 09:41:16 -0000
@@ -90,7 +90,11 @@
WARN("invalid handle\n");
return MMSYSERR_INVALHANDLE;
}
- if (!pash || pash->cbStruct < sizeof(ACMSTREAMHEADER)) {
+ if (!pash) {
+ WARN("invalid parameter\n");
+ return MMSYSERR_INVALPARAM;
+ }
+ if (pash->cbStruct < sizeof(ACMSTREAMHEADER)) {
WARN("invalid parameter\n");
return MMSYSERR_INVALPARAM;
}
@@ -299,7 +303,11 @@
WARN("invalid handle\n");
return MMSYSERR_INVALHANDLE;
}
- if (!pash || pash->cbStruct < sizeof(ACMSTREAMHEADER)) {
+ if (!pash) {
+ WARN("invalid parameter\n");
+ return MMSYSERR_INVALPARAM;
+ }
+ if (pash->cbStruct < sizeof(ACMSTREAMHEADER)) {
WARN("invalid parameter\n");
return MMSYSERR_INVALPARAM;
}
@@ -443,7 +451,11 @@
WARN("invalid handle\n");
return MMSYSERR_INVALHANDLE;
}
- if (!pash || pash->cbStruct < sizeof(ACMSTREAMHEADER)) {
+ if (!pash) {
+ WARN("invalid parameter\n");
+ return MMSYSERR_INVALPARAM;
+ }
+ if (pash->cbStruct < sizeof(ACMSTREAMHEADER)) {
WARN("invalid parameter\n");
return MMSYSERR_INVALPARAM;
}
Index: dlls/ntdll/loader.c
===================================================================
RCS file: /home/wine/wine/dlls/ntdll/loader.c,v
retrieving revision 1.87
diff -u -r1.87 loader.c
--- dlls/ntdll/loader.c 21 Mar 2005 10:52:26 -0000 1.87
+++ dlls/ntdll/loader.c 16 May 2005 09:41:28 -0000
@@ -780,7 +780,8 @@
ULONG dirsize;
dir = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_TLS, &dirsize );
- if (!dir || !dir->AddressOfCallBacks) return;
+ if (!dir) return;
+ if (!dir->AddressOfCallBacks) return;
for (callback = dir->AddressOfCallBacks; *callback; callback++)
{
@@ -1013,10 +1014,12 @@
RtlEnterCriticalSection( &loader_section );
wm = get_modref( hModule );
- if (!wm || wm->ldr.TlsIndex != -1)
- ret = STATUS_DLL_NOT_FOUND;
- else
- wm->ldr.Flags |= LDR_NO_DLL_CALLS;
+
+ ret = (!wm)? STATUS_DLL_NOT_FOUND : \
+ (wm->ldr.TlsIndex != -1) ? STATUS_DLL_NOT_FOUND : STATUS_SUCCESS;
+
+ if( ret == STATUS_SUCCESS)
+ wm->ldr.Flags |= LDR_NO_DLL_CALLS;
RtlLeaveCriticalSection( &loader_section );
Index: dlls/oleaut32/safearray.c
===================================================================
RCS file: /home/wine/wine/dlls/oleaut32/safearray.c,v
retrieving revision 1.44
diff -u -r1.44 safearray.c
--- dlls/oleaut32/safearray.c 23 Mar 2005 13:15:19 -0000 1.44
+++ dlls/oleaut32/safearray.c 16 May 2005 09:41:34 -0000
@@ -1439,8 +1439,11 @@
SAFEARRAYBOUND *oldBounds;
TRACE("(%p,%p)\n", psa, psabound);
+
+ if( !psa)
+ return E_INVALIDARG;
- if (!psa || psa->fFeatures & FADF_FIXEDSIZE || !psabound)
+ if (psa->fFeatures & FADF_FIXEDSIZE || !psabound)
return E_INVALIDARG;
if (psa->cLocks > 0)
@@ -1555,7 +1558,10 @@
TRACE("(%p,%p)\n", psa, pRinfo);
- if (!psa || !(psa->fFeatures & FADF_RECORD))
+ if (!psa)
+ return E_INVALIDARG;
+
+ if (!(psa->fFeatures & FADF_RECORD))
return E_INVALIDARG;
if (pRinfo)
@@ -1590,7 +1596,10 @@
TRACE("(%p,%p)\n", psa, pRinfo);
- if (!psa || !pRinfo || !(psa->fFeatures & FADF_RECORD))
+ if( !psa || !pRinfo)
+ return E_INVALIDARG;
+
+ if ( !(psa->fFeatures & FADF_RECORD))
return E_INVALIDARG;
*pRinfo = src[-1];
@@ -1622,7 +1631,9 @@
TRACE("(%p,%s)\n", psa, debugstr_guid(guid));
- if (!psa || !guid || !(psa->fFeatures & FADF_HAVEIID))
+ if (!psa || !guid)
+ return E_INVALIDARG;
+ if (!(psa->fFeatures & FADF_HAVEIID))
return E_INVALIDARG;
dest[-1] = *guid;
@@ -1651,7 +1662,9 @@
TRACE("(%p,%p)\n", psa, pGuid);
- if (!psa || !pGuid || !(psa->fFeatures & FADF_HAVEIID))
+ if (!psa || !pGuid)
+ return E_INVALIDARG;
+ if (!(psa->fFeatures & FADF_HAVEIID))
return E_INVALIDARG;
*pGuid = src[-1];
@@ -1724,7 +1737,9 @@
*pbstr = NULL;
- if (!psa || psa->cbElements != 1 || psa->cDims != 1)
+ if (!psa)
+ return E_INVALIDARG;
+ if (psa->cbElements != 1 || psa->cDims != 1)
return E_INVALIDARG;
*pbstr = SysAllocStringByteLen(psa->pvData, psa->rgsabound[0].cElements);
Index: dlls/user/edit.c
===================================================================
RCS file: /home/wine/wine/dlls/user/edit.c,v
retrieving revision 1.25
diff -u -r1.25 edit.c
--- dlls/user/edit.c 4 May 2005 09:45:33 -0000 1.25
+++ dlls/user/edit.c 16 May 2005 09:41:56 -0000
@@ -1026,7 +1026,7 @@
HDC dc;
HFONT old_font = 0;
LPWSTR current_position, cp;
- INT fw;
+ INT fw, buf_expanded;
LINEDEF *current_line;
LINEDEF *previous_line;
LINEDEF *start_line;
@@ -1082,7 +1082,9 @@
do {
if (current_line != start_line)
{
- if (!current_line || current_line->index + delta > current_position - es->text)
+ if( !(buf_expanded = !current_line))
+ buf_expanded = current_line->index + delta > current_position - es->text;
+ if (buf_expanded)
{
/* The buffer has been expanded, create a new line and
insert it into the link list */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://www.winehq.org/pipermail/wine-patches/attachments/20050516/af0bbc4c/attachment.pgp
More information about the wine-patches
mailing list