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

Jung-uk Kim jkim at FreeBSD.org
Thu Mar 26 19:12:04 UTC 2015

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.


Jung-uk Kim
Version: GnuPG v2

-------------- 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 

More information about the vbox-dev mailing list