xiaoxiang781216 commented on code in PR #21144:
URL: https://github.com/apache/nuttx/pull/21144#discussion_r4431046474
##########
drivers/serial/serial_io.c:
##########
@@ -286,6 +301,12 @@ void uart_recvchars(FAR uart_dev_t *dev)
pbuf = &ch;
nbytes = 1;
+#if defined(CONFIG_TTY_SIGINT) || defined(CONFIG_TTY_SIGTSTP) || \
+ defined(CONFIG_TTY_FORCE_PANIC) || defined(CONFIG_TTY_LAUNCH)
+ sig = uart_check_special(dev, pbuf, nbytes, &nkept);
Review Comment:
why not pass `&nbytes` and remove line 271
##########
drivers/serial/serial_dma.c:
##########
@@ -65,20 +65,20 @@ static int uart_recvchars_check_special(FAR uart_dev_t *dev)
if (xfer->nbytes <= xfer->length)
{
- return uart_check_special(dev, xfer->buffer, xfer->nbytes);
+ return uart_check_special(dev, xfer->buffer, xfer->nbytes, NULL);
Review Comment:
should we adjust nbytes to remove the signal char
##########
drivers/serial/serial.c:
##########
@@ -2554,72 +2554,112 @@ void uart_reset_sem(FAR uart_dev_t *dev)
* Name: uart_check_special
*
* Description:
- * Check if the SIGINT or SIGTSTP character is in the contiguous Rx DMA
- * buffer region. The first signal associated with the first such
- * character is returned.
- *
- * If there multiple such characters in the buffer, only the signal
- * associated with the first is returned (this a bug!)
+ * Act on the first special character in the size characters at buf.
+ * If nkept is not NULL, also remove every INTR and SUSP character that
+ * sends a signal (POSIX: the terminal discards it) and return in *nkept
+ * the number of characters that are left at buf.
*
* Returned Value:
- * 0 if a signal-related character does not appear in the. Otherwise,
- * SIGKILL or SIGTSTP may be returned to indicate the appropriate signal
- * action.
+ * SIGINT or SIGTSTP if the first special character sends it, else 0.
*
****************************************************************************/
#if defined(CONFIG_TTY_SIGINT) || defined(CONFIG_TTY_SIGTSTP) || \
defined(CONFIG_TTY_FORCE_PANIC) || defined(CONFIG_TTY_LAUNCH)
-int uart_check_special(FAR uart_dev_t *dev, FAR const char *buf, size_t size)
+int uart_check_special(FAR uart_dev_t *dev, FAR char *buf, size_t size,
+ FAR size_t *nkept)
{
+ bool check = true;
+ size_t kept = 0;
+ int signo = 0;
size_t i;
if ((dev->tc_lflag & ISIG) == 0)
{
+ if (nkept != NULL)
+ {
+ *nkept = size;
+ }
+
return 0;
}
for (i = 0; i < size; i++)
{
+ bool drop = false;
+ char ch = buf[i];
+
#ifdef CONFIG_TTY_FORCE_PANIC
- if (buf[i] == CONFIG_TTY_FORCE_PANIC_CHAR)
+ if (check)
Review Comment:
why need check flag?
##########
drivers/serial/serial.c:
##########
@@ -2554,72 +2554,112 @@ void uart_reset_sem(FAR uart_dev_t *dev)
* Name: uart_check_special
*
* Description:
- * Check if the SIGINT or SIGTSTP character is in the contiguous Rx DMA
- * buffer region. The first signal associated with the first such
- * character is returned.
- *
- * If there multiple such characters in the buffer, only the signal
- * associated with the first is returned (this a bug!)
+ * Act on the first special character in the size characters at buf.
+ * If nkept is not NULL, also remove every INTR and SUSP character that
+ * sends a signal (POSIX: the terminal discards it) and return in *nkept
+ * the number of characters that are left at buf.
*
* Returned Value:
- * 0 if a signal-related character does not appear in the. Otherwise,
- * SIGKILL or SIGTSTP may be returned to indicate the appropriate signal
- * action.
+ * SIGINT or SIGTSTP if the first special character sends it, else 0.
*
****************************************************************************/
#if defined(CONFIG_TTY_SIGINT) || defined(CONFIG_TTY_SIGTSTP) || \
defined(CONFIG_TTY_FORCE_PANIC) || defined(CONFIG_TTY_LAUNCH)
-int uart_check_special(FAR uart_dev_t *dev, FAR const char *buf, size_t size)
+int uart_check_special(FAR uart_dev_t *dev, FAR char *buf, size_t size,
+ FAR size_t *nkept)
{
+ bool check = true;
+ size_t kept = 0;
+ int signo = 0;
size_t i;
if ((dev->tc_lflag & ISIG) == 0)
{
+ if (nkept != NULL)
+ {
+ *nkept = size;
+ }
+
return 0;
}
for (i = 0; i < size; i++)
{
+ bool drop = false;
+ char ch = buf[i];
+
#ifdef CONFIG_TTY_FORCE_PANIC
- if (buf[i] == CONFIG_TTY_FORCE_PANIC_CHAR)
+ if (check)
{
- if (++dev->panic_count >= CONFIG_TTY_FORCE_PANIC_REPEAT_COUNT)
+ if (ch == CONFIG_TTY_FORCE_PANIC_CHAR)
Review Comment:
should we eat CONFIG_TTY_FORCE_PANIC_CHAR
##########
drivers/serial/serial.c:
##########
@@ -2554,72 +2554,112 @@ void uart_reset_sem(FAR uart_dev_t *dev)
* Name: uart_check_special
*
* Description:
- * Check if the SIGINT or SIGTSTP character is in the contiguous Rx DMA
- * buffer region. The first signal associated with the first such
- * character is returned.
- *
- * If there multiple such characters in the buffer, only the signal
- * associated with the first is returned (this a bug!)
+ * Act on the first special character in the size characters at buf.
+ * If nkept is not NULL, also remove every INTR and SUSP character that
+ * sends a signal (POSIX: the terminal discards it) and return in *nkept
+ * the number of characters that are left at buf.
*
* Returned Value:
- * 0 if a signal-related character does not appear in the. Otherwise,
- * SIGKILL or SIGTSTP may be returned to indicate the appropriate signal
- * action.
+ * SIGINT or SIGTSTP if the first special character sends it, else 0.
*
****************************************************************************/
#if defined(CONFIG_TTY_SIGINT) || defined(CONFIG_TTY_SIGTSTP) || \
defined(CONFIG_TTY_FORCE_PANIC) || defined(CONFIG_TTY_LAUNCH)
-int uart_check_special(FAR uart_dev_t *dev, FAR const char *buf, size_t size)
+int uart_check_special(FAR uart_dev_t *dev, FAR char *buf, size_t size,
+ FAR size_t *nkept)
{
+ bool check = true;
+ size_t kept = 0;
+ int signo = 0;
size_t i;
if ((dev->tc_lflag & ISIG) == 0)
{
+ if (nkept != NULL)
+ {
+ *nkept = size;
+ }
+
return 0;
}
for (i = 0; i < size; i++)
{
+ bool drop = false;
+ char ch = buf[i];
+
#ifdef CONFIG_TTY_FORCE_PANIC
- if (buf[i] == CONFIG_TTY_FORCE_PANIC_CHAR)
+ if (check)
{
- if (++dev->panic_count >= CONFIG_TTY_FORCE_PANIC_REPEAT_COUNT)
+ if (ch == CONFIG_TTY_FORCE_PANIC_CHAR)
{
- PANIC_WITH_REGS("Force panic by user.", NULL);
- }
+ if (++dev->panic_count >= CONFIG_TTY_FORCE_PANIC_REPEAT_COUNT)
+ {
+ PANIC_WITH_REGS("Force panic by user.", NULL);
+ }
- return 0;
- }
- else
- {
- dev->panic_count = 0;
+ check = false;
+ }
+ else
+ {
+ dev->panic_count = 0;
+ }
}
#endif
#ifdef CONFIG_TTY_LAUNCH
- if (buf[i] == CONFIG_TTY_LAUNCH_CHAR)
Review Comment:
should we eat CONFIG_TTY_LAUNCH_CHAR too?
--
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]
