On 30/01/2013 10:31 a.m., Rainer Weikusat wrote:
> Rainer Weikusat <rweikusat_at_mobileactivedefense.com> writes:
>
> [...]
>
>> There are probably all kinds of 'weird, stylistic issues' and
>> possible, yet unknown bugs,
> Attached is a slightly updated version of this. Mostly, it fixes an
> accidentally inverted comparison in down_heap, uses the squid x*
> memory allocation wrapper routines I found meanwhile and is more
> careful to avoid possibly invoking external code while the heap is not
> in a consistent state. It has still only been used on development
> computers.
Couple of things stand out...
* if tag is always a class ev_tag, why is it being cast to/from void*
all over the place? there is almost no need for void* to exist in C++.
Just pre-define class ev_tag; and use "ev_tag &" parameter types (as
const wherever possible).
* The code you are replacing uses the convention of return value type on
a separate line to the function name. Please follow that style.
* I know the code you are replacing uses C-style functions for each
action. But please make things like eventCancel() members of the event
object being cancelled. Particularly since you are already testing
whether that object exists before cancelling, there is no extra cost of
correcting the function scope.
* Also, can you get any performance numbers about the impact of the
change please?
... still reading the changes, so there may be more. But that seems big
enough for now.
Amos
Received on Wed Jan 30 2013 - 11:32:43 MST
This archive was generated by hypermail 2.2.0 : Wed Jan 30 2013 - 12:00:08 MST