tmedicci commented on code in PR #8985:
URL: https://github.com/apache/nuttx/pull/8985#discussion_r1165416199


##########
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'd prefer to make it on a separate patch. The `O_RDONLY` check is already 
existing 
(https://github.com/apache/nuttx/blob/74e032e9248e04de7faf0e1ed97f2eb2b34c1251/drivers/pipes/pipe_common.c#L197)
 and resides outside of the corresponding `if` block. I'll just add the 
`O_WRONLY` while similarly. Another patch can move them and create specific 
semaphores (https://github.com/apache/nuttx/pull/8985#discussion_r1164328463). 
Do you agree?



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

Reply via email to