xiaoxiang781216 commented on a change in pull request #5017:
URL: https://github.com/apache/incubator-nuttx/pull/5017#discussion_r773565811
##########
File path: drivers/serial/serial.c
##########
@@ -1332,12 +1333,28 @@ static int uart_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
}
break;
-#ifdef CONFIG_SERIAL_TERMIOS
case TCFLSH:
{
/* Empty the tx/rx buffers */
- irqstate_t flags = enter_critical_section();
+ irqstate_t flags;
+
+ /* tcdrain is a cancellation point */
+
+ if (enter_cancellation_point())
Review comment:
> According to the latest version of the Open Group standard, `tcdrain`
is required to be a thread cancellation point:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_09_05_02
>
> So, strictly speaking, the most correct would be to set the cancellation
point inside `tcdrain()`, as suggested by @Ouss4
tcdrain still contain the cancellation pointer indirectly through
ioctl::TCFLSH.
There is no real behavior difference before and after this patch:
The thread cancellation point check still has to be kept inside the kernel
space since both enter_cancellation_point/leave_cancellation_point can't be
called from user space.
The real difference is that syscall entry point switch from tcdrain to
ioctl, but it's just a implementation detail.
>. Even so, it wouldn't compromise the points raised by @xiaoxiang781216,
since it would still continue to be a userspace wrapper around the `TCDRN`
ioctl call.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
