On 30/01/2013 10:31 a.m., Rainer Weikusat wrote:
Rainer Weikusat <[email protected]> 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

Reply via email to