On 15/11/2019 15:57, Joel Sherrill wrote:


On Thu, Nov 14, 2019 at 11:59 PM Sebastian Huber <sebastian.hu...@embedded-brains.de <mailto:sebastian.hu...@embedded-brains.de>> wrote:



    On 14/11/2019 21:44, Joel Sherrill wrote:
     > +bool _TOD_Hook_Register(
     > +  TOD_Hook *hook
     > +)
     > +{
     > +  ISR_lock_Context lock_context;
     > +
     > +  /*
     > +   * At this time, this method does NOT have a Classic or POSIX API
     > +   * that exports it. Any use of this method will be a direct call.
     > +   * It should only be called while NOT holding the TOD lock.
     > +   */
     > +  _Assert( !_TOD_Is_owner() );
     > +
     > +  if ( hook == NULL ) {
     > +    return FALSE;
     > +  }
     > +
     > +  _TOD_Lock();
     > +  _TOD_Acquire( &lock_context );
     > +  _Chain_Append_unprotected( &_TOD_Hooks, &hook->Node );
     > +  _TOD_Unlock();
     > +  return true;
     > +}

    Where is the _TOD_Release()? This function should not test for a NULL
    hook and return void. Such kind of error handling should be done by
    upper layer functions and not at the lowest level.


There is no method _TOD_Release in the source tree. I followed the locking
pattern in clockset.c.

     _TOD_Lock();
     _TOD_Acquire( &lock_context );
     retval = _TOD_Set( &tod_as_timespec, &lock_context );
     _TOD_Unlock();

In this case you need the _TOD_Acquire() since _TOD_Set() takes care of the release and expects an acquired TOD lock and a valid lock context. Please remove all the unnecessary _TOD_Acquire() since they break the SMP support.


There is no upper level. Per our earlier discussions, there is not an API above the score on this for register and unregister. So the error checking is needed here. If there
were a layer about this, the locking and error checking would be there.

Testing in _TOD_Hook_Register() that that the hook pointer is non-NULL is completely superfluous from my point of view. Users of underscore functions are supposed to know what they do. If you call this function, you want to register a hook, don't you? If you feel safer, then you can add an _Assert( hook != NULL ).

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Reply via email to