git-commit-vandalism/pager.c

251 lines
5.1 KiB
C
Raw Normal View History

#include "cache.h"
#include "config.h"
#include "run-command.h"
#include "sigchain.h"
#include "alias.h"
#ifndef DEFAULT_PAGER
#define DEFAULT_PAGER "less"
#endif
static struct child_process pager_process = CHILD_PROCESS_INIT;
static const char *pager_program;
pager: don't use unsafe functions in signal handlers Since the commit a3da8821208d (pager: do wait_for_pager on signal death), we call wait_for_pager() in the pager's signal handler. The recent bug report revealed that this causes a deadlock in glibc at aborting "git log" [*1*]. When this happens, git process is left unterminated, and it can't be killed by SIGTERM but only by SIGKILL. The problem is that wait_for_pager() function does more than waiting for pager process's termination, but it does cleanups and printing errors. Unfortunately, the functions that may be used in a signal handler are very limited [*2*]. Particularly, malloc(), free() and the variants can't be used in a signal handler because they take a mutex internally in glibc. This was the cause of the deadlock above. Other than the direct calls of malloc/free, many functions calling malloc/free can't be used. strerror() is such one, either. Also the usage of fflush() and printf() in a signal handler is bad, although it seems working so far. In a safer side, we should avoid them, too. This patch tries to reduce the calls of such functions in signal handlers. wait_for_signal() takes a flag and avoids the unsafe calls. Also, finish_command_in_signal() is introduced for the same reason. There the free() calls are removed, and only waits for the children without whining at errors. [*1*] https://bugzilla.opensuse.org/show_bug.cgi?id=942297 [*2*] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03 Signed-off-by: Takashi Iwai <tiwai@suse.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-04 11:35:57 +02:00
static void wait_for_pager(int in_signal)
{
pager: don't use unsafe functions in signal handlers Since the commit a3da8821208d (pager: do wait_for_pager on signal death), we call wait_for_pager() in the pager's signal handler. The recent bug report revealed that this causes a deadlock in glibc at aborting "git log" [*1*]. When this happens, git process is left unterminated, and it can't be killed by SIGTERM but only by SIGKILL. The problem is that wait_for_pager() function does more than waiting for pager process's termination, but it does cleanups and printing errors. Unfortunately, the functions that may be used in a signal handler are very limited [*2*]. Particularly, malloc(), free() and the variants can't be used in a signal handler because they take a mutex internally in glibc. This was the cause of the deadlock above. Other than the direct calls of malloc/free, many functions calling malloc/free can't be used. strerror() is such one, either. Also the usage of fflush() and printf() in a signal handler is bad, although it seems working so far. In a safer side, we should avoid them, too. This patch tries to reduce the calls of such functions in signal handlers. wait_for_signal() takes a flag and avoids the unsafe calls. Also, finish_command_in_signal() is introduced for the same reason. There the free() calls are removed, and only waits for the children without whining at errors. [*1*] https://bugzilla.opensuse.org/show_bug.cgi?id=942297 [*2*] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03 Signed-off-by: Takashi Iwai <tiwai@suse.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-04 11:35:57 +02:00
if (!in_signal) {
fflush(stdout);
fflush(stderr);
}
/* signal EOF to pager */
close(1);
close(2);
pager: don't use unsafe functions in signal handlers Since the commit a3da8821208d (pager: do wait_for_pager on signal death), we call wait_for_pager() in the pager's signal handler. The recent bug report revealed that this causes a deadlock in glibc at aborting "git log" [*1*]. When this happens, git process is left unterminated, and it can't be killed by SIGTERM but only by SIGKILL. The problem is that wait_for_pager() function does more than waiting for pager process's termination, but it does cleanups and printing errors. Unfortunately, the functions that may be used in a signal handler are very limited [*2*]. Particularly, malloc(), free() and the variants can't be used in a signal handler because they take a mutex internally in glibc. This was the cause of the deadlock above. Other than the direct calls of malloc/free, many functions calling malloc/free can't be used. strerror() is such one, either. Also the usage of fflush() and printf() in a signal handler is bad, although it seems working so far. In a safer side, we should avoid them, too. This patch tries to reduce the calls of such functions in signal handlers. wait_for_signal() takes a flag and avoids the unsafe calls. Also, finish_command_in_signal() is introduced for the same reason. There the free() calls are removed, and only waits for the children without whining at errors. [*1*] https://bugzilla.opensuse.org/show_bug.cgi?id=942297 [*2*] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03 Signed-off-by: Takashi Iwai <tiwai@suse.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-04 11:35:57 +02:00
if (in_signal)
finish_command_in_signal(&pager_process);
else
finish_command(&pager_process);
}
static void wait_for_pager_atexit(void)
{
wait_for_pager(0);
}
static void wait_for_pager_signal(int signo)
{
pager: don't use unsafe functions in signal handlers Since the commit a3da8821208d (pager: do wait_for_pager on signal death), we call wait_for_pager() in the pager's signal handler. The recent bug report revealed that this causes a deadlock in glibc at aborting "git log" [*1*]. When this happens, git process is left unterminated, and it can't be killed by SIGTERM but only by SIGKILL. The problem is that wait_for_pager() function does more than waiting for pager process's termination, but it does cleanups and printing errors. Unfortunately, the functions that may be used in a signal handler are very limited [*2*]. Particularly, malloc(), free() and the variants can't be used in a signal handler because they take a mutex internally in glibc. This was the cause of the deadlock above. Other than the direct calls of malloc/free, many functions calling malloc/free can't be used. strerror() is such one, either. Also the usage of fflush() and printf() in a signal handler is bad, although it seems working so far. In a safer side, we should avoid them, too. This patch tries to reduce the calls of such functions in signal handlers. wait_for_signal() takes a flag and avoids the unsafe calls. Also, finish_command_in_signal() is introduced for the same reason. There the free() calls are removed, and only waits for the children without whining at errors. [*1*] https://bugzilla.opensuse.org/show_bug.cgi?id=942297 [*2*] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03 Signed-off-by: Takashi Iwai <tiwai@suse.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-04 11:35:57 +02:00
wait_for_pager(1);
sigchain_pop(signo);
raise(signo);
}
static int core_pager_config(const char *var, const char *value, void *data)
{
if (!strcmp(var, "core.pager"))
return git_config_string(&pager_program, var, value);
return 0;
}
const char *git_pager(int stdout_is_tty)
{
const char *pager;
if (!stdout_is_tty)
return NULL;
pager = getenv("GIT_PAGER");
if (!pager) {
if (!pager_program)
read_early_config(core_pager_config, NULL);
pager = pager_program;
}
if (!pager)
pager = getenv("PAGER");
if (!pager)
pager = DEFAULT_PAGER;
if (!*pager || !strcmp(pager, "cat"))
pager = NULL;
return pager;
}
static void setup_pager_env(struct argv_array *env)
{
const char **argv;
int i;
char *pager_env = xstrdup(PAGER_ENV);
int n = split_cmdline(pager_env, &argv);
if (n < 0)
die("malformed build-time PAGER_ENV: %s",
split_cmdline_strerror(n));
for (i = 0; i < n; i++) {
char *cp = strchr(argv[i], '=');
if (!cp)
die("malformed build-time PAGER_ENV");
*cp = '\0';
if (!getenv(argv[i])) {
*cp = '=';
argv_array_push(env, argv[i]);
}
}
free(pager_env);
free(argv);
}
void prepare_pager_args(struct child_process *pager_process, const char *pager)
{
argv_array_push(&pager_process->args, pager);
pager_process->use_shell = 1;
setup_pager_env(&pager_process->env_array);
pager_process->trace2_child_class = "pager";
}
void setup_pager(void)
{
const char *pager = git_pager(isatty(1));
if (!pager)
return;
/*
* After we redirect standard output, we won't be able to use an ioctl
* to get the terminal size. Let's grab it now, and then set $COLUMNS
* to communicate it to any sub-processes.
*/
{
char buf[64];
xsnprintf(buf, sizeof(buf), "%d", term_columns());
setenv("COLUMNS", buf, 0);
}
setenv("GIT_PAGER_IN_USE", "true", 1);
/* spawn the pager */
prepare_pager_args(&pager_process, pager);
pager_process.in = -1;
argv_array_push(&pager_process.env_array, "GIT_PAGER_IN_USE");
if (start_command(&pager_process))
return;
/* original process continues, but writes to the pipe */
dup2(pager_process.in, 1);
if (isatty(2))
dup2(pager_process.in, 2);
close(pager_process.in);
/* this makes sure that the parent terminates after the pager */
sigchain_push_common(wait_for_pager_signal);
pager: don't use unsafe functions in signal handlers Since the commit a3da8821208d (pager: do wait_for_pager on signal death), we call wait_for_pager() in the pager's signal handler. The recent bug report revealed that this causes a deadlock in glibc at aborting "git log" [*1*]. When this happens, git process is left unterminated, and it can't be killed by SIGTERM but only by SIGKILL. The problem is that wait_for_pager() function does more than waiting for pager process's termination, but it does cleanups and printing errors. Unfortunately, the functions that may be used in a signal handler are very limited [*2*]. Particularly, malloc(), free() and the variants can't be used in a signal handler because they take a mutex internally in glibc. This was the cause of the deadlock above. Other than the direct calls of malloc/free, many functions calling malloc/free can't be used. strerror() is such one, either. Also the usage of fflush() and printf() in a signal handler is bad, although it seems working so far. In a safer side, we should avoid them, too. This patch tries to reduce the calls of such functions in signal handlers. wait_for_signal() takes a flag and avoids the unsafe calls. Also, finish_command_in_signal() is introduced for the same reason. There the free() calls are removed, and only waits for the children without whining at errors. [*1*] https://bugzilla.opensuse.org/show_bug.cgi?id=942297 [*2*] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03 Signed-off-by: Takashi Iwai <tiwai@suse.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-04 11:35:57 +02:00
atexit(wait_for_pager_atexit);
}
int pager_in_use(void)
{
return git_env_bool("GIT_PAGER_IN_USE", 0);
}
/*
* Return cached value (if set) or $COLUMNS environment variable (if
* set and positive) or ioctl(1, TIOCGWINSZ).ws_col (if positive),
* and default to 80 if all else fails.
*/
int term_columns(void)
{
static int term_columns_at_startup;
char *col_string;
int n_cols;
if (term_columns_at_startup)
return term_columns_at_startup;
term_columns_at_startup = 80;
col_string = getenv("COLUMNS");
if (col_string && (n_cols = atoi(col_string)) > 0)
term_columns_at_startup = n_cols;
#ifdef TIOCGWINSZ
else {
struct winsize ws;
if (!ioctl(1, TIOCGWINSZ, &ws) && ws.ws_col)
term_columns_at_startup = ws.ws_col;
}
#endif
return term_columns_at_startup;
}
pager: add a helper function to clear the last line in the terminal There are a couple of places where we want to clear the last line on the terminal, e.g. when a progress bar line is overwritten by a shorter line, then the end of that progress line would remain visible, unless we cover it up. In 'progress.c' we did this by always appending a fixed number of space characters to the next line (even if it was not shorter than the previous), but as it turned out that fixed number was not quite large enough, see the fix in 9f1fd84e15 (progress: clear previous progress update dynamically, 2019-04-12). From then on we've been keeping track of the length of the last displayed progress line and appending the appropriate number of space characters to the next line, if necessary, but, alas, this approach turned out to be error prone, see the fix in 1aed1a5f25 (progress: avoid empty line when breaking the progress line, 2019-05-19). The next patch in this series is about to fix a case where we don't clear the last line, and on occasion do end up with such garbage at the end of the line. It would be great if we could do that without the need to deal with that without meticulously computing the necessary number of space characters. So add a helper function to clear the last line on the terminal using an ANSI escape sequence, which has the advantage to clear the whole line no matter how wide it is, even after the terminal width changed. Such an escape sequence is not available on dumb terminals, though, so in that case fall back to simply print a whole terminal width (as reported by term_columns()) worth of space characters. In 'editor.c' launch_specified_editor() already used this ANSI escape sequence, so replace it with a call to this function. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-24 20:13:16 +02:00
/*
* Clear the entire line, leave cursor in first column.
*/
void term_clear_line(void)
{
if (is_terminal_dumb())
/*
* Fall back to print a terminal width worth of space
* characters (hoping that the terminal is still as wide
* as it was upon the first call to term_columns()).
*/
fprintf(stderr, "\r%*s\r", term_columns(), "");
else
/*
* On non-dumb terminals use an escape sequence to clear
* the whole line, no matter how wide the terminal.
*/
fputs("\r\033[K", stderr);
}
/*
* How many columns do we need to show this number in decimal?
*/
decimal_width: avoid integer overflow The decimal_width function originally appeared in blame.c as "lineno_width", and was designed for calculating the print-width of small-ish integer values (line numbers in text files). In ec7ff5b, it was made into a reusable function, and in dc801e7, we started using it to align diffstats. Binary files in a diffstat show byte counts rather than line numbers, meaning they can be quite large (e.g., consider adding or removing a 2GB file). decimal_width is not up to the challenge for two reasons: 1. It takes the value as an "int", whereas large files may easily surpass this. The value may be truncated, in which case we will produce an incorrect value. 2. It counts "up" by repeatedly multiplying another integer by 10 until it surpasses the value. This can cause an infinite loop when the value is close to the largest representable integer. For example, consider using a 32-bit signed integer, and a value of 2,140,000,000 (just shy of 2^31-1). We will count up and eventually see that 1,000,000,000 is smaller than our value. The next step would be to multiply by 10 and see that 10,000,000,000 is too large, ending the loop. But we can't represent that value, and we have signed overflow. This is technically undefined behavior, but a common behavior is to lose the high bits, in which case our iterator will certainly be less than the number. So we'll keep multiplying, overflow again, and so on. This patch changes the argument to a uintmax_t (the same type we use to store the diffstat information for binary filese), and counts "down" by repeatedly dividing our value by 10. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-05 09:14:19 +01:00
int decimal_width(uintmax_t number)
{
decimal_width: avoid integer overflow The decimal_width function originally appeared in blame.c as "lineno_width", and was designed for calculating the print-width of small-ish integer values (line numbers in text files). In ec7ff5b, it was made into a reusable function, and in dc801e7, we started using it to align diffstats. Binary files in a diffstat show byte counts rather than line numbers, meaning they can be quite large (e.g., consider adding or removing a 2GB file). decimal_width is not up to the challenge for two reasons: 1. It takes the value as an "int", whereas large files may easily surpass this. The value may be truncated, in which case we will produce an incorrect value. 2. It counts "up" by repeatedly multiplying another integer by 10 until it surpasses the value. This can cause an infinite loop when the value is close to the largest representable integer. For example, consider using a 32-bit signed integer, and a value of 2,140,000,000 (just shy of 2^31-1). We will count up and eventually see that 1,000,000,000 is smaller than our value. The next step would be to multiply by 10 and see that 10,000,000,000 is too large, ending the loop. But we can't represent that value, and we have signed overflow. This is technically undefined behavior, but a common behavior is to lose the high bits, in which case our iterator will certainly be less than the number. So we'll keep multiplying, overflow again, and so on. This patch changes the argument to a uintmax_t (the same type we use to store the diffstat information for binary filese), and counts "down" by repeatedly dividing our value by 10. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-05 09:14:19 +01:00
int width;
decimal_width: avoid integer overflow The decimal_width function originally appeared in blame.c as "lineno_width", and was designed for calculating the print-width of small-ish integer values (line numbers in text files). In ec7ff5b, it was made into a reusable function, and in dc801e7, we started using it to align diffstats. Binary files in a diffstat show byte counts rather than line numbers, meaning they can be quite large (e.g., consider adding or removing a 2GB file). decimal_width is not up to the challenge for two reasons: 1. It takes the value as an "int", whereas large files may easily surpass this. The value may be truncated, in which case we will produce an incorrect value. 2. It counts "up" by repeatedly multiplying another integer by 10 until it surpasses the value. This can cause an infinite loop when the value is close to the largest representable integer. For example, consider using a 32-bit signed integer, and a value of 2,140,000,000 (just shy of 2^31-1). We will count up and eventually see that 1,000,000,000 is smaller than our value. The next step would be to multiply by 10 and see that 10,000,000,000 is too large, ending the loop. But we can't represent that value, and we have signed overflow. This is technically undefined behavior, but a common behavior is to lose the high bits, in which case our iterator will certainly be less than the number. So we'll keep multiplying, overflow again, and so on. This patch changes the argument to a uintmax_t (the same type we use to store the diffstat information for binary filese), and counts "down" by repeatedly dividing our value by 10. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-05 09:14:19 +01:00
for (width = 1; number >= 10; width++)
number /= 10;
return width;
}
struct pager_command_config_data {
const char *cmd;
int want;
char *value;
};
static int pager_command_config(const char *var, const char *value, void *vdata)
{
struct pager_command_config_data *data = vdata;
const char *cmd;
if (skip_prefix(var, "pager.", &cmd) && !strcmp(cmd, data->cmd)) {
int b = git_parse_maybe_bool(value);
if (b >= 0)
data->want = b;
else {
data->want = 1;
data->value = xstrdup(value);
}
}
return 0;
}
/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
int check_pager_config(const char *cmd)
{
struct pager_command_config_data data;
data.cmd = cmd;
data.want = -1;
data.value = NULL;
read_early_config(pager_command_config, &data);
if (data.value)
pager_program = data.value;
return data.want;
}