pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r869599520


##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,18 +145,22 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, 
uint32_t delay)
  *
  ****************************************************************************/
 
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
-                                   uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
 {
+  clock_t start = clock_systime_ticks();
   int ret;
 
   do
     {
       /* Take the semaphore (perhaps waiting) */
 
-      ret = nxsem_tickwait(sem, start, delay);
+      ret = nxsem_tickwait(sem, delay);

Review Comment:
   this code is bad. `delay` is not adjusted in case if `nxsem_tickwait` was 
interrupted, so for example in case of `nxsem_tickwait_uninterruptible(sem, 
10);` the `nxsem_tickwait` gets interrupted at 9th tick, the 
`clock_systime_ticks() - start` == `9` and `9 < 10` so `nxsem_tickwait(sem, 
10);` will be called the second time and will lead to `19` ticks delay instead 
of `10`.
   The existing code was exactly handing such cases correctly.



##########
arch/risc-v/src/bl602/bl602_os_hal.c:
##########
@@ -1481,12 +1481,7 @@ int32_t bl_os_sem_take(void *semphr, uint32_t ticks)
           return false;
         }
 
-      if (ticks)
-        {
-          bl_os_update_time(&timeout, ticks);

Review Comment:
   why this call is removed?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to