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 2020/04/05 17:12:30 UTC
[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #734: Check
return from nxsem_wait_uninterruptible()
Ouss4 opened a new pull request #734: Check return from nxsem_wait_uninterruptible()
URL: https://github.com/apache/incubator-nuttx/pull/734
This commits is for the remaining files in arch/arm/src/cxd56xx
arch/arm/src/imxrt and arch/arm/src/stm32l4
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request
#734: Check return from nxsem_wait_uninterruptible()
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #734: Check return from nxsem_wait_uninterruptible()
URL: https://github.com/apache/incubator-nuttx/pull/734#discussion_r403731636
##########
File path: arch/arm/src/cxd56xx/cxd56_emmc.c
##########
@@ -440,7 +440,17 @@ static void emmc_send(int datatype, uint32_t opcode, uint32_t arg,
/* Wait for command or data transfer done */
- emmc_takesem(&g_waitsem);
+ do
+ {
+ ret = emmc_takesem(&g_waitsem);
+
+ /* The only expected error is ECANCELED which would occur if the
+ * calling thread were canceled.
+ */
+
+ DEBUGASSERT(ret == OK || ret == -ECANCELED);
+ }
+ while (ret < 0);
Review comment:
This will lose the cancelation error. The function will return OK and the cancelation will not occur. I recommend you consider return ret at the bottom of the file instead of OK.
Perhaps that ret value is destroyed by later logic??? I can't see that here. If that is the case then you should save any error returned by emmc_taksem() in some other variable and return that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] Ouss4 commented on issue #734: Check return from
nxsem_wait_uninterruptible()
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on issue #734: Check return from nxsem_wait_uninterruptible()
URL: https://github.com/apache/incubator-nuttx/pull/734#issuecomment-609449959
The style issues are the same as described #652
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request
#734: Check return from nxsem_wait_uninterruptible()
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #734: Check return from nxsem_wait_uninterruptible()
URL: https://github.com/apache/incubator-nuttx/pull/734#discussion_r403731226
##########
File path: arch/arm/src/cxd56xx/cxd56_sysctl.c
##########
@@ -127,13 +164,24 @@ int cxd56_sysctlcmd(uint8_t id, uint32_t data)
ret = cxd56_iccsend(CXD56_PROTO_SYSCTL, &msg, SYSCTL_TIMEOUT);
if (ret < 0)
{
+ sysctl_semgive(&g_exc);
_err("Timeout.\n");
return ret;
}
/* Wait for reply message from system CPU */
- sysctl_semtake(&g_sync);
+ do
+ {
+ ret = sysctl_semtake(&g_sync);
+
+ /* The only expected error is ECANCELED which would occur if the
+ * calling thread were canceled.
+ */
+
+ DEBUGASSERT(ret == OK || ret == -ECANCELED);
+ }
+ while (ret < 0);
Review comment:
It there any reason for this loop here? This loop basically ignores cancellation requests (and loses the cancellation status permanently). Why can't you just return following the same logic as at lines 167-169.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo merged pull request #734: Check return
from nxsem_wait_uninterruptible()
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #734: Check return from nxsem_wait_uninterruptible()
URL: https://github.com/apache/incubator-nuttx/pull/734
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services