You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by gn...@apache.org on 2020/02/07 13:10:49 UTC

[incubator-nuttx] branch pr215 updated (3b311ab -> db54252)

This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a change to branch pr215
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git.


    from 3b311ab  tools/testbuild.sh: use function to call make and fail handle in common
     new 2e6de18  Split common_ioctl to telnet_ioctl and factory_ioctl
     new 8d5b332  Remove the unnecessary '\0' terminator from telnet driver
     new 134da2c  Refine Ctrl+C handling in telnet driver
     new 386b2d8  telnet driver should return -EAGAIN if O_NONBLOCK is active
     new e315873  Remove g_telnet_common global variable
     new 526fa7b  Trigger SGA and ECHO proactively in the character mode
     new db54252  Unify the coding style in telnet driver

The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 drivers/net/telnet.c | 407 +++++++++++++++++++++------------------------------
 1 file changed, 168 insertions(+), 239 deletions(-)


[incubator-nuttx] 01/07: Split common_ioctl to telnet_ioctl and factory_ioctl

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a commit to branch pr215
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit 2e6de18c403801ebca6f60221310fb6e522f9141
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Wed Feb 5 16:12:53 2020 +0800

    Split common_ioctl to telnet_ioctl and factory_ioctl
    
    and remove the wrong telnet_poll from g_factory_fops
    
    Change-Id: I39f278763ff279d464c5be6728b9936c6cab16eb
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
 drivers/net/telnet.c | 109 +++++++++++++++++++++++++++++----------------------
 1 file changed, 62 insertions(+), 47 deletions(-)

diff --git a/drivers/net/telnet.c b/drivers/net/telnet.c
index 206349f..92e6b05 100644
--- a/drivers/net/telnet.c
+++ b/drivers/net/telnet.c
@@ -215,6 +215,8 @@ static ssize_t telnet_read(FAR struct file *filep, FAR char *buffer,
                  size_t len);
 static ssize_t telnet_write(FAR struct file *filep, FAR const char *buffer,
                  size_t len);
+static int     telnet_ioctl(FAR struct file *filep, int cmd,
+                 unsigned long arg);
 static int     telnet_poll(FAR struct file *filep, FAR struct pollfd *fds,
                  bool setup);
 
@@ -228,7 +230,7 @@ static ssize_t factory_read(FAR struct file *filep, FAR char *buffer,
                  size_t buflen);
 static ssize_t factory_write(FAR struct file *filep, FAR const char *buffer,
                  size_t buflen);
-static int     common_ioctl(FAR struct file *filep, int cmd,
+static int     factory_ioctl(FAR struct file *filep, int cmd,
                  unsigned long arg);
 
 /****************************************************************************
@@ -242,7 +244,7 @@ static const struct file_operations g_telnet_fops =
   telnet_read,   /* read */
   telnet_write,  /* write */
   NULL,          /* seek */
-  common_ioctl,  /* ioctl */
+  telnet_ioctl,  /* ioctl */
   telnet_poll    /* poll */
 #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
   , NULL         /* unlink */
@@ -256,8 +258,8 @@ static const struct file_operations g_factory_fops =
   factory_read,  /* read */
   factory_write, /* write */
   NULL,          /* seek */
-  common_ioctl,  /* ioctl */
-  telnet_poll    /* poll */
+  factory_ioctl, /* ioctl */
+  NULL           /* poll */
 #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
   , NULL         /* unlink */
 #endif
@@ -1215,6 +1217,58 @@ static ssize_t factory_write(FAR struct file *filep, FAR const char *buffer,
 }
 
 /****************************************************************************
+ * Name: telnet_ioctl
+ ****************************************************************************/
+
+static int telnet_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct telnet_dev_s *priv = inode->i_private;
+  int ret = OK;
+
+  switch (cmd)
+    {
+#ifdef HAVE_SIGNALS
+      /* Make the given terminal the controlling terminal of the calling process */
+
+    case TIOCSCTTY:
+      {
+        /* Check if the ISIG flag is set in the termios c_lflag to enable
+         * this feature.  This flag is set automatically for a serial console
+         * device.
+         */
+
+        /* Save the PID of the recipient of the SIGINT signal. */
+
+        priv->td_pid = (pid_t)arg;
+        DEBUGASSERT((unsigned long)(priv->td_pid) == arg);
+      }
+      break;
+#endif
+
+#ifdef CONFIG_TELNET_SUPPORT_NAWS
+      case TIOCGWINSZ:
+        {
+          FAR struct winsize *pw = (FAR struct winsize *)((uintptr_t)arg);
+
+          /* Get row/col from the private data */
+
+          pw->ws_row = priv->td_rows;
+          pw->ws_col = priv->td_cols;
+        }
+      break;
+#endif
+
+    default:
+      ret = -ENOTTY;
+      break;
+    }
+
+  UNUSED(priv);  /* Avoid warning if not used */
+  return ret;
+}
+
+/****************************************************************************
  * Name: telnet_poll
  *
  * Description:
@@ -1383,15 +1437,12 @@ static int telnet_io_main(int argc, FAR char** argv)
 }
 
 /****************************************************************************
- * Name: common_ioctl
+ * Name: factory_ioctl
  ****************************************************************************/
 
-static int common_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+static int factory_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
 {
-  FAR struct inode *inode = filep->f_inode;
-  FAR struct telnet_dev_s *priv = inode->i_private;
-
-  int ret;
+  int ret = OK;
 
   switch (cmd)
     {
@@ -1405,7 +1456,7 @@ static int common_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
     case SIOCTELNET:
       {
         FAR struct telnet_session_s *session =
-            (FAR struct telnet_session_s *)((uintptr_t) arg);
+            (FAR struct telnet_session_s *)((uintptr_t)arg);
 
         if (session == NULL)
           {
@@ -1418,47 +1469,11 @@ static int common_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
       }
       break;
 
-#ifdef HAVE_SIGNALS
-      /* Make the given terminal the controlling terminal of the calling process */
-
-    case TIOCSCTTY:
-      {
-        /* Check if the ISIG flag is set in the termios c_lflag to enable
-         * this feature.  This flag is set automatically for a serial console
-         * device.
-         */
-
-        /* Save the PID of the recipient of the SIGINT signal. */
-
-        priv->pid = (pid_t)arg;
-        DEBUGASSERT((unsigned long)(priv->pid) == arg);
-
-        ret = OK;
-      }
-      break;
-#endif
-
-#ifdef CONFIG_TELNET_SUPPORT_NAWS
-      case TIOCGWINSZ:
-        {
-          FAR struct winsize *pw = (FAR struct winsize *)((uintptr_t)arg);
-
-          /* Get row/col from the private data */
-
-          pw->ws_row = priv->td_rows;
-          pw->ws_col = priv->td_cols;
-
-          ret = OK;
-        }
-      break;
-#endif
-
     default:
       ret = -ENOTTY;
       break;
     }
 
-  UNUSED(priv);  /* Avoid warning if not used */
   return ret;
 }
 


[incubator-nuttx] 06/07: Trigger SGA and ECHO proactively in the character mode

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a commit to branch pr215
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit 526fa7b77359c38ac21d2856c7f5d3baf365ec08
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Wed Feb 5 23:52:37 2020 +0800

    Trigger SGA and ECHO proactively in the character mode
    
    otherwise Ubuntu bultin telnet can't enter this mode
    
    Change-Id: I8aa2ab2b31c35007077c701c264b3971152435f0
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
 drivers/net/telnet.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/telnet.c b/drivers/net/telnet.c
index 6ab3ab2..d5a980c 100644
--- a/drivers/net/telnet.c
+++ b/drivers/net/telnet.c
@@ -466,19 +466,9 @@ static ssize_t telnet_receive(FAR struct telnet_dev_s *priv,
 
           case STATE_DO:
 #ifdef CONFIG_TELNET_CHARACTER_MODE
-            if (ch == TELNET_SGA)
+            if (ch == TELNET_SGA || ch == TELNET_ECHO)
               {
-                /* If it received 'Suppress Go Ahead', reply with a WILL */
-
-                telnet_sendopt(priv, TELNET_WILL, ch);
-
-                /* Also, send 'WILL ECHO' */
-
-                telnet_sendopt(priv, TELNET_WILL, TELNET_ECHO);
-              }
-            else if (ch == TELNET_ECHO)
-              {
-                /* If it received 'ECHO', then do nothing */
+                /* If it received 'ECHO' or 'Suppress Go Ahead', then do nothing */
               }
             else
               {
@@ -1108,6 +1098,11 @@ static int telnet_session(FAR struct telnet_session_s *session)
   telnet_sendopt(priv, TELNET_DO, TELNET_NAWS);
 #endif
 
+#ifdef CONFIG_TELNET_CHARACTER_MODE
+  telnet_sendopt(priv, TELNET_WILL, TELNET_SGA);
+  telnet_sendopt(priv, TELNET_WILL, TELNET_ECHO);
+#endif
+
   /* Has the I/O thread been started? */
 
   if (g_telnet_io_kthread == (pid_t)0)


[incubator-nuttx] 07/07: Unify the coding style in telnet driver

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a commit to branch pr215
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit db542520f93d9da6e481cf6f46c7ce410a53f941
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Wed Feb 5 17:52:26 2020 +0800

    Unify the coding style in telnet driver
    
    ensure kernel version API used in all place
    and remove the unused stuff
    
    Change-Id: Id9bec757095b15e204bedc89cab24c965031304e
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
 drivers/net/telnet.c | 108 ++++++++++++++++++++-------------------------------
 1 file changed, 42 insertions(+), 66 deletions(-)

diff --git a/drivers/net/telnet.c b/drivers/net/telnet.c
index d5a980c..33fe9df 100644
--- a/drivers/net/telnet.c
+++ b/drivers/net/telnet.c
@@ -45,22 +45,16 @@
 
 #include <nuttx/config.h>
 
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/stat.h>
-
-#include <stdint.h>
-#include <stdbool.h>
+#include <assert.h>
 #include <stdio.h>
-#include <unistd.h>
-#include <stdlib.h>
 #include <fcntl.h>
-#include <string.h>
 #include <poll.h>
 #include <errno.h>
 #include <debug.h>
 
+#include <nuttx/kmalloc.h>
 #include <nuttx/kthread.h>
+#include <nuttx/signal.h>
 #include <nuttx/semaphore.h>
 #include <nuttx/fs/fs.h>
 #include <nuttx/net/net.h>
@@ -101,8 +95,8 @@
 
 /* Telnet protocol stuff ****************************************************/
 
-#define ISO_nl                0x0a
-#define ISO_cr                0x0d
+#define TELNET_NL             0x0a
+#define TELNET_CR             0x0d
 
 /* Telnet commands */
 
@@ -120,15 +114,9 @@
 #define TELNET_SB             250
 #define TELNET_SE             240
 
-/* Linemode sub options */
-
-#define TELNET_LM_MODE        1
-#define TELNET_LM_FORWARDMASK 2
-#define TELNET_LM_SLC         3
-
 /* Device stuff *************************************************************/
 
-#define TELNETD_DEVFMT "/dev/telnet%d"
+#define TELNET_DEVFMT         "/dev/telnet%d"
 
 /****************************************************************************
  * Private Types
@@ -166,9 +154,9 @@ struct telnet_dev_s
   int               td_sb_count;  /* Count of TELNET_SB bytes received */
 #endif
 #ifdef HAVE_SIGNALS
-  pid_t             pid;
+  pid_t             td_pid;
 #endif
-  struct pollfd     fds;
+  struct pollfd     td_fds;
   FAR struct socket td_psock;     /* A clone of the internal socket structure */
   char td_rxbuffer[CONFIG_TELNET_RXBUFFER_SIZE];
   char td_txbuffer[CONFIG_TELNET_TXBUFFER_SIZE];
@@ -195,7 +183,7 @@ static bool    telnet_putchar(FAR struct telnet_dev_s *priv, uint8_t ch,
                  int *nwritten);
 static void    telnet_sendopt(FAR struct telnet_dev_s *priv, uint8_t option,
                  uint8_t value);
-static int     telnet_io_main(int argc, char** argv);
+static int     telnet_io_main(int argc, FAR char** argv);
 
 /* Telnet character driver methods */
 
@@ -304,7 +292,7 @@ static void telnet_check_ctrlchar(FAR struct telnet_dev_s *priv,
 {
   int signo = 0;
 
-  for (; priv->pid >= 0 && len > 0; buffer++, len--)
+  for (; priv->td_pid >= 0 && len > 0; buffer++, len--)
     {
 #ifdef CONFIG_TTY_SIGINT
       /* Is this the special character that will generate the SIGINT signal? */
@@ -338,7 +326,7 @@ static void telnet_check_ctrlchar(FAR struct telnet_dev_s *priv,
 
   if (signo != 0)
     {
-      kill(priv->pid, signo);
+      nxsig_kill(priv->td_pid, signo);
     }
 }
 #endif
@@ -359,7 +347,7 @@ static void telnet_getchar(FAR struct telnet_dev_s *priv, uint8_t ch,
 #ifndef CONFIG_TELNET_CHARACTER_MODE
   /* Ignore carriage returns */
 
-  if (ch != ISO_cr)
+  if (ch != TELNET_CR)
 #endif
     {
       /* Add all other characters to the destination buffer */
@@ -598,7 +586,7 @@ static bool telnet_putchar(FAR struct telnet_dev_s *priv, uint8_t ch,
 
   /* Ignore carriage returns (we will put these in automatically as necessary) */
 
-  if (ch != ISO_cr)
+  if (ch != TELNET_CR)
     {
       /* Add all other characters to the destination buffer */
 
@@ -607,11 +595,11 @@ static bool telnet_putchar(FAR struct telnet_dev_s *priv, uint8_t ch,
 
       /* Check for line feeds */
 
-      if (ch == ISO_nl)
+      if (ch == TELNET_NL)
         {
           /* Now add the carriage return */
 
-          priv->td_txbuffer[index++] = ISO_cr;
+          priv->td_txbuffer[index++] = TELNET_CR;
 
           /* End of line */
 
@@ -737,8 +725,7 @@ static int telnet_close(FAR struct file *filep)
     {
       /* Re-create the path to the driver. */
 
-      sched_lock();
-      ret = asprintf(&devpath, TELNETD_DEVFMT, priv->td_minor);
+      ret = asprintf(&devpath, TELNET_DEVFMT, priv->td_minor);
       if (ret < 0)
         {
           nerr("ERROR: Failed to allocate the driver path\n");
@@ -762,9 +749,13 @@ static int telnet_close(FAR struct file *filep)
                   nerr("ERROR: Failed to unregister the driver %s: %d\n",
                        devpath, ret);
                 }
+              else
+                {
+                  ret = OK;
+                }
             }
 
-          free(devpath);
+          kmm_free(devpath);
         }
 
       /* Remove ourself from the clients list */
@@ -774,19 +765,19 @@ static int telnet_close(FAR struct file *filep)
         {
           if (g_telnet_clients[i] == priv)
             {
-              g_telnet_clients[i] = 0;
+              g_telnet_clients[i] = NULL;
               break;
             }
         }
 
       /* If the socket is still polling */
 
-      if (priv->fds.events)
+      if (priv->td_fds.events)
         {
           /* Tear down the poll */
 
-          psock_poll(&priv->td_psock, &priv->fds, FALSE);
-          priv->fds.events = 0;
+          psock_poll(&priv->td_psock, &priv->td_fds, FALSE);
+          priv->td_fds.events = 0;
         }
 
       nxsem_post(&g_clients_sem);
@@ -799,13 +790,6 @@ static int telnet_close(FAR struct file *filep)
 
       psock_close(&priv->td_psock);
 
-#ifdef CONFIG_TERMCURSES
-      if (priv->tcurs != NULL)
-        {
-          free(priv->tcurs);
-        }
-#endif
-
       /* Release the driver memory.  What if there are threads waiting on
        * td_exclsem?  They will never be awakened!  How could this happen?
        * crefs == 1 so there are no other open references to the driver.
@@ -816,12 +800,9 @@ static int telnet_close(FAR struct file *filep)
 
       DEBUGASSERT(priv->td_exclsem.semcount == 0);
       nxsem_destroy(&priv->td_exclsem);
-      free(priv);
-      sched_unlock();
+      kmm_free(priv);
     }
 
-  ret = OK;
-
 errout:
   return ret;
 }
@@ -854,7 +835,7 @@ static ssize_t telnet_read(FAR struct file *filep, FAR char *buffer,
         {
           /* poll fds.revents contains last poll status in case of error */
 
-          if ((priv->fds.revents & (POLLHUP | POLLERR)) != 0)
+          if ((priv->td_fds.revents & (POLLHUP | POLLERR)) != 0)
             {
               return -EPIPE;
             }
@@ -883,8 +864,6 @@ static ssize_t telnet_read(FAR struct file *filep, FAR char *buffer,
     }
   while (ret == 0);
 
-  return ret;
-
   /* Returned Value:
    *
    * ret > 0:  The number of characters copied into the user buffer by
@@ -993,7 +972,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
 
   /* Allocate instance data for this driver */
 
-  priv = (FAR struct telnet_dev_s *)zalloc(sizeof(struct telnet_dev_s));
+  priv = (FAR struct telnet_dev_s *)kmm_zalloc(sizeof(struct telnet_dev_s));
   if (!priv)
     {
       nerr("ERROR: Failed to allocate the driver data structure\n");
@@ -1017,16 +996,13 @@ static int telnet_session(FAR struct telnet_session_s *session)
   priv->td_pending   = 0;
   priv->td_offset    = 0;
 #ifdef HAVE_SIGNALS
-  priv->pid          = -1;
+  priv->td_pid       = -1;
 #endif
 #ifdef CONFIG_TELNET_SUPPORT_NAWS
   priv->td_rows      = 25;
   priv->td_cols      = 80;
   priv->td_sb_count  = 0;
 #endif
-#ifdef CONFIG_TERMCURSES
-  priv->tcurs        = NULL;
-#endif
 
   /* Clone the internal socket structure.  We do this so that it will be
    * independent of threads and of socket descriptors (the original socket
@@ -1067,7 +1043,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
       if (g_telnet_clients[priv->td_minor] == NULL)
         {
           snprintf(session->ts_devpath, TELNET_DEVPATH_MAX,
-                   TELNETD_DEVFMT, priv->td_minor);
+                   TELNET_DEVFMT, priv->td_minor);
           break;
         }
       priv->td_minor++;
@@ -1117,7 +1093,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
 
       g_telnet_io_kthread =
         kthread_create("telnet_io", CONFIG_TELNET_IOTHREAD_PRIORITY,
-                       CONFIG_TELNET_IOTHREAD_STACKSIZE, telnet_io_main, 0);
+                       CONFIG_TELNET_IOTHREAD_STACKSIZE, telnet_io_main, NULL);
     }
 
   /* Save ourself in the list of Telnet client threads */
@@ -1287,19 +1263,19 @@ static int telnet_io_main(int argc, FAR char** argv)
       for (i = 0; i < CONFIG_TELNET_MAXLCLIENTS; i++)
         {
           priv = g_telnet_clients[i];
-          if (priv != NULL && !(priv->fds.revents & (POLLHUP | POLLERR)))
+          if (priv != NULL && !(priv->td_fds.revents & (POLLHUP | POLLERR)))
             {
-              priv->fds.sem     = &g_iosem;
-              priv->fds.events  = POLLIN | POLLHUP | POLLERR;
-              priv->fds.revents = 0;
+              priv->td_fds.sem     = &g_iosem;
+              priv->td_fds.events  = POLLIN | POLLHUP | POLLERR;
+              priv->td_fds.revents = 0;
 
-              psock_poll(&priv->td_psock, &priv->fds, TRUE);
+              psock_poll(&priv->td_psock, &priv->td_fds, TRUE);
             }
         }
 
       nxsem_post(&g_clients_sem);
 
-      /* Wait for any Telnet connect/disconnect events to
+      /* Wait for any Telnet connect/disconnect events
        * to include/remove client sockets from polling
        */
 
@@ -1314,11 +1290,11 @@ static int telnet_io_main(int argc, FAR char** argv)
 
           /* If poll was setup previously (events != 0) */
 
-          if (priv != NULL && priv->fds.events)
+          if (priv != NULL && priv->td_fds.events)
             {
               /* Check for a pending poll() */
 
-              if (priv->fds.revents & POLLIN)
+              if (priv->td_fds.revents & POLLIN)
                 {
                   if (priv->td_pending < CONFIG_TELNET_RXBUFFER_SIZE)
                     {
@@ -1352,14 +1328,14 @@ static int telnet_io_main(int argc, FAR char** argv)
 
               /* Tear it down */
 
-              psock_poll(&priv->td_psock, &priv->fds, FALSE);
-              priv->fds.events = 0;
+              psock_poll(&priv->td_psock, &priv->td_fds, FALSE);
+              priv->td_fds.events = 0;
 
               /* POLLHUP (or POLLERR) indicates that this session has
                * terminated.
                */
 
-              if (priv->fds.revents & (POLLHUP | POLLERR))
+              if (priv->td_fds.revents & (POLLHUP | POLLERR))
                 {
                   /* notify the client thread */
 


[incubator-nuttx] 04/07: telnet driver should return -EAGAIN if O_NONBLOCK is active

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a commit to branch pr215
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit 386b2d834ad4c1f655fa544dfc1ea6f7fa66f72b
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Wed Feb 5 22:04:34 2020 +0800

    telnet driver should return -EAGAIN if O_NONBLOCK is active
    
    also should report -EPIPE first
    
    Change-Id: I7ad2df15377c7bec8e22d0f5d1b54f7ce33eb0db
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
 drivers/net/telnet.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/net/telnet.c b/drivers/net/telnet.c
index 881abba..c8e7ad2 100644
--- a/drivers/net/telnet.c
+++ b/drivers/net/telnet.c
@@ -692,14 +692,6 @@ static int telnet_open(FAR struct file *filep)
 
   ninfo("td_crefs: %d\n", priv->td_crefs);
 
-  /* O_NONBLOCK is not supported */
-
-  if (filep->f_oflags & O_NONBLOCK)
-    {
-      ret = -ENOSYS;
-      goto errout;
-    }
-
   /* Get exclusive access to the device structures */
 
   ret = nxsem_wait(&priv->td_exclsem);
@@ -871,7 +863,7 @@ static ssize_t telnet_read(FAR struct file *filep, FAR char *buffer,
 {
   FAR struct inode *inode = filep->f_inode;
   FAR struct telnet_dev_s *priv = inode->i_private;
-  ssize_t ret;
+  ssize_t ret = 0;
 
   ninfo("len: %d\n", len);
 
@@ -888,21 +880,22 @@ static ssize_t telnet_read(FAR struct file *filep, FAR char *buffer,
 
       if (priv->td_pending == 0)
         {
+          /* poll fds.revents contains last poll status in case of error */
+
+          if ((priv->fds.revents & (POLLHUP | POLLERR)) != 0)
+            {
+              return -EPIPE;
+            }
+
           if (filep->f_oflags & O_NONBLOCK)
             {
-              return 0;
+              return -EAGAIN;
             }
 
           /* Wait for new data (or error) */
 
           nxsem_wait_uninterruptible(&priv->td_iosem);
-
-          /* poll fds.revents contains last poll status in case of error */
-
-          if ((priv->fds.revents & (POLLHUP | POLLERR)) != 0)
-            {
-              return -EPIPE;
-            }
+          continue;
         }
 
       /* Take exclusive access to data buffer */
@@ -1304,7 +1297,10 @@ static int telnet_poll(FAR struct file *filep, FAR struct pollfd *fds,
       /* Yes.. then signal the poll logic */
 
       fds->revents |= (POLLRDNORM & fds->events);
-      nxsem_post(fds->sem);
+      if (fds->revents)
+        {
+          nxsem_post(fds->sem);
+        }
     }
 
   /* Then let psock_poll() do the heavy lifting */


[incubator-nuttx] 05/07: Remove g_telnet_common global variable

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a commit to branch pr215
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit e3158730265cbfc454deb5d1051d41d41da7e498
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Wed Feb 5 17:57:58 2020 +0800

    Remove g_telnet_common global variable
    
    we can reuse g_clients_sem as the lock guard
    
    Change-Id: Ic3af9f2116f70523a4249b29c65bd1fb83ca4da2
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
 drivers/net/telnet.c | 87 +++++++++++++++-------------------------------------
 1 file changed, 24 insertions(+), 63 deletions(-)

diff --git a/drivers/net/telnet.c b/drivers/net/telnet.c
index c8e7ad2..6ab3ab2 100644
--- a/drivers/net/telnet.c
+++ b/drivers/net/telnet.c
@@ -159,7 +159,7 @@ struct telnet_dev_s
   uint8_t           td_pending;   /* Number of valid, pending bytes in the rxbuffer */
   uint8_t           td_offset;    /* Offset to the valid, pending bytes in the rxbuffer */
   uint8_t           td_crefs;     /* The number of open references to the session */
-  int               td_minor;     /* Minor device number */
+  uint8_t           td_minor;     /* Minor device number */
 #ifdef CONFIG_TELNET_SUPPORT_NAWS
   uint16_t          td_rows;      /* Number of NAWS rows */
   uint16_t          td_cols;      /* Number of NAWS cols */
@@ -174,16 +174,6 @@ struct telnet_dev_s
   char td_txbuffer[CONFIG_TELNET_TXBUFFER_SIZE];
 };
 
-/* This structure  contains global information visible to all telnet driver
- * instances.
- */
-
-struct telnet_common_s
-{
-  sem_t             tc_exclsem;   /* Enforces exclusive access to 'minor' */
-  uint16_t          tc_minor;     /* The next minor number to use */
-};
-
 /****************************************************************************
  * Private Function Prototypes
  ****************************************************************************/
@@ -265,14 +255,6 @@ static const struct file_operations g_factory_fops =
 #endif
 };
 
-/* Global information shared amongst telnet driver instances. */
-
-static struct telnet_common_s g_telnet_common =
-{
-  SEM_INITIALIZER(1),
-  0
-};
-
 /* This is an global data set of all of all active Telnet drivers.  This
  * additional logic in included to handle killing of task via control
  * characters received via Telenet (via Ctrl-C SIGINT, in particular).
@@ -1017,10 +999,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
 {
   FAR struct telnet_dev_s *priv;
   FAR struct socket *psock;
-  struct stat statbuf;
-  uint16_t start;
   int ret;
-  int i;
 
   /* Allocate instance data for this driver */
 
@@ -1044,6 +1023,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
 
   priv->td_state     = STATE_NORMAL;
   priv->td_crefs     = 0;
+  priv->td_minor     = 0;
   priv->td_pending   = 0;
   priv->td_offset    = 0;
 #ifdef HAVE_SIGNALS
@@ -1083,7 +1063,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
    * Get exclusive access to the minor counter.
    */
 
-  ret = nxsem_wait_uninterruptible(&g_telnet_common.tc_exclsem);
+  ret = nxsem_wait_uninterruptible(&g_clients_sem);
   if (ret < 0)
     {
       nerr("ERROR: nxsem_wait failed: %d\n", ret);
@@ -1092,23 +1072,18 @@ static int telnet_session(FAR struct telnet_session_s *session)
 
   /* Loop until the device name is verified to be unique. */
 
-  start = g_telnet_common.tc_minor;
-  do
+  while (priv->td_minor < CONFIG_TELNET_MAXLCLIENTS)
     {
-      /* Get the next candiate minor number */
-
-      priv->td_minor = g_telnet_common.tc_minor;
-      g_telnet_common.tc_minor++;
-
-      snprintf(session->ts_devpath, TELNET_DEVPATH_MAX, TELNETD_DEVFMT,
-               priv->td_minor);
-
-      ret = stat(session->ts_devpath, &statbuf);
-      DEBUGASSERT(ret >= 0 || get_errno() == ENOENT);
+      if (g_telnet_clients[priv->td_minor] == NULL)
+        {
+          snprintf(session->ts_devpath, TELNET_DEVPATH_MAX,
+                   TELNETD_DEVFMT, priv->td_minor);
+          break;
+        }
+      priv->td_minor++;
     }
-  while (ret >= 0 && start != g_telnet_common.tc_minor);
 
-  if (ret >= 0)
+  if (priv->td_minor >= CONFIG_TELNET_MAXLCLIENTS)
     {
       nerr("ERROR: Too many sessions\n");
       ret = -ENFILE;
@@ -1152,26 +1127,14 @@ static int telnet_session(FAR struct telnet_session_s *session)
 
   /* Save ourself in the list of Telnet client threads */
 
-  nxsem_wait(&g_clients_sem);
-  for (i = 0; i < CONFIG_TELNET_MAXLCLIENTS; i++)
-    {
-      if (g_telnet_clients[i] == NULL)
-        {
-          g_telnet_clients[i] = priv;
-          break;
-        }
-    }
-
+  g_telnet_clients[priv->td_minor] = priv;
   nxsem_post(&g_clients_sem);
   nxsem_post(&g_iosem);
 
-  /* Return the path to the new telnet driver */
-
-  nxsem_post(&g_telnet_common.tc_exclsem);
   return OK;
 
 errout_with_semaphore:
-  nxsem_post(&g_telnet_common.tc_exclsem);
+  nxsem_post(&g_clients_sem);
 
 errout_with_clone:
   psock_close(&priv->td_psock);
@@ -1328,9 +1291,9 @@ static int telnet_io_main(int argc, FAR char** argv)
       nxsem_wait(&g_clients_sem);
       for (i = 0; i < CONFIG_TELNET_MAXLCLIENTS; i++)
         {
-          if (g_telnet_clients[i] != 0)
+          priv = g_telnet_clients[i];
+          if (priv != NULL && !(priv->fds.revents & (POLLHUP | POLLERR)))
             {
-              priv              = g_telnet_clients[i];
               priv->fds.sem     = &g_iosem;
               priv->fds.events  = POLLIN | POLLHUP | POLLERR;
               priv->fds.revents = 0;
@@ -1352,11 +1315,14 @@ static int telnet_io_main(int argc, FAR char** argv)
       nxsem_wait(&g_clients_sem);
       for (i = 0; i < CONFIG_TELNET_MAXLCLIENTS; i++)
         {
-          if (g_telnet_clients[i] != 0)
+          priv = g_telnet_clients[i];
+
+          /* If poll was setup previously (events != 0) */
+
+          if (priv != NULL && priv->fds.events)
             {
               /* Check for a pending poll() */
 
-              priv = g_telnet_clients[i];
               if (priv->fds.revents & POLLIN)
                 {
                   if (priv->td_pending < CONFIG_TELNET_RXBUFFER_SIZE)
@@ -1389,13 +1355,10 @@ static int telnet_io_main(int argc, FAR char** argv)
                     }
                 }
 
-              /* If poll was setup previously (events != 0), tear it down */
+              /* Tear it down */
 
-              if (priv->fds.events)
-                {
-                  psock_poll(&priv->td_psock, &priv->fds, FALSE);
-                  priv->fds.events = 0;
-                }
+              psock_poll(&priv->td_psock, &priv->fds, FALSE);
+              priv->fds.events = 0;
 
               /* POLLHUP (or POLLERR) indicates that this session has
                * terminated.
@@ -1403,8 +1366,6 @@ static int telnet_io_main(int argc, FAR char** argv)
 
               if (priv->fds.revents & (POLLHUP | POLLERR))
                 {
-                  g_telnet_clients[i] = 0;
-
                   /* notify the client thread */
 
                   nxsem_post(&priv->td_iosem);


[incubator-nuttx] 03/07: Refine Ctrl+C handling in telnet driver

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a commit to branch pr215
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit 134da2c272018b703895bdceeca5fa236b13eafd
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Wed Feb 5 17:14:30 2020 +0800

    Refine Ctrl+C handling in telnet driver
    
    to avoid issue the kill more than once
    
    Change-Id: I9fcec5d861ea85258170f379d741d2bb8e4d9b9e
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
 drivers/net/telnet.c | 49 +++++++++++++++++++------------------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/net/telnet.c b/drivers/net/telnet.c
index e44feb7..881abba 100644
--- a/drivers/net/telnet.c
+++ b/drivers/net/telnet.c
@@ -309,7 +309,7 @@ static inline void telnet_dumpbuffer(FAR const char *msg,
 #endif
 
 /****************************************************************************
- * Name: telnet_check_ctrl_char
+ * Name: telnet_check_ctrlchar
  *
  * Description:
  *   Check if an incoming control character should generate a signal.
@@ -317,34 +317,31 @@ static inline void telnet_dumpbuffer(FAR const char *msg,
  ****************************************************************************/
 
 #ifdef HAVE_SIGNALS
-static void telnet_check_ctrl_char (FAR struct telnet_dev_s *priv,
-                                    uint8_t ch)
+static void telnet_check_ctrlchar(FAR struct telnet_dev_s *priv,
+                                  FAR char *buffer, size_t len)
 {
   int signo = 0;
 
+  for (; priv->pid >= 0 && len > 0; buffer++, len--)
+    {
 #ifdef CONFIG_TTY_SIGINT
-  /* Is this the special character that will generate the SIGINT signal? */
+      /* Is this the special character that will generate the SIGINT signal? */
 
-  if (priv->pid >= 0 && ch == CONFIG_TTY_SIGINT_CHAR)
-    {
-      /* Yes.. note that the kill is needed and do not put the character
-       * into the Rx buffer.  It should not be read as normal data.
-       */
+      if (*buffer == CONFIG_TTY_SIGINT_CHAR)
+        {
+          /* Yes.. note that the kill is needed and do not put the character
+           * into the Rx buffer.  It should not be read as normal data.
+           */
 
-      signo = SIGINT;
-    }
-  else
+          signo = SIGINT;
+          break;
+        }
+      else
 #endif
 #ifdef CONFIG_TTY_SIGSTP
-  /* Is this the special character that will generate the SIGSTP signal? */
+      /* Is this the special character that will generate the SIGSTP signal? */
 
-  if (priv->pid >= 0 && ch == CONFIG_TTY_SIGSTP_CHAR)
-    {
-#ifdef CONFIG_TTY_SIGINT
-      /* Give precedence to SIGINT */
-
-      if (signo == 0)
-#endif
+      if (*buffer == CONFIG_TTY_SIGSTP_CHAR)
         {
           /* Note that the kill is needed and do not put the character
            * into the Rx buffer.  It should not be read as normal data.
@@ -352,17 +349,15 @@ static void telnet_check_ctrl_char (FAR struct telnet_dev_s *priv,
 
           signo = SIGSTP;
         }
-    }
 #endif
+    }
 
-#if defined(CONFIG_TTY_SIGINT) || defined(CONFIG_TTY_SIGSTP)
   /* Send the signal if necessary */
 
   if (signo != 0)
     {
       kill(priv->pid, signo);
     }
-#endif
 }
 #endif
 
@@ -1326,9 +1321,6 @@ static int telnet_io_main(int argc, FAR char** argv)
   FAR struct telnet_dev_s *priv;
   FAR char *buffer;
   int i;
-#ifdef HAVE_SIGNALS
-  int c;
-#endif
   int ret;
 
   while (1)
@@ -1396,10 +1388,7 @@ static int telnet_io_main(int argc, FAR char** argv)
                        * control that should generate a signal.
                        */
 
-                      for (c = 0; c < ret; c++)
-                        {
-                          telnet_check_ctrl_char(priv, buffer[c]);
-                        }
+                      telnet_check_ctrlchar(priv, buffer, ret);
 #endif
                     }
                 }


[incubator-nuttx] 02/07: Remove the unnecessary '\0' terminator from telnet driver

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a commit to branch pr215
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git

commit 8d5b33235fdd7b8b792b7423aab42ce7dc04968e
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Wed Feb 5 16:24:54 2020 +0800

    Remove the unnecessary '\0' terminator from telnet driver
    
    Change-Id: If648641651a0355f5c445dd2b65e14d08cddb8c1
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
 drivers/net/telnet.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/telnet.c b/drivers/net/telnet.c
index 92e6b05..e44feb7 100644
--- a/drivers/net/telnet.c
+++ b/drivers/net/telnet.c
@@ -645,7 +645,6 @@ static bool telnet_putchar(FAR struct telnet_dev_s *priv, uint8_t ch,
           /* Now add the carriage return */
 
           priv->td_txbuffer[index++] = ISO_cr;
-          priv->td_txbuffer[index++] = '\0';
 
           /* End of line */
 
@@ -669,17 +668,16 @@ static bool telnet_putchar(FAR struct telnet_dev_s *priv, uint8_t ch,
 static void telnet_sendopt(FAR struct telnet_dev_s *priv, uint8_t option,
                            uint8_t value)
 {
-  uint8_t optbuf[4];
+  uint8_t optbuf[3];
   int ret;
 
   optbuf[0] = TELNET_IAC;
   optbuf[1] = option;
   optbuf[2] = value;
-  optbuf[3] = 0;
 
-  telnet_dumpbuffer("Send optbuf", optbuf, 4);
+  telnet_dumpbuffer("Send optbuf", optbuf, 3);
 
-  ret = psock_send(&priv->td_psock, optbuf, 4, 0);
+  ret = psock_send(&priv->td_psock, optbuf, 3, 0);
   if (ret < 0)
     {
       nerr("ERROR: Failed to send TELNET_IAC: %d\n", ret);
@@ -968,10 +966,10 @@ static ssize_t telnet_write(FAR struct file *filep, FAR const char *buffer,
       eol = telnet_putchar(priv, ch, &ncopied);
 
       /* Was that the end of a line? Or is the buffer too full to hold the
-       * next largest character sequence ("\r\n\0")?
+       * next largest character sequence ("\r\n")?
        */
 
-      if (eol || ncopied > CONFIG_TELNET_TXBUFFER_SIZE - 3)
+      if (eol || ncopied > CONFIG_TELNET_TXBUFFER_SIZE - 2)
         {
           /* Yes... send the data now */
 
@@ -1003,9 +1001,8 @@ static ssize_t telnet_write(FAR struct file *filep, FAR const char *buffer,
 
   /* Notice that we don't actually return the number of bytes sent, but
    * rather, the number of bytes that the caller asked us to send.  We may
-   * have sent more bytes (because of CR-LF expansion and because of NULL
-   * termination). But it confuses some logic if you report that you sent
-   * more than you were requested to.
+   * have sent more bytes (because of CR-LF expansion). But it confuses
+   * some logic if you report that you sent more than you were requested to.
    */
 
   return len;