[vbox-dev] [PATCH 1/8] Fix misused Assert*() macros

[Jung-uk Kim]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 03/26/2015 05:08, Frank Mehnert wrote:
> Hi,
> 
> On Friday 13 March 2015 16:56:47 Jung-uk Kim wrote:
>> include/VBox/com/array.h - fixes for null-pointer dereferences.
> 
> that's difficult. I know that compilers and static code checking
> tools complain but the intention behind these statements is to
> return NULL in case we are trying to access a NULL array or an
> index outside of the current array dimensions. Returning NULL to
> the caller will either be handled correctly by the caller or it
> will crash the application in a controlled way. Otherwise we would
> risk heap corruption which is hard to debug.

I understand what you're trying to do.  However, this code is
incorrect because it does not return NULL.  In fact, **(FOO **)NULL
accesses invalid memory and its caller can never have a chance to
handle it gracefully.  It can only raises an exception AFAICT.  BTW,
there is a line like this in the file:

    return m.arr[aIdx] ? *m.arr[aIdx] : *nsIDRef::Empty;

You need to do something like that if you really need to make it safe.

> But I know that many tools are not happy with these statements...

Rightly so because it is really wrong.

>> src/VBox/Runtime/common/ldr/ldrELFRelocatable.cpp.h - fixes
>> misused AssertReturn().
> 
> This is definitely a bug but we have to discuss internally if your
> fix is correct. Perhaps using AssertMsgReturn() would be more
> appropriate.
> 
>> src/VBox/VMM/VMMR3/PDMDriver.cpp - fixes misused
>> AssertLogRelReturn().
> 
> Correct. Applied.

I just realized I missed a file (attached).  Please commit this, too.

Thanks!

Jung-uk Kim
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVFFoAAAoJEHyflib82/FGH/kIAJ/p9VhApLsIJkpR/RviOTs5
0rVNJ76/gJBhw7CX1y/HGfybfB4oByicx4EWVYqTnSFRaH/Duv+rSjP9kleI3vFr
zJ1HBXMQuLS3SL4vB4lroU9DAttlZWeaTV9SZMEnz1d59HE/EhUUbfAu2kvTGnBP
erBqwiJXolfWjvlwX7ZOCJmy3cXxLPFarwNT0KOA3pk+miZCtwrNlrnL8eI/2z0y
eAT6T1IdlV9DEPhrVMPLIPgV5VdabXfJCfChiH400FjEYK81lroai0C7XZdCPiuS
Po2W42ixmnMgXAW6cD65EdHo0pDWIrbyqQnPZhhNJE+xy/NAciSYa6MVmQL7ZDk=
=XGzo
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch01-2.diff
Type: text/x-patch
Size: 734 bytes
Desc: not available
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20150326/52a980a1/attachment.bin>