[haiku-gsoc] Re: New timer patch (again) (Was: Re: First timer.diff (from private e-mails))

Ah, I'm glad you caught that one. Working on a new patch now :)

On Mon, Jun 30, 2008 at 7:48 AM, Stephan Assmus <superstippi@xxxxxx> wrote:
>
> Dustin Howett wrote:
>> New version with more fixes from Axel's suggestions, a lot of which can
>> be credited to Stefano.
>> I reworked the priority/init system.. prio is now -1 until initialized,
>> set by the module when it's initialized, and the timer loading code
>> treats -1 as non-initialized. This introduces another function into the
>> structure, but removes the initialized bool, so it can be const.
>
> The patch still has a few "if(", just search for them. Those should be "if
> (". But the coding style is a lot more conforming now.
>
> I spotted the following:
>
> +       for (i = 0; (timer = sTimers[i]) != NULL; i++) {
> +               timerPriority = timer->get_priority();
> +               if (timerPriority == -1) {
> +                       TRACE(("arch_init_timer: %s was not initialized. 
> Attempting.\n",
> timer->name));
> +                       if (timer->init(args, 1) != B_OK) {
> +                               TRACE(("arch_init_timer: %s would not 
> initialize. Skipping.\n",
> timer->name));
> +                               continue;
> +                       }
> +               }
>
> -       // apic timer interrupt set up by smp code
> +               if (timerPriority == -1) {
> +                       TRACE(("arch_init_timer: %s still not valid but 
> didn't fail init..\n",
> timer->name));
> +                       continue;
> +               }
>
> You do not reassign timerPriority after calling timer->init(). How can this
> work, unless I am missing something?
>
> Best regards,
> -Stephan
>
>
>
>



-- 

- DH

Other related posts: