From 3ae5b23971d7178ce791132e0dbb2df787ac7bf5 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 12 Aug 2019 09:16:24 -0700 Subject: [PATCH] Migrate append_history_item_to_buffer to history_file.cpp Also eliminate history_output_buffer_t, which no longer does anything useful. --- src/history.cpp | 108 ++++++++++++------------------------------- src/history_file.cpp | 23 +++++++++ src/history_file.h | 5 ++ 3 files changed, 58 insertions(+), 78 deletions(-) diff --git a/src/history.cpp b/src/history.cpp index fd3080a9a..fb167caac 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -76,38 +76,18 @@ static constexpr int max_save_tries = 1024; namespace { -/// Helper class for certain output. This is basically a string that allows us to ensure we only -/// flush at record boundaries, and avoids the copying of ostringstream. Have you ever tried to -/// implement your own streambuf? Total insanity. -class history_output_buffer_t { - std::string buffer; - - public: - /// Add a bit more to HISTORY_OUTPUT_BUFFER_SIZE because we flush once we've exceeded that size. - explicit history_output_buffer_t(size_t reserve_amt = HISTORY_OUTPUT_BUFFER_SIZE + 128) { - buffer.reserve(reserve_amt); +/// If the size of \p buffer is at least \p min_size, output the contents of a string \p str to \p +/// fd, and clear the string. \return 0 on success, an error code on failure. +static int flush_to_fd(std::string *buffer, int fd, size_t min_size) { + if (buffer->empty() || buffer->size() < min_size) { + return 0; } - - /// Append one or more strings. - void append(const char *s1, const char *s2 = NULL, const char *s3 = NULL) { - if (s1) buffer.append(s1); - if (s2) buffer.append(s2); - if (s3) buffer.append(s3); + if (write_loop(fd, buffer->data(), buffer->size()) < 0) { + return errno; } - - /// Output to a given fd, resetting our buffer. Returns true on success, false on error. - bool flush_to_fd(int fd) { - if (buffer.empty()) { - return true; - } - bool result = write_loop(fd, buffer.data(), buffer.size()) >= 0; - buffer.clear(); - return result; - } - - /// Return how much data we've accumulated. - size_t output_size() const { return buffer.size(); } -}; + buffer->clear(); + return 0; +} class time_profiler_t { const char *what; @@ -231,26 +211,6 @@ bool history_item_t::matches_search(const wcstring &term, enum history_search_ty DIE("unexpected history_search_type_t value"); } -/// Append our YAML history format to the provided vector at the given offset, updating the offset. -static void append_yaml_to_buffer(const wcstring &wcmd, time_t timestamp, - const path_list_t &required_paths, - history_output_buffer_t *buffer) { - std::string cmd = wcs2string(wcmd); - escape_yaml_fish_2_0(&cmd); - buffer->append("- cmd: ", cmd.c_str(), "\n"); - buffer->append(" when: ", std::to_string(timestamp).c_str(), "\n"); - - if (!required_paths.empty()) { - buffer->append(" paths:\n"); - - for (auto const &wpath : required_paths) { - std::string path = wcs2string(wpath); - escape_yaml_fish_2_0(&path); - buffer->append(" - ", path.c_str(), "\n"); - } - } -} - struct history_impl_t { // Privately add an item. If pending, the item will not be returned by history searches until a // call to resolve_pending. @@ -744,27 +704,22 @@ bool history_impl_t::rewrite_to_temporary_file(int existing_fd, int dst_fd) cons }); // Write them out. - bool ok = true; - history_output_buffer_t buffer(HISTORY_OUTPUT_BUFFER_SIZE); + int err = 0; + std::string buffer; + buffer.reserve(HISTORY_OUTPUT_BUFFER_SIZE + 128); for (const auto &key_item : lru) { - const history_item_t &item = key_item.second; - append_yaml_to_buffer(item.str(), item.timestamp(), item.required_paths, &buffer); - if (buffer.output_size() >= HISTORY_OUTPUT_BUFFER_SIZE) { - ok = buffer.flush_to_fd(dst_fd); - if (!ok) { - debug(2, L"Error %d when writing to temporary history file", errno); - break; - } - } + append_history_item_to_buffer(key_item.second, &buffer); + err = flush_to_fd(&buffer, dst_fd, HISTORY_OUTPUT_BUFFER_SIZE); + if (err) break; + } + if (!err) { + err = flush_to_fd(&buffer, dst_fd, 0); + } + if (err) { + debug(2, L"Error %d when writing to temporary history file", err); } - if (ok) { - ok = buffer.flush_to_fd(dst_fd); - if (!ok) { - debug(2, L"Error %d when writing to temporary history file", errno); - } - } - return ok; + return err == 0; } // Returns the fd of an opened temporary file, or -1 on failure @@ -962,23 +917,20 @@ bool history_impl_t::save_internal_via_appending() { // write even a pending item - pending items are ignored by history within the command // itself, but should still be written to the file. // TODO: consider filling the buffer ahead of time, so we can just lock, splat, and unlock? - bool errored = false; + int err = 0; // Use a small buffer size for appending, we usually only have 1 item - history_output_buffer_t buffer(64); + std::string buffer; while (first_unwritten_new_item_index < new_items.size()) { const history_item_t &item = new_items.at(first_unwritten_new_item_index); - append_yaml_to_buffer(item.str(), item.timestamp(), item.get_required_paths(), &buffer); - if (buffer.output_size() >= HISTORY_OUTPUT_BUFFER_SIZE) { - errored = !buffer.flush_to_fd(history_fd); - if (errored) break; - } - + append_history_item_to_buffer(item, &buffer); + err = flush_to_fd(&buffer, history_fd, HISTORY_OUTPUT_BUFFER_SIZE); + if (err) break; // We wrote this item, hooray. first_unwritten_new_item_index++; } - if (!errored && buffer.flush_to_fd(history_fd)) { - ok = true; + if (!err) { + err = flush_to_fd(&buffer, history_fd, 0); } // Since we just modified the file, update our mmap_file_id to match its current state diff --git a/src/history_file.cpp b/src/history_file.cpp index 4a416cf2c..b1e00b6f7 100644 --- a/src/history_file.cpp +++ b/src/history_file.cpp @@ -428,6 +428,29 @@ static size_t offset_of_next_item_fish_2_0(const history_file_contents_t &conten return result; } +void append_history_item_to_buffer(const history_item_t &item, std::string *buffer) { + auto append = [=](const char *a, const char *b = nullptr, const char *c = nullptr) { + if (a) buffer->append(a); + if (b) buffer->append(b); + if (c) buffer->append(c); + }; + + std::string cmd = wcs2string(item.str()); + escape_yaml_fish_2_0(&cmd); + append("- cmd: ", cmd.c_str(), "\n"); + append(" when: ", std::to_string(item.timestamp()).c_str(), "\n"); + const path_list_t &paths = item.get_required_paths(); + if (!paths.empty()) { + append(" paths:\n"); + + for (const auto &wpath : paths) { + std::string path = wcs2string(wpath); + escape_yaml_fish_2_0(&path); + append(" - ", path.c_str(), "\n"); + } + } +} + /// Remove backslashes from all newlines. This makes a string from the history file better formated /// for on screen display. static wcstring history_unescape_newlines_fish_1_x(const wcstring &in_str) { diff --git a/src/history_file.h b/src/history_file.h index 449ef0523..bc0065099 100644 --- a/src/history_file.h +++ b/src/history_file.h @@ -10,6 +10,8 @@ #include +class history_item_t; + // History file types. enum history_file_type_t { history_type_fish_2_0, history_type_fish_1_x }; @@ -65,6 +67,9 @@ class history_file_contents_t { void operator=(history_file_contents_t &&) = delete; }; +/// Append a history item to a buffer, in preparation for outputting it to the history file. +void append_history_item_to_buffer(const history_item_t &item, std::string *buffer); + // Support for escaping and unescaping the nonstandard "yaml" format introduced in fish 2.0. void escape_yaml_fish_2_0(std::string *str); void unescape_yaml_fish_2_0(std::string *str);