pkarashchenko commented on code in PR #8743:
URL: https://github.com/apache/nuttx/pull/8743#discussion_r1197819706


##########
include/semaphore.h:
##########
@@ -113,7 +113,7 @@ struct sem_s
 # if CONFIG_SEM_PREALLOCHOLDERS > 0
   FAR struct semholder_s *hhead; /* List of holders of semaphore counts */
 # else
-  struct semholder_s holder[2];  /* Slot for old and new holder */
+  struct semholder_s holder;     /* Slot for old and new holder */

Review Comment:
   @davids5 the main idea here is in case of CONFIG_SEM_PREALLOCHOLDERS==0 not 
to wait until `sem_restorebaseprio()` call, but free holder immediately in 
`nxsem_release_holder()` so it can be directly reused. With assumption of 
`CONFIG_SEM_PREALLOCHOLDERS==0` the only task that requires priority 
restoration is the currently running task (the "old" holder).
   I need to re-examine carefully if that's gonna work, but seems to be 
reasonable.



##########
sched/semaphore/sem_holder.c:
##########
@@ -145,19 +141,13 @@ nxsem_findholder(FAR sem_t *sem, FAR struct tcb_s *htcb)
         }
     }
 #else
-  int i;
+  /* We have one hard-allocated holder structures in sem_t */
 
-  /* We have two hard-allocated holder structures in sem_t */
-
-  for (i = 0; i < 2; i++)
+  if (sem->holder.htcb == htcb)
     {
-      pholder = &sem->holder[i];
-      if (pholder->htcb == htcb)
-        {
-          /* Got it! */
+      /* Got it! */
 
-          return pholder;
-        }
+      return pholder;

Review Comment:
   returning a stack garbage is not good



-- 
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