This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: cyg_alarm - suggestion for enhancement
On Fri, 2002-08-30 at 10:51, Nick Garnett wrote:
> 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.
I see your point here but I did it that way since I (maybe incorrectly)
assumed that the alarm would be in an inactive state if you are changing
the callback function. If the retrigger value is non 0 you have a race
condition because if you change the CB while it's running, you cannot
know beforehand which CB will get called since you can be pre-empted at
anytime.
How about this: I prevent the callback from being changed unless the
alarm's retrigger time is set to 0 or the alarm is disabled? After all:
my justification for this is to make it compatible with the wd functions
of vxWorks, which doesn't have a retrigger value. Still, there is the
possibility that a second thread could enable the alarm while the alarm
is being modified, but if a coder is doing that, they are asking for
trouble anyhow.
I will combine the "get" routines, I don't have religious connections to
it since I abstract the OS always in my projects with inline functions
and macros.
-Rich
> --
> Nick Garnett - eCos Kernel Architect
> http://www.eCosCentric.com/
>