This is the mail archive of the 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

On Fri, 2002-08-30 at 10:51, Nick Garnett wrote:
> NavEcos <> 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

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.


> -- 
> Nick Garnett - eCos Kernel Architect

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