This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: cyg_alarm - suggestion for enhancement


NavEcos <ecos@navosha.com> writes:

> *cough*.  My that was incredibly stupid of me wasn't it?  I decided to
> retrieve the values of the alarm callback function and data that
> accompanies it as 2 separate functions to avoid pointers.  If you would
> rather have it as a single call, let me know and I'll fix it.
> 
> Here are the prototypes now:
> 
> void cyg_alarm_set_callback(
>      cyg_handle_t       alarm,
>      cyg_alarm_t        *alarmfn, /* new alarm callback function  */
>      cyg_addrword_t     data      /* new alarm callback data      */
> );
> 
> cyg_alarm_t *cyg_alarm_get_callback( cyg_handle_t alarm );
> 
> cyg_addrword_t cyg_alarm_get_data( cyg_handle_t alarm );
> 
> 

That's better. Personally I would have used a single get function to
emphasize the "togetherness" of the two values. But it is not a big
enough point to argue about.

However, one thing that did occur to me is that the set function
should be protected by taking the scheduler lock. If an interrupt
occurs between setting the alarm function and setting the data, we
could end up calling the function with the wrong data.

So the set_callback function should look like this:

inline void Cyg_Alarm::set_callback (
        cyg_alarm_fn  *alarmfn,
        CYG_ADDRWORD  newdata
        )
{
    Cyg_Scheduler::lock();
    alarm = alarmfn;
    data = newdata;
    Cyg_Scheduler::unlock();
}

I think with that change I would be happy to approve it.

-- 
Nick Garnett - eCos Kernel Architect
http://www.eCosCentric.com/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]