<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 14, 2019 at 11:59 PM Sebastian Huber <<a href="mailto:sebastian.huber@embedded-brains.de">sebastian.huber@embedded-brains.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On 14/11/2019 21:44, Joel Sherrill wrote:<br>
> +bool _TOD_Hook_Register(<br>
> +  TOD_Hook *hook<br>
> +)<br>
> +{<br>
> +  ISR_lock_Context lock_context;<br>
> +<br>
> +  /*<br>
> +   * At this time, this method does NOT have a Classic or POSIX API<br>
> +   * that exports it. Any use of this method will be a direct call.<br>
> +   * It should only be called while NOT holding the TOD lock.<br>
> +   */<br>
> +  _Assert( !_TOD_Is_owner() );<br>
> +<br>
> +  if ( hook == NULL ) {<br>
> +    return FALSE;<br>
> +  }<br>
> +<br>
> +  _TOD_Lock();<br>
> +  _TOD_Acquire( &lock_context );<br>
> +  _Chain_Append_unprotected( &_TOD_Hooks, &hook->Node );<br>
> +  _TOD_Unlock();<br>
> +  return true;<br>
> +}<br>
<br>
Where is the _TOD_Release()? This function should not test for a NULL <br>
hook and return void. Such kind of error handling should be done by <br>
upper layer functions and not at the lowest level.<br></blockquote><div><br></div><div>There is no method _TOD_Release in the source tree. I followed the locking </div><div>pattern in clockset.c.</div><div><br></div><div>    _TOD_Lock();<br>    _TOD_Acquire( &lock_context );<br>    retval = _TOD_Set( &tod_as_timespec, &lock_context );<br>    _TOD_Unlock();<br><br></div><div>There is no upper level. Per our earlier discussions, there is not an API above the</div><div>score on this for register and unregister. So the error checking is needed here. If there </div><div>were a layer about this, the locking and error checking would be there.</div><div><br></div><div>And for Chris.. FALSE/false is a habit from the pre-C99 days. Thanks for catching that.</div><div><br></div><div>Also for Chris _Assert() is only active when debug is enabled.</div><div><br></div><div>--joel</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
-- <br>
Sebastian Huber, embedded brains GmbH<br>
<br>
Address : Dornierstr. 4, D-82178 Puchheim, Germany<br>
Phone   : +49 89 189 47 41-16<br>
Fax     : +49 89 189 47 41-09<br>
E-Mail  : <a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a><br>
PGP     : Public key available on request.<br>
<br>
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.<br>
</blockquote></div></div>