[vlc-devel] [PATCH] cli: fix line trimming with socket cli

[Alexandre Janniaux]

Hi,

In term of UX, it avoids this issue:

    $ telnet 192.168.1.31 9999
    Trying 192.168.1.31...
    Connected to 192.168.1.31.
    Escape character is '^]'.
    previous
    '. Type `help' for help.s
    prev
    '. Type `help' for help.
    .
    '. Type `help' for help.
    .
    '. Type `help' for help.
    help
    '. Type `help' for help.
    help
    '. Type `help' for help.
    start
    '. Type `help' for help.

which was previously leading to the correct behaviour before
the refactor of the CLI code.

Regards,
--
Alexandre Janniaux
Videolabs


On Thu, Nov 05, 2020 at 03:01:12PM +0200, Rémi Denis-Courmont wrote:
> Hi,
>
> This looks plain wrong. CR is not a special character, or specifically a line ending in a Unix CLI (main use of RC interface) or in netcat-type tools (secondary use with in Unix or TCP socket modes).
>
> Not to be confused with IETF protocols.
> I'm not sure about the Telnet protocol specifically, but VLC RC and VLC Telnet interfaces are different modules.
>
> Le 5 novembre 2020 13:01:49 GMT+02:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
> >When using cli with --rc-host and --rc-fake-tty to control the cli
> >interface remotely, and using telnet to connect to it, \r\n is appended
> >at the end of each command. The code was removing the \n but not the \r
> >character. This refactor trimming into a function for clarity and uses
> >it to clean the beginning (additional spaces) and end (additional
> >spaces
> >or line feed / carriage return) of the command.
> >---
> > modules/control/cli/cli.c | 28 +++++++++++++++++++++++-----
> > 1 file changed, 23 insertions(+), 5 deletions(-)
> >
> >diff --git a/modules/control/cli/cli.c b/modules/control/cli/cli.c
> >index c741e7fcad..2095d9b8e8 100644
> >--- a/modules/control/cli/cli.c
> >+++ b/modules/control/cli/cli.c
> >@@ -287,11 +287,9 @@ static void UnknownCmd(intf_thread_t *intf, const
> >char *const *args,
> >     (void) count;
> > }
> >
> >-static void Process(intf_thread_t *intf, const char *line)
> >+static void Process(intf_thread_t *intf, char *cmd)
> > {
> >     intf_sys_t *sys = intf->p_sys;
> >-    /* Skip heading spaces */
> >-    const char *cmd = line + strspn(line, " ");
> >
> >     if (*cmd == '\0')
> >         return; /* Ignore empty line */
> >@@ -349,6 +347,27 @@ error:      wordfree(&we);
> > #endif
> > }
> >
> >+static char *TrimLine(char *line)
> >+{
> >+    /* Skip heading spaces */
> >+    line = &line[strspn(line, " ")];
> >+
> >+    size_t length = strlen(line);
> >+
> >+    /* remove trailing CR/LF */
> >+    while (length > 0)
> >+    {
> >+        char c = line[length - 1];
> >+        if (c != '\r' && c != '\n' && c != ' ')
> >+            return line;
> >+
> >+        line[length - 1] = '\0';
> >+        length--;
> >+    }
> >+
> >+    return line;
> >+}
> >+
> > #ifndef _WIN32
> > static void *Run(void *data)
> > {
> >@@ -386,8 +405,7 @@ static void *Run(void *data)
> >         if (cmd != NULL)
> >         {
> >             int canc = vlc_savecancel();
> >-            if (cmd[0] != '\0')
> >-                cmd[strlen(cmd) - 1] = '\0'; /* remove trailing LF */
> >+            cmd = TrimLine(cmd);
> >             Process(intf, cmd);
> >             vlc_restorecancel(canc);
> >         }
> >--
> >2.29.2
> >
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel