* [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git. @ 2018-01-24 0:38 Arvind Sankar 2018-01-24 20:54 ` Josh Poimboeuf 0 siblings, 1 reply; 8+ messages in thread From: Arvind Sankar @ 2018-01-24 0:38 UTC (permalink / raw) To: linux-kernel; +Cc: Greg Kroah-Hartman, Josh Poimboeuf We inherited this hack with the original code from the Git project. The select call is invalid as the two fd_set pointers should not be aliased. We could fix it, but the Git project removed this hack in 2012 in commit e8320f3 (pager: drop "wait for output to run less" hack). The bug it worked around was apparently fixed in less back in June 2007. So remove the hack from here as well. Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> --- tools/lib/subcmd/pager.c | 17 ----------------- tools/lib/subcmd/run-command.c | 2 -- tools/lib/subcmd/run-command.h | 1 - 3 files changed, 20 deletions(-) diff --git a/tools/lib/subcmd/pager.c b/tools/lib/subcmd/pager.c index 5ba754d17952..94d61d9b511f 100644 --- a/tools/lib/subcmd/pager.c +++ b/tools/lib/subcmd/pager.c @@ -1,5 +1,4 @@ // SPDX-License-Identifier: GPL-2.0 -#include <sys/select.h> #include <stdlib.h> #include <stdio.h> #include <string.h> @@ -23,21 +22,6 @@ void pager_init(const char *pager_env) subcmd_config.pager_env = pager_env; } -static void pager_preexec(void) -{ - /* - * Work around bug in "less" by not starting it until we - * have real input - */ - fd_set in; - - FD_ZERO(&in); - FD_SET(0, &in); - select(1, &in, NULL, &in, NULL); - - setenv("LESS", "FRSX", 0); -} - static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; static struct child_process pager_process; @@ -84,7 +68,6 @@ void setup_pager(void) pager_argv[2] = pager; pager_process.argv = pager_argv; pager_process.in = -1; - pager_process.preexec_cb = pager_preexec; if (start_command(&pager_process)) return; diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c index 5cdac2162532..9e9dca717ed7 100644 --- a/tools/lib/subcmd/run-command.c +++ b/tools/lib/subcmd/run-command.c @@ -120,8 +120,6 @@ int start_command(struct child_process *cmd) unsetenv(*cmd->env); } } - if (cmd->preexec_cb) - cmd->preexec_cb(); if (cmd->exec_cmd) { execv_cmd(cmd->argv); } else { diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h index 17d969c6add3..6256268802b5 100644 --- a/tools/lib/subcmd/run-command.h +++ b/tools/lib/subcmd/run-command.h @@ -46,7 +46,6 @@ struct child_process { unsigned no_stderr:1; unsigned exec_cmd:1; /* if this is to be external sub-command */ unsigned stdout_to_stderr:1; - void (*preexec_cb)(void); }; int start_command(struct child_process *); -- 2.13.6 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git. 2018-01-24 0:38 [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git Arvind Sankar @ 2018-01-24 20:54 ` Josh Poimboeuf 2018-01-25 8:16 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 8+ messages in thread From: Josh Poimboeuf @ 2018-01-24 20:54 UTC (permalink / raw) To: Arvind Sankar Cc: linux-kernel, Greg Kroah-Hartman, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo On Tue, Jan 23, 2018 at 07:38:37PM -0500, Arvind Sankar wrote: > We inherited this hack with the original code from the Git project. The > select call is invalid as the two fd_set pointers should not be aliased. > > We could fix it, but the Git project removed this hack in 2012 in commit > e8320f3 (pager: drop "wait for output to run less" hack). The bug it > worked around was apparently fixed in less back in June 2007. > > So remove the hack from here as well. > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> Looks good to me. Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> Libsubcmd is used by perf and objtool, so adding the perf maintainers to CC. Arnaldo, do you want to pick this one up? > --- > tools/lib/subcmd/pager.c | 17 ----------------- > tools/lib/subcmd/run-command.c | 2 -- > tools/lib/subcmd/run-command.h | 1 - > 3 files changed, 20 deletions(-) > > diff --git a/tools/lib/subcmd/pager.c b/tools/lib/subcmd/pager.c > index 5ba754d17952..94d61d9b511f 100644 > --- a/tools/lib/subcmd/pager.c > +++ b/tools/lib/subcmd/pager.c > @@ -1,5 +1,4 @@ > // SPDX-License-Identifier: GPL-2.0 > -#include <sys/select.h> > #include <stdlib.h> > #include <stdio.h> > #include <string.h> > @@ -23,21 +22,6 @@ void pager_init(const char *pager_env) > subcmd_config.pager_env = pager_env; > } > > -static void pager_preexec(void) > -{ > - /* > - * Work around bug in "less" by not starting it until we > - * have real input > - */ > - fd_set in; > - > - FD_ZERO(&in); > - FD_SET(0, &in); > - select(1, &in, NULL, &in, NULL); > - > - setenv("LESS", "FRSX", 0); > -} > - > static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; > static struct child_process pager_process; > > @@ -84,7 +68,6 @@ void setup_pager(void) > pager_argv[2] = pager; > pager_process.argv = pager_argv; > pager_process.in = -1; > - pager_process.preexec_cb = pager_preexec; > > if (start_command(&pager_process)) > return; > diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c > index 5cdac2162532..9e9dca717ed7 100644 > --- a/tools/lib/subcmd/run-command.c > +++ b/tools/lib/subcmd/run-command.c > @@ -120,8 +120,6 @@ int start_command(struct child_process *cmd) > unsetenv(*cmd->env); > } > } > - if (cmd->preexec_cb) > - cmd->preexec_cb(); > if (cmd->exec_cmd) { > execv_cmd(cmd->argv); > } else { > diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h > index 17d969c6add3..6256268802b5 100644 > --- a/tools/lib/subcmd/run-command.h > +++ b/tools/lib/subcmd/run-command.h > @@ -46,7 +46,6 @@ struct child_process { > unsigned no_stderr:1; > unsigned exec_cmd:1; /* if this is to be external sub-command */ > unsigned stdout_to_stderr:1; > - void (*preexec_cb)(void); > }; > > int start_command(struct child_process *); > -- > 2.13.6 > -- Josh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git. 2018-01-24 20:54 ` Josh Poimboeuf @ 2018-01-25 8:16 ` Arnaldo Carvalho de Melo 2018-01-25 13:24 ` Arvind Sankar 2018-02-09 0:00 ` Arvind Sankar 0 siblings, 2 replies; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-01-25 8:16 UTC (permalink / raw) To: Josh Poimboeuf Cc: Arvind Sankar, linux-kernel, Greg Kroah-Hartman, Peter Zijlstra, Ingo Molnar Em Wed, Jan 24, 2018 at 02:54:11PM -0600, Josh Poimboeuf escreveu: > On Tue, Jan 23, 2018 at 07:38:37PM -0500, Arvind Sankar wrote: > > We inherited this hack with the original code from the Git project. The > > select call is invalid as the two fd_set pointers should not be aliased. > > > > We could fix it, but the Git project removed this hack in 2012 in commit > > e8320f3 (pager: drop "wait for output to run less" hack). The bug it > > worked around was apparently fixed in less back in June 2007. > > > > So remove the hack from here as well. > > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> > > Looks good to me. > > Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> > > Libsubcmd is used by perf and objtool, so adding the perf maintainers to > CC. Arnaldo, do you want to pick this one up? Sure, I'll put it in my perf/core branch. - Arnaldo > > --- > > tools/lib/subcmd/pager.c | 17 ----------------- > > tools/lib/subcmd/run-command.c | 2 -- > > tools/lib/subcmd/run-command.h | 1 - > > 3 files changed, 20 deletions(-) > > > > diff --git a/tools/lib/subcmd/pager.c b/tools/lib/subcmd/pager.c > > index 5ba754d17952..94d61d9b511f 100644 > > --- a/tools/lib/subcmd/pager.c > > +++ b/tools/lib/subcmd/pager.c > > @@ -1,5 +1,4 @@ > > // SPDX-License-Identifier: GPL-2.0 > > -#include <sys/select.h> > > #include <stdlib.h> > > #include <stdio.h> > > #include <string.h> > > @@ -23,21 +22,6 @@ void pager_init(const char *pager_env) > > subcmd_config.pager_env = pager_env; > > } > > > > -static void pager_preexec(void) > > -{ > > - /* > > - * Work around bug in "less" by not starting it until we > > - * have real input > > - */ > > - fd_set in; > > - > > - FD_ZERO(&in); > > - FD_SET(0, &in); > > - select(1, &in, NULL, &in, NULL); > > - > > - setenv("LESS", "FRSX", 0); > > -} > > - > > static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; > > static struct child_process pager_process; > > > > @@ -84,7 +68,6 @@ void setup_pager(void) > > pager_argv[2] = pager; > > pager_process.argv = pager_argv; > > pager_process.in = -1; > > - pager_process.preexec_cb = pager_preexec; > > > > if (start_command(&pager_process)) > > return; > > diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c > > index 5cdac2162532..9e9dca717ed7 100644 > > --- a/tools/lib/subcmd/run-command.c > > +++ b/tools/lib/subcmd/run-command.c > > @@ -120,8 +120,6 @@ int start_command(struct child_process *cmd) > > unsetenv(*cmd->env); > > } > > } > > - if (cmd->preexec_cb) > > - cmd->preexec_cb(); > > if (cmd->exec_cmd) { > > execv_cmd(cmd->argv); > > } else { > > diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h > > index 17d969c6add3..6256268802b5 100644 > > --- a/tools/lib/subcmd/run-command.h > > +++ b/tools/lib/subcmd/run-command.h > > @@ -46,7 +46,6 @@ struct child_process { > > unsigned no_stderr:1; > > unsigned exec_cmd:1; /* if this is to be external sub-command */ > > unsigned stdout_to_stderr:1; > > - void (*preexec_cb)(void); > > }; > > > > int start_command(struct child_process *); > > -- > > 2.13.6 > > > > -- > Josh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git. 2018-01-25 8:16 ` Arnaldo Carvalho de Melo @ 2018-01-25 13:24 ` Arvind Sankar 2018-02-03 0:38 ` Arvind Sankar 2018-02-09 0:00 ` Arvind Sankar 1 sibling, 1 reply; 8+ messages in thread From: Arvind Sankar @ 2018-01-25 13:24 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Josh Poimboeuf, linux-kernel, Greg Kroah-Hartman, Peter Zijlstra, Ingo Molnar Thanks. This was found because gcc 8 appears to be enabling -Wrestrict in -Wall, so there is a build failure with mainline gcc. On Thu, Jan 25, 2018 at 05:16:52AM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, Jan 24, 2018 at 02:54:11PM -0600, Josh Poimboeuf escreveu: > > On Tue, Jan 23, 2018 at 07:38:37PM -0500, Arvind Sankar wrote: > > > We inherited this hack with the original code from the Git project. The > > > select call is invalid as the two fd_set pointers should not be aliased. > > > > > > We could fix it, but the Git project removed this hack in 2012 in commit > > > e8320f3 (pager: drop "wait for output to run less" hack). The bug it > > > worked around was apparently fixed in less back in June 2007. > > > > > > So remove the hack from here as well. > > > > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> > > > > Looks good to me. > > > > Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > Libsubcmd is used by perf and objtool, so adding the perf maintainers to > > CC. Arnaldo, do you want to pick this one up? > > Sure, I'll put it in my perf/core branch. > > - Arnaldo > > > > --- > > > tools/lib/subcmd/pager.c | 17 ----------------- > > > tools/lib/subcmd/run-command.c | 2 -- > > > tools/lib/subcmd/run-command.h | 1 - > > > 3 files changed, 20 deletions(-) > > > > > > diff --git a/tools/lib/subcmd/pager.c b/tools/lib/subcmd/pager.c > > > index 5ba754d17952..94d61d9b511f 100644 > > > --- a/tools/lib/subcmd/pager.c > > > +++ b/tools/lib/subcmd/pager.c > > > @@ -1,5 +1,4 @@ > > > // SPDX-License-Identifier: GPL-2.0 > > > -#include <sys/select.h> > > > #include <stdlib.h> > > > #include <stdio.h> > > > #include <string.h> > > > @@ -23,21 +22,6 @@ void pager_init(const char *pager_env) > > > subcmd_config.pager_env = pager_env; > > > } > > > > > > -static void pager_preexec(void) > > > -{ > > > - /* > > > - * Work around bug in "less" by not starting it until we > > > - * have real input > > > - */ > > > - fd_set in; > > > - > > > - FD_ZERO(&in); > > > - FD_SET(0, &in); > > > - select(1, &in, NULL, &in, NULL); > > > - > > > - setenv("LESS", "FRSX", 0); > > > -} > > > - > > > static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; > > > static struct child_process pager_process; > > > > > > @@ -84,7 +68,6 @@ void setup_pager(void) > > > pager_argv[2] = pager; > > > pager_process.argv = pager_argv; > > > pager_process.in = -1; > > > - pager_process.preexec_cb = pager_preexec; > > > > > > if (start_command(&pager_process)) > > > return; > > > diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c > > > index 5cdac2162532..9e9dca717ed7 100644 > > > --- a/tools/lib/subcmd/run-command.c > > > +++ b/tools/lib/subcmd/run-command.c > > > @@ -120,8 +120,6 @@ int start_command(struct child_process *cmd) > > > unsetenv(*cmd->env); > > > } > > > } > > > - if (cmd->preexec_cb) > > > - cmd->preexec_cb(); > > > if (cmd->exec_cmd) { > > > execv_cmd(cmd->argv); > > > } else { > > > diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h > > > index 17d969c6add3..6256268802b5 100644 > > > --- a/tools/lib/subcmd/run-command.h > > > +++ b/tools/lib/subcmd/run-command.h > > > @@ -46,7 +46,6 @@ struct child_process { > > > unsigned no_stderr:1; > > > unsigned exec_cmd:1; /* if this is to be external sub-command */ > > > unsigned stdout_to_stderr:1; > > > - void (*preexec_cb)(void); > > > }; > > > > > > int start_command(struct child_process *); > > > -- > > > 2.13.6 > > > > > > > -- > > Josh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git. 2018-01-25 13:24 ` Arvind Sankar @ 2018-02-03 0:38 ` Arvind Sankar 0 siblings, 0 replies; 8+ messages in thread From: Arvind Sankar @ 2018-02-03 0:38 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Josh Poimboeuf, linux-kernel, Greg Kroah-Hartman, Peter Zijlstra, Ingo Molnar, Sergey Senozhatsky Hi, it looks like Sergey has put in a patch to fix the aliasing, looking at the linux-next tree. Are we still looking to remove this hack altogether? Thanks On Thu, Jan 25, 2018 at 08:24:26AM -0500, Arvind Sankar wrote: > Thanks. > > This was found because gcc 8 appears to be enabling -Wrestrict in -Wall, > so there is a build failure with mainline gcc. > > On Thu, Jan 25, 2018 at 05:16:52AM -0300, Arnaldo Carvalho de Melo wrote: > > Em Wed, Jan 24, 2018 at 02:54:11PM -0600, Josh Poimboeuf escreveu: > > > On Tue, Jan 23, 2018 at 07:38:37PM -0500, Arvind Sankar wrote: > > > > We inherited this hack with the original code from the Git project. The > > > > select call is invalid as the two fd_set pointers should not be aliased. > > > > > > > > We could fix it, but the Git project removed this hack in 2012 in commit > > > > e8320f3 (pager: drop "wait for output to run less" hack). The bug it > > > > worked around was apparently fixed in less back in June 2007. > > > > > > > > So remove the hack from here as well. > > > > > > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> > > > > > > Looks good to me. > > > > > > Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > > > Libsubcmd is used by perf and objtool, so adding the perf maintainers to > > > CC. Arnaldo, do you want to pick this one up? > > > > Sure, I'll put it in my perf/core branch. > > > > - Arnaldo > > > > > > --- > > > > tools/lib/subcmd/pager.c | 17 ----------------- > > > > tools/lib/subcmd/run-command.c | 2 -- > > > > tools/lib/subcmd/run-command.h | 1 - > > > > 3 files changed, 20 deletions(-) > > > > > > > > diff --git a/tools/lib/subcmd/pager.c b/tools/lib/subcmd/pager.c > > > > index 5ba754d17952..94d61d9b511f 100644 > > > > --- a/tools/lib/subcmd/pager.c > > > > +++ b/tools/lib/subcmd/pager.c > > > > @@ -1,5 +1,4 @@ > > > > // SPDX-License-Identifier: GPL-2.0 > > > > -#include <sys/select.h> > > > > #include <stdlib.h> > > > > #include <stdio.h> > > > > #include <string.h> > > > > @@ -23,21 +22,6 @@ void pager_init(const char *pager_env) > > > > subcmd_config.pager_env = pager_env; > > > > } > > > > > > > > -static void pager_preexec(void) > > > > -{ > > > > - /* > > > > - * Work around bug in "less" by not starting it until we > > > > - * have real input > > > > - */ > > > > - fd_set in; > > > > - > > > > - FD_ZERO(&in); > > > > - FD_SET(0, &in); > > > > - select(1, &in, NULL, &in, NULL); > > > > - > > > > - setenv("LESS", "FRSX", 0); > > > > -} > > > > - > > > > static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; > > > > static struct child_process pager_process; > > > > > > > > @@ -84,7 +68,6 @@ void setup_pager(void) > > > > pager_argv[2] = pager; > > > > pager_process.argv = pager_argv; > > > > pager_process.in = -1; > > > > - pager_process.preexec_cb = pager_preexec; > > > > > > > > if (start_command(&pager_process)) > > > > return; > > > > diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c > > > > index 5cdac2162532..9e9dca717ed7 100644 > > > > --- a/tools/lib/subcmd/run-command.c > > > > +++ b/tools/lib/subcmd/run-command.c > > > > @@ -120,8 +120,6 @@ int start_command(struct child_process *cmd) > > > > unsetenv(*cmd->env); > > > > } > > > > } > > > > - if (cmd->preexec_cb) > > > > - cmd->preexec_cb(); > > > > if (cmd->exec_cmd) { > > > > execv_cmd(cmd->argv); > > > > } else { > > > > diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h > > > > index 17d969c6add3..6256268802b5 100644 > > > > --- a/tools/lib/subcmd/run-command.h > > > > +++ b/tools/lib/subcmd/run-command.h > > > > @@ -46,7 +46,6 @@ struct child_process { > > > > unsigned no_stderr:1; > > > > unsigned exec_cmd:1; /* if this is to be external sub-command */ > > > > unsigned stdout_to_stderr:1; > > > > - void (*preexec_cb)(void); > > > > }; > > > > > > > > int start_command(struct child_process *); > > > > -- > > > > 2.13.6 > > > > > > > > > > -- > > > Josh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git. 2018-01-25 8:16 ` Arnaldo Carvalho de Melo 2018-01-25 13:24 ` Arvind Sankar @ 2018-02-09 0:00 ` Arvind Sankar 2018-02-09 1:17 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 8+ messages in thread From: Arvind Sankar @ 2018-02-09 0:00 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Josh Poimboeuf, linux-kernel, Greg Kroah-Hartman, Peter Zijlstra, Ingo Molnar We inherited this hack with the original code from the Git project. The select call is invalid as the two fd_set pointers should not be aliased. We could fix it, but the Git project removed this hack in 2012 in commit e8320f3 (pager: drop "wait for output to run less" hack). The bug it worked around was apparently fixed in less back in June 2007. So remove the hack from here as well. Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> --- - Merge with commit ad343a98 (tools/lib/subcmd/pager.c: do not alias select() params) by Sergey tools/lib/subcmd/pager.c | 20 -------------------- tools/lib/subcmd/run-command.c | 2 -- tools/lib/subcmd/run-command.h | 1 - 3 files changed, 23 deletions(-) diff --git a/tools/lib/subcmd/pager.c b/tools/lib/subcmd/pager.c index 9997a8805a82..94d61d9b511f 100644 --- a/tools/lib/subcmd/pager.c +++ b/tools/lib/subcmd/pager.c @@ -1,5 +1,4 @@ // SPDX-License-Identifier: GPL-2.0 -#include <sys/select.h> #include <stdlib.h> #include <stdio.h> #include <string.h> @@ -23,24 +22,6 @@ void pager_init(const char *pager_env) subcmd_config.pager_env = pager_env; } -static void pager_preexec(void) -{ - /* - * Work around bug in "less" by not starting it until we - * have real input - */ - fd_set in; - fd_set exception; - - FD_ZERO(&in); - FD_ZERO(&exception); - FD_SET(0, &in); - FD_SET(0, &exception); - select(1, &in, NULL, &exception, NULL); - - setenv("LESS", "FRSX", 0); -} - static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; static struct child_process pager_process; @@ -87,7 +68,6 @@ void setup_pager(void) pager_argv[2] = pager; pager_process.argv = pager_argv; pager_process.in = -1; - pager_process.preexec_cb = pager_preexec; if (start_command(&pager_process)) return; diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c index 5cdac2162532..9e9dca717ed7 100644 --- a/tools/lib/subcmd/run-command.c +++ b/tools/lib/subcmd/run-command.c @@ -120,8 +120,6 @@ int start_command(struct child_process *cmd) unsetenv(*cmd->env); } } - if (cmd->preexec_cb) - cmd->preexec_cb(); if (cmd->exec_cmd) { execv_cmd(cmd->argv); } else { diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h index 17d969c6add3..6256268802b5 100644 --- a/tools/lib/subcmd/run-command.h +++ b/tools/lib/subcmd/run-command.h @@ -46,7 +46,6 @@ struct child_process { unsigned no_stderr:1; unsigned exec_cmd:1; /* if this is to be external sub-command */ unsigned stdout_to_stderr:1; - void (*preexec_cb)(void); }; int start_command(struct child_process *); -- 2.13.6 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git. 2018-02-09 0:00 ` Arvind Sankar @ 2018-02-09 1:17 ` Arnaldo Carvalho de Melo 2018-04-20 1:12 ` Arvind Sankar 0 siblings, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-02-09 1:17 UTC (permalink / raw) To: Arvind Sankar Cc: Josh Poimboeuf, linux-kernel, Greg Kroah-Hartman, Peter Zijlstra, Ingo Molnar Em Thu, Feb 08, 2018 at 07:00:10PM -0500, Arvind Sankar escreveu: > We inherited this hack with the original code from the Git project. The > select call is invalid as the two fd_set pointers should not be aliased. > > We could fix it, but the Git project removed this hack in 2012 in commit > e8320f3 (pager: drop "wait for output to run less" hack). The bug it > worked around was apparently fixed in less back in June 2007. > > So remove the hack from here as well. Ok, so e8320f3 was merged directly by Linus, I'll wait till perf/urgent pulls from upstream and then will merge this, Thanks, - Arnaldo > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> > --- > - Merge with commit ad343a98 (tools/lib/subcmd/pager.c: do not alias > select() params) by Sergey > > tools/lib/subcmd/pager.c | 20 -------------------- > tools/lib/subcmd/run-command.c | 2 -- > tools/lib/subcmd/run-command.h | 1 - > 3 files changed, 23 deletions(-) > > diff --git a/tools/lib/subcmd/pager.c b/tools/lib/subcmd/pager.c > index 9997a8805a82..94d61d9b511f 100644 > --- a/tools/lib/subcmd/pager.c > +++ b/tools/lib/subcmd/pager.c > @@ -1,5 +1,4 @@ > // SPDX-License-Identifier: GPL-2.0 > -#include <sys/select.h> > #include <stdlib.h> > #include <stdio.h> > #include <string.h> > @@ -23,24 +22,6 @@ void pager_init(const char *pager_env) > subcmd_config.pager_env = pager_env; > } > > -static void pager_preexec(void) > -{ > - /* > - * Work around bug in "less" by not starting it until we > - * have real input > - */ > - fd_set in; > - fd_set exception; > - > - FD_ZERO(&in); > - FD_ZERO(&exception); > - FD_SET(0, &in); > - FD_SET(0, &exception); > - select(1, &in, NULL, &exception, NULL); > - > - setenv("LESS", "FRSX", 0); > -} > - > static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; > static struct child_process pager_process; > > @@ -87,7 +68,6 @@ void setup_pager(void) > pager_argv[2] = pager; > pager_process.argv = pager_argv; > pager_process.in = -1; > - pager_process.preexec_cb = pager_preexec; > > if (start_command(&pager_process)) > return; > diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c > index 5cdac2162532..9e9dca717ed7 100644 > --- a/tools/lib/subcmd/run-command.c > +++ b/tools/lib/subcmd/run-command.c > @@ -120,8 +120,6 @@ int start_command(struct child_process *cmd) > unsetenv(*cmd->env); > } > } > - if (cmd->preexec_cb) > - cmd->preexec_cb(); > if (cmd->exec_cmd) { > execv_cmd(cmd->argv); > } else { > diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h > index 17d969c6add3..6256268802b5 100644 > --- a/tools/lib/subcmd/run-command.h > +++ b/tools/lib/subcmd/run-command.h > @@ -46,7 +46,6 @@ struct child_process { > unsigned no_stderr:1; > unsigned exec_cmd:1; /* if this is to be external sub-command */ > unsigned stdout_to_stderr:1; > - void (*preexec_cb)(void); > }; > > int start_command(struct child_process *); > -- > 2.13.6 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git. 2018-02-09 1:17 ` Arnaldo Carvalho de Melo @ 2018-04-20 1:12 ` Arvind Sankar 0 siblings, 0 replies; 8+ messages in thread From: Arvind Sankar @ 2018-04-20 1:12 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Josh Poimboeuf, linux-kernel, Greg Kroah-Hartman, Peter Zijlstra, Ingo Molnar [-- Attachment #1: Type: text/plain, Size: 3535 bytes --] Hey did this ever make it in? On Thu, Feb 8, 2018 at 8:17 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > Em Thu, Feb 08, 2018 at 07:00:10PM -0500, Arvind Sankar escreveu: > > We inherited this hack with the original code from the Git project. The > > select call is invalid as the two fd_set pointers should not be aliased. > > > > We could fix it, but the Git project removed this hack in 2012 in commit > > e8320f3 (pager: drop "wait for output to run less" hack). The bug it > > worked around was apparently fixed in less back in June 2007. > > > > So remove the hack from here as well. > > Ok, so e8320f3 was merged directly by Linus, I'll wait till perf/urgent > pulls from upstream and then will merge this, > > Thanks, > > - Arnaldo > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> > > --- > > - Merge with commit ad343a98 (tools/lib/subcmd/pager.c: do not alias > > select() params) by Sergey > > > > tools/lib/subcmd/pager.c | 20 -------------------- > > tools/lib/subcmd/run-command.c | 2 -- > > tools/lib/subcmd/run-command.h | 1 - > > 3 files changed, 23 deletions(-) > > > > diff --git a/tools/lib/subcmd/pager.c b/tools/lib/subcmd/pager.c > > index 9997a8805a82..94d61d9b511f 100644 > > --- a/tools/lib/subcmd/pager.c > > +++ b/tools/lib/subcmd/pager.c > > @@ -1,5 +1,4 @@ > > // SPDX-License-Identifier: GPL-2.0 > > -#include <sys/select.h> > > #include <stdlib.h> > > #include <stdio.h> > > #include <string.h> > > @@ -23,24 +22,6 @@ void pager_init(const char *pager_env) > > subcmd_config.pager_env = pager_env; > > } > > > > -static void pager_preexec(void) > > -{ > > - /* > > - * Work around bug in "less" by not starting it until we > > - * have real input > > - */ > > - fd_set in; > > - fd_set exception; > > - > > - FD_ZERO(&in); > > - FD_ZERO(&exception); > > - FD_SET(0, &in); > > - FD_SET(0, &exception); > > - select(1, &in, NULL, &exception, NULL); > > - > > - setenv("LESS", "FRSX", 0); > > -} > > - > > static const char *pager_argv[] = { "sh", "-c", NULL, NULL }; > > static struct child_process pager_process; > > > > @@ -87,7 +68,6 @@ void setup_pager(void) > > pager_argv[2] = pager; > > pager_process.argv = pager_argv; > > pager_process.in = -1; > > - pager_process.preexec_cb = pager_preexec; > > > > if (start_command(&pager_process)) > > return; > > diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run- > command.c > > index 5cdac2162532..9e9dca717ed7 100644 > > --- a/tools/lib/subcmd/run-command.c > > +++ b/tools/lib/subcmd/run-command.c > > @@ -120,8 +120,6 @@ int start_command(struct child_process *cmd) > > unsetenv(*cmd->env); > > } > > } > > - if (cmd->preexec_cb) > > - cmd->preexec_cb(); > > if (cmd->exec_cmd) { > > execv_cmd(cmd->argv); > > } else { > > diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run- > command.h > > index 17d969c6add3..6256268802b5 100644 > > --- a/tools/lib/subcmd/run-command.h > > +++ b/tools/lib/subcmd/run-command.h > > @@ -46,7 +46,6 @@ struct child_process { > > unsigned no_stderr:1; > > unsigned exec_cmd:1; /* if this is to be external sub-command */ > > unsigned stdout_to_stderr:1; > > - void (*preexec_cb)(void); > > }; > > > > int start_command(struct child_process *); > > -- > > 2.13.6 > [-- Attachment #2: Type: text/html, Size: 4951 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-20 1:12 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-24 0:38 [PATCH] tools: libsubcmd: Drop the less hack that was inherited from Git Arvind Sankar 2018-01-24 20:54 ` Josh Poimboeuf 2018-01-25 8:16 ` Arnaldo Carvalho de Melo 2018-01-25 13:24 ` Arvind Sankar 2018-02-03 0:38 ` Arvind Sankar 2018-02-09 0:00 ` Arvind Sankar 2018-02-09 1:17 ` Arnaldo Carvalho de Melo 2018-04-20 1:12 ` Arvind Sankar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).