From 01dbfb0a3f62c5a2765fbed481e17752191ac755 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Tue, 20 Dec 2016 16:35:43 -0800 Subject: [PATCH] replace writestr() with fwprintf() in reader.cpp There are several places that use writestr() which should instead be using fwprintf() or equivalent. Also, clarify the documentation for why writestr() and writechr() exist so they aren't used inappropriately again. Fixes #3657 --- src/common.h | 18 +++++++++--------- src/output.cpp | 25 +++++++++++++++---------- src/reader.cpp | 16 ++++++++-------- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/common.h b/src/common.h index 500f209ee..09edff927 100644 --- a/src/common.h +++ b/src/common.h @@ -199,15 +199,15 @@ void write_ignore(int fd, const void *buff, size_t count); /// the tty. extern bool has_working_tty_timestamps; -/// This macro is used to check that an input argument is not null. It is a bit lika a non-fatal -/// form of assert. Instead of exit-ing on failure, the current function is ended at once. The -/// second parameter is the return value of the current function on failure. -#define CHECK(arg, retval) \ - if (!(arg)) { \ - debug(0, "function %s called with null value for argument %s. ", __func__, #arg); \ - bugreport(); \ - show_stackframe(L'E'); \ - return retval; \ +/// This macro is used to check that an argument is true. It is a bit like a non-fatal form of +/// assert. Instead of exiting on failure, the current function is ended at once. The second +/// parameter is the return value of the current function on failure. +#define CHECK(arg, retval) \ + if (!(arg)) { \ + debug(0, "function %s called with false value for argument %s", __func__, #arg); \ + bugreport(); \ + show_stackframe(L'E'); \ + return retval; \ } // Pause for input, then exit the program. If supported, print a backtrace first. diff --git a/src/output.cpp b/src/output.cpp index 707a922aa..ef87c5ea7 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -301,7 +301,11 @@ int writeb(tputs_arg_t b) { return 0; } -/// Write a wide character using the output method specified using output_set_writer(). +/// Write a wide character using the output method specified using output_set_writer(). This should +/// only be used when writing characters from user supplied strings. This is needed due to our use +/// of the ENCODE_DIRECT_BASE mechanism to allow the user to specify arbitrary byte values to be +/// output. Such as in a `printf` invocation that includes literal byte values such as `\x1B`. +/// This should not be used for writing non-user supplied characters. int writech(wint_t ch) { char buff[MB_LEN_MAX + 1]; size_t len; @@ -328,15 +332,16 @@ int writech(wint_t ch) { return 0; } -/// Write a wide character string to FD 1. +/// Write a wide character string to stdout. This should not be used to output things like warning +/// messages; just use debug() or fwprintf() for that. It should only be used to output user +/// supplied strings that might contain literal bytes; e.g., "\342\224\214" from issue #1894. This +/// is needed because those strings may contain chars specially encoded using ENCODE_DIRECT_BASE. void writestr(const wchar_t *str) { CHECK(str, ); - if (MB_CUR_MAX == 1) // single-byte locale (C/POSIX/ISO-8859) - { - while (*str) { - writech(*str++); - } + if (MB_CUR_MAX == 1) { + // Single-byte locale (C/POSIX/ISO-8859). + while (*str) writech(*str++); return; } @@ -349,11 +354,11 @@ void writestr(const wchar_t *str) { // Convert the string. len++; char *buffer, static_buffer[256]; - if (len <= sizeof static_buffer) + if (len <= sizeof static_buffer) { buffer = static_buffer; - else + } else { buffer = new char[len]; - + } wcstombs(buffer, str, len); // Write the string. diff --git a/src/reader.cpp b/src/reader.cpp index 679adef1c..6c867c2c3 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -698,17 +698,17 @@ void reader_write_title(const wcstring &cmd, bool reset_cursor_position) { proc_push_interactive(0); if (exec_subshell(fish_title_command, lst, false /* ignore exit status */) != -1 && !lst.empty()) { - writestr(L"\x1b]0;"); + fputs("\e]0;", stdout); for (size_t i = 0; i < lst.size(); i++) { - writestr(lst.at(i).c_str()); + fputws(lst.at(i).c_str(), stdout); } - writestr(L"\7"); + fputc('\a', stdout); } proc_pop_interactive(); set_color(rgb_color_t::reset(), rgb_color_t::reset()); if (reset_cursor_position && !lst.empty()) { // Put the cursor back at the beginning of the line (issue #2453). - writestr(L"\r"); + fputc('\r', stdout); } } @@ -1300,7 +1300,7 @@ static void reader_flash() { } reader_repaint(); - writestr(L"\a"); + fputwc(L'\a', stdout); pollint.tv_sec = 0; pollint.tv_nsec = 100 * 1000000; @@ -2244,8 +2244,8 @@ static void handle_end_loop() { } if (!data->prev_end_loop && bg_jobs) { - writestr(_(L"There are stopped or running jobs.\n")); - writestr(_(L"A second attempt to exit will force their termination.\n")); + fputws(_(L"There are stopped or running jobs.\n"), stdout); + fputws(_(L"A second attempt to exit will force their termination.\n"), stdout); reader_exit(0, 0); data->prev_end_loop = 1; return; @@ -3260,7 +3260,7 @@ const wchar_t *reader_readline(int nchars) { reader_repaint_if_needed(); } - writestr(L"\n"); + fputc('\n', stdout); // Ensure we have no pager contents when we exit. if (!data->pager.empty()) {