shell completion somehow damages the shell history Topic is solved

[nikiwaibel]

unfort i have not yet figured out how or why it happens, but at least here is what happens:

commands "a", "bb", "ccc", "dddd" and "eeeee" have been entered (followed by <crlf>, obviously):

Code: Select all

Breakpoint 2, shellThread (p=0x200015c4 <shellConfig>) at ../../chibios-svn/os/various/shell/shell.c:368
368       chprintf(chp, SHELL_PROMPT_STR);
(gdb) x/32c shellHistoryBuffer
0x20007fb0 <shellHistoryBuffer>:   1 '\001'   97 'a'   2 '\002'   98 'b'   98 'b'   3 '\003'   99 'c'   99 'c'
0x20007fb8 <shellHistoryBuffer+8>:   99 'c'   4 '\004'   100 'd'   100 'd'   100 'd'   100 'd'   5 '\005'   101 'e'
0x20007fc0 <shellHistoryBuffer+16>:   101 'e'   101 'e'   101 'e'   101 'e'   0 '\000'   0 '\000'   0 '\000'   0 '\000'
0x20007fc8 <shellHistoryBuffer+24>:   0 '\000'   0 '\000'   0 '\000'   0 '\000'   0 '\000'   0 '\000'   0 '\000'   0 '\000'
(gdb) p shp->sh_beg
$5 = 0
(gdb) p shp->sh_cur
$6 = 0
(gdb) p shp->sh_end
$7 = 20

… the buffer looks fine (breakpoint at shell.c:366 [-r15302]).

now i type "e" and hit <tab> ("echo " appears) and <crlf>. history buffer is damaged:

Code: Select all

Breakpoint 2, shellThread (p=0x200015c4 <shellConfig>) at ../../chibios-svn/os/various/shell/shell.c:368
368       chprintf(chp, SHELL_PROMPT_STR);
(gdb) x/32c shellHistoryBuffer
0x20007fb0 <shellHistoryBuffer>:   0 '\000'   0 '\000'   0 '\000'   0 '\000'   98 'b'   3 '\003'   99 'c'   99 'c'
0x20007fb8 <shellHistoryBuffer+8>:   99 'c'   4 '\004'   100 'd'   100 'd'   100 'd'   100 'd'   5 '\005'   101 'e'
0x20007fc0 <shellHistoryBuffer+16>:   101 'e'   101 'e'   101 'e'   101 'e'   4 '\004'   101 'e'   99 'c'   104 'h'
0x20007fc8 <shellHistoryBuffer+24>:   111 'o'   0 '\000'   0 '\000'   0 '\000'   0 '\000'   0 '\000'   0 '\000'   0 '\000'
(gdb) p shp->sh_beg
$8 = 0
(gdb) p shp->sh_cur
$9 = 0
(gdb) p shp->sh_end
$10 = 25

… the end ist fine, but the start is bad by now.

next time you hit <up>, you end up in a loop at:

Code: Select all

^C
Program received signal SIGINT, Interrupt.
0x0800370a in get_history (shp=shp@entry=0x200078b4 <mShellThreadWorkingArea+4324>, line=line@entry=0x200078e4 <mShellThreadWorkingArea+4372> "echo", dir=dir@entry=0) at ../../chibios-svn/os/various/shell/shell.c:187
187       if (idx >= shp->sh_size)
(gdb) bt
#0  0x0800370a in get_history (shp=shp@entry=0x200078b4 <mShellThreadWorkingArea+4324>,
    line=line@entry=0x200078e4 <mShellThreadWorkingArea+4372> "echo", dir=dir@entry=0)
    at ../../chibios-svn/os/various/shell/shell.c:187
#1  0x08006584 in shellGetLine (scfg=scfg@entry=0x200015c4 <shellConfig>,
    line=line@entry=0x200078e4 <mShellThreadWorkingArea+4372> "echo", size=size@entry=64,
    shp=shp@entry=0x200078b4 <mShellThreadWorkingArea+4324>) at ../../chibios-svn/os/various/shell/shell.c:501
#2  0x08007bfa in shellThread (p=0x200015c4 <shellConfig>) at ../../chibios-svn/os/various/shell/shell.c:369
#3  0x08000346 in __port_thread_start () at ../../chibios-svn/os/common/ports/ARMv7-M/compilers/GCC/chcoreasm.S:203
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) list
182   
183     /* Count the number of lines saved in the buffer */
184     int idx = shp->sh_beg;
185     while (idx != shp->sh_end) {
186       idx += *(shp->sh_buffer + idx) + 1;
187       if (idx >= shp->sh_size)
188         idx -= shp->sh_size;
189       count++;
190     }
191   
(gdb) p count
$11 = 82121462

… count gets larger and larger.

[nikiwaibel]

hmmm, this may happen, if shellCompletion[] is allocated like this

Code: Select all

#if (SHELL_USE_COMPLETION == TRUE)
static char *shellCompletion[] = { NULL };
#endif
#if (SHELL_USE_HISTORY == TRUE)
static char shellHistoryBuffer[SHELL_MAX_HIST_BUFF];
#endif

now trying

Code: Select all

#if (SHELL_USE_COMPLETION == TRUE)
static char *shellCompletion[] = { NULL, NULL };
#endif
#if (SHELL_USE_HISTORY == TRUE)
static char shellHistoryBuffer[SHELL_MAX_HIST_BUFF];
#endif

…

[Giovanni]

Hi,

Thanks for finding, I never really looked into that feature, it is contributed code. If you find the problem just post the changes here.

Giovanni

[nikiwaibel]

i've not seen any issue since i've added the 2nd NULL. the shell-code seems to write to that location. (if 2nd NULL does not exist and the history buffer follows, the first bytes of the history buffer are badly overwritten - i think with 0x00000000).

the shell is a very handy feature. having completion (tab) for the commands and a history (cursor up/down) is really nice. implementing left/right cursors would be nice, but the code is heavily nested and very hard to read and understand (for me).

unfortunately, there is no/little documentation about the shell (i've found things mostly in some demos). and there is even less info about shell completion. hope these few lines here may help.

Code: Select all

#define SHELL_THREAD_NAME "shell"
#define MCU_DEVICE_USART_CONSOLE   SD2

static void mCmdFn(BaseSequentialStream *, int, char **);

Code: Select all

#if (SHELL_USE_COMPLETION == TRUE)
static char *mShellCompletion[] = { NULL, NULL };
#endif

#if (SHELL_USE_HISTORY == TRUE)
static char mShellHistoryBuffer[SHELL_MAX_HIST_BUFF];
#endif

static const ShellCommand mShellCommands[] = {
        { "mCmd", mCmdFn },
        { NULL,   NULL   },
};

Code: Select all

static const ShellConfig mShellConfig = {
        (BaseSequentialStream *)(&MCU_DEVICE_USART_CONSOLE),
        mShellCommands,
#if (SHELL_USE_HISTORY == TRUE)
        mShellHistoryBuffer,
        SHELL_MAX_HIST_BUFF,
#endif
#if (SHELL_USE_COMPLETION == TRUE)
        mShellCompletion,
#endif
};

Code: Select all

static THD_WORKING_AREA(mShellThreadWorkingArea, 4 * 1024);

static const thread_descriptor_t mShellThreadDescriptor = {
        .name   = SHELL_THREAD_NAME,
        .wbase  = THD_WORKING_AREA_BASE(mShellThreadWorkingArea),
        .wend   = THD_WORKING_AREA_END(mShellThreadWorkingArea),
        .prio   = NORMALPRIO - 1,
        .funcp  = shellThread,
        .arg    = &mShellConfig,
};

Code: Select all

static void
mCmdFn(BaseSequentialStream *chp, int argc, char *argv[])
{
        /* yourCode */
}

Code: Select all

shellInit();
mThreadShell = chThdCreate(&mShellThreadDescriptor);

terminating the shell properly is a funny thing as well, but i think we've gone quite far here anyway.

[Stephane]

Hello,

I use often the shell and use the history and completion options.
And also faced many times the same bug that freezes the command line after recalling an old command.
Apparently, this bug occurs after a command completion.

I tried the nikiwaibel fix (thanks), it improved the robustness.
But having a growing numbers of commands in my shell, i faced again the bug.

Here's my attempt to fix it:

Remainder: the config is declared as:

Code: Select all

const ShellConfig GL_My_Shell_Config_USB =              // Config for shell on USB
{
    PORT_USB,
    shellCmds,
    shell_histbuff_usb,                                 // Option if History is activated
    SHELL_MAX_HIST_BUFF,                                // Option if History is activated
    shell_complbuff_usb                                 // Option if Completion is activated
};

1) Change the declaration for the completion to:

Code: Select all

   static char *shell_complbuff_usb[SHELL_MAX_COMPLETIONS] = { NULL };

2) In shellconf.h, be sure that SHELL_MAX_COMPLETIONS greater or equal to the maximum number of occurrences.
Typically, think of the maximum number of commands starting with the same letter.
To be safe, you can put the total number of commands. (your commands + the internal shell commands (help, info, etc) )

3) Optional: bulletproof the function get_completion in shell.c to be safe in case of the value of SHELL_MAX_COMPLETIONS is not big enough.
This will limit the displayed completions but avoid writing non reserved memory.
The modifications (only additions) are marked by <---

Code: Select all

static void get_completions(ShellConfig *scfg, char *line) {
  const ShellCommand *lcp = shell_local_commands;
  const ShellCommand *scp = scfg->sc_commands;
  char **scmp = scfg->sc_completion;
  char help_cmp[] = "help";
  uint32_t compl_count = 0;                             // <---

  if (strstr(help_cmp, line) == help_cmp) {
    *scmp++ = help_cmp;
  }
  while (lcp->sc_name != NULL) {
    if (strstr(lcp->sc_name, line) == lcp->sc_name) {
      *scmp++ = (char *)lcp->sc_name;
      compl_count++;                                    // <---
    }
    lcp++;
  }
  if (scp != NULL) {
    while (scp->sc_name != NULL) {
      if (strstr(scp->sc_name, line) == scp->sc_name) {
        *scmp++ = (char *)scp->sc_name;
        compl_count++;                                  // <---
        if (compl_count >= SHELL_MAX_COMPLETIONS) {     // <---
          break;                                        // <---
        }                                               // <---
      }
      scp++;
    }
  }
  *scmp = NULL;
}

This has been tested few weeks on two custom boards, for the moment without problems.

I hope this helps.

Stephane

[Giovanni]

Hi,

Now there is also a new "xshell" in trunk, devs should try that, it is very improved.

The new shell will be back-ported to 21.x together with the trunk HAL in next weeks.

Giovanni