Skip to content

Commit

Permalink
If non-mmap: use stdio reading of files instead of std::stream
Browse files Browse the repository at this point in the history
std::fstream is much slower than the good ol' standard implementation.
In addition, there are no risks of throwing exceptions.
  • Loading branch information
hzeller committed Oct 4, 2024
1 parent 3415c94 commit 13e0cd0
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 25 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/verible-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@ jobs:
uses: actions/cache@v3
with:
path: "c:/users/runneradmin/_bazel_runneradmin"
key: bazelcache_windows2_${{ steps.cache_timestamp.outputs.time }}
restore-keys: bazelcache_windows2_
key: bazelcache_windows_${{ steps.cache_timestamp.outputs.time }}
restore-keys: bazelcache_windows_

- name: Install dependencies
run: |
Expand Down
56 changes: 34 additions & 22 deletions common/util/file_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@

#include "common/util/file_util.h"

#include <fcntl.h>

#include <algorithm>
#include <cerrno>
#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <filesystem>
#include <fstream>
#include <iostream>
#include <memory>
#include <string>
#include <system_error>
Expand All @@ -34,10 +36,11 @@
#include "common/util/logging.h"

#ifndef _WIN32
#include <fcntl.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <unistd.h>
#else
#include <io.h>
#endif

namespace fs = std::filesystem;
Expand Down Expand Up @@ -154,34 +157,35 @@ absl::Status FileExists(const std::string &filename) {

absl::StatusOr<std::string> GetContentAsString(absl::string_view filename) {
std::string content;
std::ifstream fs;
std::istream *stream = nullptr;
FILE *stream = nullptr;
const bool use_stdin = IsStdin(filename);
if (use_stdin) {
stream = &std::cin;
#ifdef _WIN32
_setmode(_fileno(stdin), _O_BINARY); // Work around DOS/Win silliness.
#endif
stream = stdin;
} else {
const std::string filename_str = std::string{filename};
if (absl::Status status = FileExists(filename_str); !status.ok()) {
return status; // Bail
}
fs.open(filename_str.c_str());
stream = fopen(filename_str.c_str(), "rb");
std::error_code err;
const size_t prealloc = fs::file_size(filename_str, err);
if (err.value() == 0) content.reserve(prealloc);
stream = &fs;
}
if (!stream->good()) {
if (!stream) {
return CreateErrorStatusFromErrno(filename, "can't read");
}
char buffer[4096];
while (stream->good() && !stream->eof()) {
stream->read(buffer, sizeof(buffer));
content.append(buffer, stream->gcount());
}

// Allow stdin to be reopened for more input.
if (use_stdin && std::cin.eof()) std::cin.clear();
return std::move(content);
int bytes_read;
do {
bytes_read = fread(buffer, 1, sizeof(buffer), stream);
content.append(buffer, bytes_read);
} while (bytes_read > 0);
fclose(stream);

return content;
}

static absl::StatusOr<std::unique_ptr<MemBlock>> AttemptMemMapFile(
Expand Down Expand Up @@ -244,11 +248,19 @@ absl::StatusOr<std::unique_ptr<MemBlock>> GetContentAsMemBlock(
absl::Status SetContents(absl::string_view filename,
absl::string_view content) {
VLOG(1) << __FUNCTION__ << ": Writing file: " << filename;
std::ofstream f(std::string(filename).c_str());
if (!f.good()) return CreateErrorStatusFromErrno(filename, "can't write.");
f << content;
f.close();
if (!f.good()) return CreateErrorStatusFromErrno(filename, "closing.");
FILE *out = fopen(std::string(filename).c_str(), "wb");
if (!out) return CreateErrorStatusFromErrno(filename, "can't write.");
const int64_t expected_write = content.size();
int64_t total_written = 0;
while (!content.empty()) {
int64_t w = fwrite(content.data(), 1, content.size(), out);
total_written += w;
content.remove_prefix(w);
}
const bool written_completely = (total_written == expected_write);
if (fclose(out) != 0 || !written_completely) {
return CreateErrorStatusFromErrno(filename, "closing.");
}
return absl::OkStatus();
}

Expand Down
3 changes: 2 additions & 1 deletion verilog/analysis/checkers/token_stream_lint_rule.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ void TokenStreamLintRule::HandleSymbol(const verible::Symbol &symbol,
return p->Tag().tag == TK_StringLiteral;
});
const auto &string_literal = SymbolCastToLeaf(**literal);
if (absl::StrContains(string_literal.get().text(), "\\\n")) {
if (absl::StrContains(string_literal.get().text(), "\\\n") ||
absl::StrContains(string_literal.get().text(), "\\\r")) {
violations_.insert(LintViolation(string_literal, kMessage, context));
}
}
Expand Down
6 changes: 6 additions & 0 deletions verilog/analysis/checkers/token_stream_lint_rule_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ TEST(StringLiteralConcatenationTest, FunctionFailures) {
"\"Humpty Dumpty sat on a wall. \\\nHumpty Dumpty had a great fall.\""},
";",
"\nendmodule"},
{"module m;\n",
"string tmp=",
{kToken,
"\"Humpty Dumpty discovers CRLF \\\r\nHumpty Dumpty CRinged.\""},
";",
"\nendmodule"},
{"module m;\n",
"string tmp=",
{kToken,
Expand Down
11 changes: 11 additions & 0 deletions verilog/tools/obfuscator/verilog_obfuscate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
#include <sstream> // IWYU pragma: keep // for ostringstream
#include <string> // for string, allocator, etc

#ifdef _WIN32
#include <fcntl.h>
#include <io.h>
#endif

#include "absl/flags/flag.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
Expand Down Expand Up @@ -70,6 +75,12 @@ static constexpr absl::string_view kBuiltinFunctions[] = {
};

int main(int argc, char **argv) {
#ifdef _WIN32
// stdio: Windows messes with newlines by default. Fix this here.
_setmode(_fileno(stdin), _O_BINARY);
_setmode(_fileno(stdout), _O_BINARY);
#endif

const auto usage = absl::StrCat("usage: ", argv[0],
" [options] < original > output\n"
R"(
Expand Down

0 comments on commit 13e0cd0

Please sign in to comment.