Thanks a lot for your review. ----- Gedare Bloom <ged...@rtems.org> schrieb: > I scanned through most of it. I just have a few minor issues/questions: > > There is an _Assert(0) followed by a TODO. This _Assert() is likely > dead/untested code block. It may be worth fabricating a fatal test for > it, if possible?
This one: > + if ( pthread_kill ( ptimer->thread_id, ptimer->inf.sigev_signo ) ) { > + _Assert( FALSE ); > + /* > + * TODO: What if an error happens at run-time? This should never > + * occur because the timer should be canceled if the thread > + * is deleted. This method is being invoked from the Clock > + * Tick ISR so even if we decide to take action on an error, > + * we don't have many options. We shouldn't shut the system > down. > + */ Its a code move from one file to another, so nothing new. > > "be placed on Red-Black Trees for set management." copy-pasted comment > should be Chains? Thanks, for spotting this. > > "watchdog is scheduled and a black node". ditto, black should be red > for the second one. Oh, yes. > > _Watchdog_Ticks_from_seconds(): why is ticks = seconds<<30 the right > thing to do? Same for _Watchdog_Ticks_from_timespec(). I am missing > some assumption here, I guess. It might improve readability to provide > a helper function for this. Ok, sorry for the magic numbers. 2**30 == 1073741824 enough to cope with 1e9 nanoseconds. So, we have 2**34 seconds available, leading to a year 2514 problem. _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel