Re: [Lcd4linux-devel] [lcd4linux-devel] Re: Twice eval. patch

[Mattia Jona-L. <mat...@gm...>]

Hi everybody,

I've been following this discussion about timers and I mostly agree
with Martin when it comes to the implementation of timers in
lcd4linux. As far as I can see there are two major drawbacks in the
present implementation. First, all timers are stored in a malloc'ed
vector that is reallocated every time a new slot is required. This is
absolutely sub-optimal even when handling a handful of timers. The
memory where slots are allocated is forced to be contiguous and there
is no need for that. When no contiguous memory is available, realloc
has to copy all pointers from the old location to the new one. Not to
mention the fact that timers are added in a random order, filling the
first available slot. This strategy requires the extra loop over all
timers to find the next timer to be processed. The second drawback is
that processed and unprocessed timers are kept in the same vector,
triggering the bug  reported by Marcus.

What I propose is to switch to a linked list for timers and to create
two separate linked lists, one for processed timers and one for
unprocessed ones. The timers should be kept time ordered in the list
and this will make the aforementioned extra loop not required. The
processing routine will go through the active list and will process
all timers one by one, unlinking them from the active list as soon as
they are processed and linking them to the inactive list. Once the
active list is empty, all timers will have been processed and are in
the inactive list. Then it is sufficient to swap the active and
inactive list and start again.

In the meantime I attach a patch to fix the realloc error handling bug.

Please feel free to comment on both! :)

Mattia


On Sun, Mar 25, 2012 at 2:05 PM, Michael Reinelt <mi...@re...> wrote:
> Hi Marcus, Martin,
>
> I try to think about the other stuff ASAP, but....
>
>> I don't understand the realloc error handling in 'timer_add_widget'
>> (similar in 'timer_add').In the case of a realloc error 'TimerGroupWidgets'
>> was set to NULL. So there's no valid pointer to the existing
>> TimerGroupWidgets after that. Won't that cause a memory leak and
>> following errors cause nTimerGroupWidgets is only decreased by 1
>> instead of set it to 0? Shouldn't a additional pointer be used for
>> the realloc return value to avoid this?
>
> You are *that* right. This is junk. junk junk junk.
>
> OTOH I think this will never happen in real life. When such a small allocation fails, I'm pretty sure that you will have
> other problems :-)
>
> anyway, this should be cleaned up.
>
>
>
> sunny greetings, Michael
>
>
> --
> Michael Reinelt <mi...@re...>
> http://home.pages.at/reinelt
> GPG-Key 0xDF13BA50
> ICQ #288386781
>
> ------------------------------------------------------------------------------
> This SF email is sponsosred by:
> Try Windows Azure free for 90 days Click Here
> http://p.sf.net/sfu/sfd2d-msazure
> _______________________________________________
> Lcd4linux-devel mailing list
> Lcd...@li...
> https://lists.sourceforge.net/lists/listinfo/lcd4linux-devel