pkarashchenko commented on code in PR #6435:
URL: https://github.com/apache/incubator-nuttx/pull/6435#discussion_r897512045
##########
drivers/syslog/syslog_device.c:
##########
@@ -169,15 +150,7 @@ static inline int syslog_dev_takesem(FAR struct
syslog_dev_s *syslog_dev)
static inline void syslog_dev_givesem(FAR struct syslog_dev_s *syslog_dev)
{
-#ifdef CONFIG_DEBUG_ASSERTIONS
- pid_t me = getpid();
- DEBUGASSERT(syslog_dev->sl_holder == me);
Review Comment:
Do we need to keep debug assertion here?
##########
drivers/syslog/syslog_device.c:
##########
@@ -83,8 +79,7 @@ struct syslog_dev_s
uint8_t sl_state; /* See enum syslog_dev_state */
uint8_t sl_oflags; /* Saved open mode (for re-open) */
uint16_t sl_mode; /* Saved open flags (for re-open) */
- sem_t sl_sem; /* Enforces mutually exclusive access */
- pid_t sl_holder; /* PID of the thread that holds the semaphore */
+ rmutex_t sl_lock; /* Enforces mutually exclusive access */
Review Comment:
I understand your intention to get the changes back asap, but `sl_lock` is
not a recursive mutex by it's nature. It is a regular mutex + holder checking
for error. We can reuse recursive mutex in this place, but for me it is an odd
design decision.
--
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: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
