Re: event engine patch update

[Jan-Henrik Haukeland]

Martin Pala <address@hidden> writes:

> Jan-Henrik Haukeland wrote:

>> 1) Have you run monit with this patch through valgrind? To me it
>> looks like events are added to the list but not dequeued and that
>> there is a massive memory leak here. Event_free() is not called at
>> all in the code.
>
> The event queue is per service - each service has its own related
> events list.
...

Okay, I got it, the event list is filled with event objects for
possible events on the service, but objects in the list are reused
when the same event occurs again. In other words there is no memory
leak. (Maybe you can remove Event_free() then or preferably reuse this
in gc.c)

Anyway thanks for the detailed explanation and the helpful
drawing. When you have time you should think about putting this up on
monit's web-site in an article named "internal workings of monit" or
something like that :-)

>> 3) Please use the "one-true bracket style" in the code i.e.
>> if(x) {
>>              blabla
>>          }
>>    and also break lines at 79 column width. Apropos I found out that
>>    it is possible to use 'C-c .' to set the C indentation style (to
>>    gnu) in (X)emacs, which is the style we use.
>
> OK. I used in event.c style which i preffer, i hope it will not
> interfer too much.

It wont interferer if you change it ;-) Sorry for nitpicking but we
should keep with the gnu coding style when we can.

> The new data structure encapsulates the objects and is based on
> objects relationships and inharitence/sharing. I hope above text and
> picture will help.

Absolutely. A last control question, did you try the patch with
valgrind? For example, in event.c:Event_post() it looks like you add a
new message to an event without freeing the previous one.

When the patch runs cleanly with valgrind I'm +2 for checkin it into
the CVS.

-- 
Jan-Henrik Haukeland