tmedicci commented on code in PR #8985:
URL: https://github.com/apache/nuttx/pull/8985#discussion_r1164010390
##########
drivers/pipes/pipe_common.c:
##########
@@ -171,56 +173,109 @@ 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) == OK && 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 */
Review Comment:
I'm afraid that the logic wouldn't be the same if we move this `while` block
into `if` block at 172 + removing the `O_WRONLY` check:
`if ((filep->f_oflags & O_WROK) != 0)` only checks if the file is open for
writing (it could be open for writing and reading and this check would still be
valid). But the logic for blocking open, according to IEEE Std 1003.1-2017, is
valid only when is being opened for writing/reading-only (`(filep->f_oflags &
O_RDWR) == O_WRONLY` assures that). An intermediary solution would be moving
but letting the `O_WRONLY` check.
Am I missing something?
--
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
