pkarashchenko commented on code in PR #8985:
URL: https://github.com/apache/nuttx/pull/8985#discussion_r1161714048
##########
drivers/pipes/pipe.c:
##########
@@ -243,22 +264,48 @@ int pipe2(int fd[2], int flags)
return ERROR;
}
- /* Get a write file descriptor */
+ /* Check for the O_NONBLOCK bit on flags */
+
+ blocking = flags & O_NONBLOCK ? false : true;
Review Comment:
```suggestion
blocking = !(flags & O_NONBLOCK);
```
or
```suggestion
blocking = (flags & O_NONBLOCK) == 0;
```
##########
drivers/pipes/pipe_common.c:
##########
@@ -171,50 +171,81 @@ int pipecommon_open(FAR struct file *filep)
{
dev->d_nwriters++;
- /* If this is the first writer, then the read semaphore indicates the
- * number of readers waiting for the first writer. Wake them all up.
+ /* If this is the first writer, then the n-writers semaphore
+ * indicates the number of readers waiting for the first writer.
+ * Wake them all up!
*/
if (dev->d_nwriters == 1)
{
- while (nxsem_get_value(&dev->d_rdsem, &sval) == 0 && sval <= 0)
+ while (nxsem_get_value(&dev->d_nwrsem, &sval) == 0 && sval <= 0)
{
- nxsem_post(&dev->d_rdsem);
+ nxsem_post(&dev->d_nwrsem);
}
}
}
+ while ((filep->f_oflags & O_NONBLOCK) == 0 && /* Blocking */
+ (filep->f_oflags & O_RDWR) == O_WRONLY && /* Write-only */
+ dev->d_nreaders < 1 && /* No readers on the pipe
*/
+ dev->d_wrndx == dev->d_rdndx) /* Buffer is empty */
+ {
+ /* If opened for write-only, then wait for at least one reader
+ * on the pipe.
+ */
+
+ nxmutex_unlock(&dev->d_bflock);
+
+ ret = nxsem_wait(&dev->d_nrdsem);
+ if (ret < 0 || (ret = nxmutex_lock(&dev->d_bflock)) < 0)
+ {
+ /* The nxmutex_lock() call should fail if we are awakened by a
+ * signal or if the task is canceled.
+ */
+
+ ferr("ERROR: nxmutex_lock failed: %d\n", ret);
Review Comment:
it is not clear is return code printed returned from `nxsem_wait` or from
`nxmutex_lock`.
I know that is is copied from another place
##########
drivers/pipes/pipe_common.h:
##########
@@ -117,6 +117,8 @@ struct pipe_dev_s
mutex_t d_bflock; /* Used to serialize access to d_buffer and
indices */
sem_t d_rdsem; /* Empty buffer - Reader waits for data write */
sem_t d_wrsem; /* Full buffer - Writer waits for data read */
+ sem_t d_nrdsem; /* Waits for readers */
+ sem_t d_nwrsem; /* Waits for writers */
Review Comment:
Where those semaphores are initialized?
##########
drivers/pipes/pipe_common.c:
##########
@@ -171,50 +171,81 @@ int pipecommon_open(FAR struct file *filep)
{
dev->d_nwriters++;
- /* If this is the first writer, then the read semaphore indicates the
- * number of readers waiting for the first writer. Wake them all up.
+ /* If this is the first writer, then the n-writers semaphore
+ * indicates the number of readers waiting for the first writer.
+ * Wake them all up!
*/
if (dev->d_nwriters == 1)
{
- while (nxsem_get_value(&dev->d_rdsem, &sval) == 0 && sval <= 0)
+ while (nxsem_get_value(&dev->d_nwrsem, &sval) == 0 && sval <= 0)
{
- nxsem_post(&dev->d_rdsem);
+ nxsem_post(&dev->d_nwrsem);
}
}
}
+ while ((filep->f_oflags & O_NONBLOCK) == 0 && /* Blocking */
+ (filep->f_oflags & O_RDWR) == O_WRONLY && /* Write-only */
+ dev->d_nreaders < 1 && /* No readers on the pipe
*/
+ dev->d_wrndx == dev->d_rdndx) /* Buffer is empty */
+ {
+ /* If opened for write-only, then wait for at least one reader
+ * on the pipe.
+ */
+
+ nxmutex_unlock(&dev->d_bflock);
+
+ ret = nxsem_wait(&dev->d_nrdsem);
+ if (ret < 0 || (ret = nxmutex_lock(&dev->d_bflock)) < 0)
+ {
+ /* The nxmutex_lock() call should fail if we are awakened by a
+ * signal or if the task is canceled.
+ */
+
+ ferr("ERROR: nxmutex_lock failed: %d\n", ret);
+
+ /* Immediately close the pipe that we just opened */
+
+ pipecommon_close(filep);
+ return ret;
+ }
+ }
+
/* If opened for reading, increment the count of reader on on the pipe
* instance.
*/
if ((filep->f_oflags & O_RDOK) != 0)
{
dev->d_nreaders++;
+
+ /* If this is the first reader, then the n-readers semaphore
+ * indicates the number of writers waiting for the first reader.
+ * Wake them all up.
+ */
+
+ if (dev->d_nreaders == 1)
+ {
+ while (nxsem_get_value(&dev->d_nrdsem, &sval) == 0 && sval <= 0)
Review Comment:
```suggestion
while (nxsem_get_value(&dev->d_nrdsem, &sval) == OK && sval <= 0)
```
--
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
