Re: cyg_alarm - suggestion for enhancement

[NavEcos <ecos at navosha dot com>]

Sorry I took so long to get this in, frankly I forgot about it.  Here is
the newest alarm modifications.

CVS log message should be something like:

"Added the ability to set the alarm callback function as well as
retrieve the current callback function to make alarms more compatible
with vxWorks' wdXXX functions."

The C API has had the following functions added:

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     */
);

externC void cyg_alarm_get_callback(
     cyg_handle_t       alarm,
     cyg_alarm_t        **alarmfn,/* alarm callback function     */
     cyg_addrword_t     *data     /* alarm callback data         */
);

Diffs are attached.

Thanks,
-Rich

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.
> 
> -- 
> Nick Garnett - eCos Kernel Architect
> http://www.eCosCentric.com/
>

Attachment: diff.txt
Description: Text document