Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apps/system: replace CONFIG_NSH_LINELEN to LINE_MAX #2918

Closed
wants to merge 4 commits into from
Closed

apps/system: replace CONFIG_NSH_LINELEN to LINE_MAX #2918

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
73220c3
nshlib/cn_line: replace CONFIG_NSH_LINELEN to sizeof
anchao Dec 27, 2024
bff158b
nshlib/traceline: move traceline buffer from stack to heap
anchao Dec 27, 2024
18538c4
nshlib/nshline: move all temp line buffer to heap
anchao Dec 27, 2024
ae0325a
apps/system: replace CONFIG_NSH_LINELEN to LINE_MAX
anchao Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions nshlib/nsh_console.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,14 @@ struct nsh_vtbl_s
char traceline[CONFIG_NSH_LINELEN];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but traceline is used only when launching program, isn't good to keep the buffer in the whole nsh life cycle

#endif

/* Temporary line buffer */

#if (!defined(CONFIG_NSH_DISABLE_MEMDUMP) && defined(NSH_HAVE_WRITEFILE)) || \
!defined(CONFIG_NSH_DISABLEBG) || defined(CONFIG_NSH_PIPELINE) || \
!defined(CONFIG_NSH_DISABLE_WATCH)
char templine[CONFIG_NSH_LINELEN];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this will increase the heap consumption for the temp use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared with irregular heap access, shared template and will make it more deterministic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you split the rename from moving buffer to nsh_vtbl?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the temp buffer is only used in few command, why move it to nsh_vtbl to extend the life to the whole session?

#endif

/* Current working directory */

#ifdef CONFIG_DISABLE_ENVIRON
Expand Down
10 changes: 5 additions & 5 deletions nshlib/nsh_mmcmds.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,11 @@ int cmd_free(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv)

int cmd_memdump(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv)
{
char arg[CONFIG_NSH_LINELEN] = "";
int i;

if (argc == 1)
{
strlcpy(arg, "used", CONFIG_NSH_LINELEN);
strlcpy(vtbl->templine, "used", sizeof(vtbl->templine));
}
else if (argc >= 2 && (strcmp(argv[1], "-h") == 0 ||
strcmp(argv[1], "help") == 0))
Expand All @@ -73,15 +72,16 @@ int cmd_memdump(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv)
{
for (i = 1; i < argc; i++)
{
strlcat(arg, argv[i], CONFIG_NSH_LINELEN);
strlcat(vtbl->templine, argv[i], sizeof(vtbl->templine));
if (i < argc - 1)
{
strlcat(arg, " ", CONFIG_NSH_LINELEN);
strlcat(vtbl->templine, " ", sizeof(vtbl->templine));
}
}
}

return nsh_writefile(vtbl, argv[0], arg, strlen(arg),
return nsh_writefile(vtbl, argv[0], vtbl->templine,
strlen(vtbl->templine),
CONFIG_NSH_PROC_MOUNTPOINT "/memdump");
}

Expand Down
21 changes: 10 additions & 11 deletions nshlib/nsh_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,25 +605,24 @@ static int nsh_execute(FAR struct nsh_vtbl_s *vtbl,
{
FAR char *sh_argv[4];
FAR char *sh_cmd = "sh";
char sh_arg2[CONFIG_NSH_LINELEN];

DEBUGASSERT(strncmp(argv[0], sh_cmd, 3) != 0);

sh_arg2[0] = '\0';
vtbl->templine[0] = '\0';

for (ret = 0; ret < argc; ret++)
{
strlcat(sh_arg2, argv[ret], sizeof(sh_arg2));
strlcat(vtbl->templine, argv[ret], sizeof(vtbl->templine));

if (ret < argc - 1)
{
strcat(sh_arg2, " ");
strcat(vtbl->templine, " ");
}
}

sh_argv[0] = sh_cmd;
sh_argv[1] = "-c";
sh_argv[2] = sh_arg2;
sh_argv[2] = vtbl->templine;
sh_argv[3] = NULL;

/* np.np_bg still there, try use nsh_builtin or nsh_fileapp to
Expand Down Expand Up @@ -2466,7 +2465,7 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline)

/* Initialize parser state */

memset(argv, 0, MAX_ARGV_ENTRIES*sizeof(FAR char *));
memset(argv, 0, MAX_ARGV_ENTRIES * sizeof(FAR char *));
NSH_MEMLIST_INIT(memlist);
NSH_ALIASLIST_INIT(alist);

Expand Down Expand Up @@ -2681,7 +2680,6 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline)
{
FAR char *arg;
FAR char *sh_argv[4];
char sh_arg2[CONFIG_NSH_LINELEN];

if (argv[argc][g_pipeline1_len])
{
Expand All @@ -2699,21 +2697,22 @@ static int nsh_parse_command(FAR struct nsh_vtbl_s *vtbl, FAR char *cmdline)
goto dynlist_free;
}

sh_arg2[0] = '\0';
vtbl->templine[0] = '\0';

for (ret = 0; ret < argc; ret++)
{
strlcat(sh_arg2, argv[ret], sizeof(sh_arg2));
strlcat(vtbl->templine, argv[ret],
sizeof(vtbl->templine));

if (ret < argc - 1)
{
strcat(sh_arg2, " ");
strcat(vtbl->templine, " ");
}
}

sh_argv[0] = "sh";
sh_argv[1] = "-c";
sh_argv[2] = sh_arg2;
sh_argv[2] = vtbl->templine;
sh_argv[3] = NULL;

ret = pipe2(pipefd, 0);
Expand Down
5 changes: 2 additions & 3 deletions nshlib/nsh_timcmds.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,6 @@ int cmd_timedatectl(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv)
#ifndef CONFIG_NSH_DISABLE_WATCH
int cmd_watch(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv)
{
char buffer[CONFIG_NSH_LINELEN];
int interval = 2;
int count = -1;
FAR char *cmd;
Expand Down Expand Up @@ -593,8 +592,8 @@ int cmd_watch(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv)

for (i = 0; i < count; i++)
{
strlcpy(buffer, cmd, CONFIG_NSH_LINELEN);
ret = nsh_parse(vtbl, buffer);
strlcpy(vtbl->templine, cmd, sizeof(vtbl->templine));
ret = nsh_parse(vtbl, vtbl->templine);
if (ret < 0)
{
nsh_error(vtbl, g_fmtcmdfailed, argv[0], cmd, NSH_ERRNO);
Expand Down