You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/09/07 12:22:56 UTC

[GitHub] [incubator-nuttx] anchao opened a new pull request, #7032: drivers/ipcc: enter/leave critical section should in pairs

anchao opened a new pull request, #7032:
URL: https://github.com/apache/incubator-nuttx/pull/7032

   ## Summary
   
   drivers/ipcc: enter/leave critical section should in pairs
   
   Signed-off-by: chao an <an...@xiaomi.com>
   
   ## Impact
   
   N/A
   
   ## Testing
   
   N/A


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7032: drivers/ipcc: enter/leave critical section should in pairs

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7032:
URL: https://github.com/apache/incubator-nuttx/pull/7032#discussion_r965493310


##########
drivers/ipcc/ipcc_write.c:
##########
@@ -239,15 +239,20 @@ ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
        */
 
       nxsem_post(&priv->exclsem);
+
       if ((ret = nxsem_wait(&priv->txsem)))
         {
+          leave_critical_section(flags);
+
           /* We were interrupted by signal, return error or number
            * of bytes written
            */
 
           return nwritten ? nwritten : -EINTR;
         }
 
+      leave_critical_section(flags);

Review Comment:
   Done 



##########
drivers/ipcc/ipcc_read.c:
##########
@@ -230,20 +230,22 @@ ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer,
         }
 
       /* We are in blocking mode, so we have to wait for data to arrive.
-       * nxsem_wait() will atomically leave critical section for us so
-       * we don't have to do it.
        */
 
       nxsem_post(&priv->exclsem);
       if ((ret = nxsem_wait(&priv->rxsem)))
         {
+          leave_critical_section(flags);
+
           /* We were interrupted by signal, but we have not written
            * any data to caller's buffer, so we return with error
            */
 
           return ret;
         }
 
+      leave_critical_section(flags);

Review Comment:
   done



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7032: drivers/ipcc: enter/leave critical section should in pairs

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7032:
URL: https://github.com/apache/incubator-nuttx/pull/7032#discussion_r965181956


##########
drivers/ipcc/ipcc_write.c:
##########
@@ -239,15 +239,20 @@ ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
        */
 
       nxsem_post(&priv->exclsem);
+
       if ((ret = nxsem_wait(&priv->txsem)))
         {
+          leave_critical_section(flags);
+
           /* We were interrupted by signal, return error or number
            * of bytes written
            */
 
           return nwritten ? nwritten : -EINTR;
         }
 
+      leave_critical_section(flags);

Review Comment:
   Maybe just place `flags = enter_critical_section();` before `for (nwritten = 0; ; )` instead of adding `leave_critical_section(flags);` here?



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #7032: drivers/ipcc: enter/leave critical section should in pairs

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7032:
URL: https://github.com/apache/incubator-nuttx/pull/7032


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7032: drivers/ipcc: enter/leave critical section should in pairs

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7032:
URL: https://github.com/apache/incubator-nuttx/pull/7032#discussion_r965183110


##########
drivers/ipcc/ipcc_read.c:
##########
@@ -230,20 +230,22 @@ ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer,
         }
 
       /* We are in blocking mode, so we have to wait for data to arrive.
-       * nxsem_wait() will atomically leave critical section for us so
-       * we don't have to do it.
        */
 
       nxsem_post(&priv->exclsem);
       if ((ret = nxsem_wait(&priv->rxsem)))
         {
+          leave_critical_section(flags);
+
           /* We were interrupted by signal, but we have not written
            * any data to caller's buffer, so we return with error
            */
 
           return ret;
         }
 
+      leave_critical_section(flags);

Review Comment:
   ditto



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #7032: drivers/ipcc: enter/leave critical section should in pairs

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #7032:
URL: https://github.com/apache/incubator-nuttx/pull/7032#discussion_r964820463


##########
drivers/ipcc/ipcc_read.c:
##########
@@ -234,6 +234,7 @@ ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer,
        * we don't have to do it.
        */
 

Review Comment:
   If `leave_critical_section()` is added here, then comment above is stale.



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7032: drivers/ipcc: enter/leave critical section should in pairs

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7032:
URL: https://github.com/apache/incubator-nuttx/pull/7032#discussion_r964832339


##########
drivers/ipcc/ipcc_read.c:
##########
@@ -234,6 +234,7 @@ ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer,
        * we don't have to do it.
        */
 

Review Comment:
   Thank you, I will remove the commit



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org