bsdiff: Change all stderr and perror to LOG(ERROR)
We need to be able to turn off the logging at runtime. When performing fuzz
testing, the input data is mostly invalid. But the invalid input data causes all
the error logs in the output which is noisy, inefficient, and not
necessary. More information can be found in:
https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/getting_started.md#disable-noisy-error-message-logging
One option to turn off the logging at runtime is:
std::cerr.setstate(std::ios_base::failbit);
which works but not when we are doing fprintf to stderr. Another option
is in the lines of the following:
freopen("/dev/null", "w", stderr);
But, this seem to disable everything even useful needed logs.
This patch, changes all codes (except in bsdiff_main.cc) that logs using
fprintf() and perror() to the new uniform form of logging (defined in
logging.h). This new logging alternative allows universal manipulation of the
logging if necessary.
Bug: none
Test: unittests pass
Change-Id: I42bb3e5856c5c215dd82925ccdca44556124655d
diff --git a/bspatch.cc b/bspatch.cc
index 2aedd43..d552dcf 100644
--- a/bspatch.cc
+++ b/bspatch.cc
@@ -50,6 +50,7 @@
#include "bsdiff/extents_file.h"
#include "bsdiff/file.h"
#include "bsdiff/file_interface.h"
+#include "bsdiff/logging.h"
#include "bsdiff/memory_file.h"
#include "bsdiff/patch_reader.h"
#include "bsdiff/sink_file.h"
@@ -68,12 +69,12 @@
while (size > 0) {
size_t bytes_to_output = std::min(size, buf_size);
if (!read_func(buf, bytes_to_output)) {
- fprintf(stderr, "Failed to read stream.\n");
+ LOG(ERROR) << "Failed to read stream.";
return 2;
}
if (!WriteAll(file, buf, bytes_to_output)) {
- perror("WriteAll() failed");
+ PLOG(ERROR) << "WriteAll() failed.";
return 1;
}
size -= bytes_to_output;
@@ -117,13 +118,11 @@
if (stat(new_filename, &new_stat) == -1) {
if (errno == ENOENT)
return false;
- fprintf(stderr, "Error stat the new file %s: %s\n", new_filename,
- strerror(errno));
+ PLOG(ERROR) << "Error stat the new file: " << new_filename;
return true;
}
if (stat(old_filename, &old_stat) == -1) {
- fprintf(stderr, "Error stat the old file %s: %s\n", old_filename,
- strerror(errno));
+ PLOG(ERROR) << "Error stat the old file: " << old_filename;
return true;
}
@@ -154,16 +153,14 @@
std::unique_ptr<FileInterface> patch_file =
File::FOpen(patch_filename, O_RDONLY);
if (!patch_file) {
- fprintf(stderr, "Error opening the patch file %s: %s\n", patch_filename,
- strerror(errno));
+ PLOG(ERROR) << "Error opening the patch file: " << patch_filename;
return 1;
}
uint64_t patch_size;
patch_file->GetSize(&patch_size);
std::vector<uint8_t> patch(patch_size);
if (!ReadAll(patch_file, patch.data(), patch_size)) {
- fprintf(stderr, "Error reading the patch file %s: %s\n", patch_filename,
- strerror(errno));
+ PLOG(ERROR) << "Error reading the patch file: " << patch_filename;
return 1;
}
patch_file.reset();
@@ -187,15 +184,14 @@
// Open input file for reading.
std::unique_ptr<FileInterface> old_file = File::FOpen(old_filename, O_RDONLY);
if (!old_file) {
- fprintf(stderr, "Error opening the old file %s: %s\n", old_filename,
- strerror(errno));
+ PLOG(ERROR) << "Error opening the old file: " << old_filename;
return 1;
}
std::vector<ex_t> parsed_old_extents;
if (using_extents) {
if (!ParseExtentStr(old_extents, &parsed_old_extents)) {
- fprintf(stderr, "Error parsing the old extents\n");
+ LOG(ERROR) << "Error parsing the old extents.";
return 2;
}
old_file.reset(new ExtentsFile(std::move(old_file), parsed_old_extents));
@@ -205,15 +201,14 @@
std::unique_ptr<FileInterface> new_file =
File::FOpen(new_filename, O_CREAT | O_WRONLY);
if (!new_file) {
- fprintf(stderr, "Error opening the new file %s: %s\n", new_filename,
- strerror(errno));
+ PLOG(ERROR) << "Error opening the new file: " << new_filename;
return 1;
}
std::vector<ex_t> parsed_new_extents;
if (using_extents) {
if (!ParseExtentStr(new_extents, &parsed_new_extents)) {
- fprintf(stderr, "Error parsing the new extents\n");
+ LOG(ERROR) << "Error parsing the new extents.";
return 2;
}
new_file.reset(new ExtentsFile(std::move(new_file), parsed_new_extents));
@@ -251,13 +246,13 @@
size_t patch_size) {
BsdiffPatchReader patch_reader;
if (!patch_reader.Init(patch_data, patch_size)) {
- fprintf(stderr, "Failed to initialize patch reader\n");
+ LOG(ERROR) << "Failed to initialize patch reader.";
return 2;
}
uint64_t old_file_size;
if (!old_file->GetSize(&old_file_size)) {
- fprintf(stderr, "Cannot obtain the size of old file.\n");
+ LOG(ERROR) << "Cannot obtain the size of old file.";
return 1;
}
@@ -270,13 +265,13 @@
while (newpos < patch_reader.new_file_size()) {
ControlEntry control_entry(0, 0, 0);
if (!patch_reader.ParseControlEntry(&control_entry)) {
- fprintf(stderr, "Failed to read control stream\n");
+ LOG(ERROR) << "Failed to read control stream.";
return 2;
}
// Sanity-check.
if (newpos + control_entry.diff_size > patch_reader.new_file_size()) {
- fprintf(stderr, "Corrupt patch.\n");
+ LOG(ERROR) << "Corrupt patch.";
return 2;
}
@@ -299,8 +294,7 @@
// We just checked that |seek_offset| is not negative.
if (static_cast<uint64_t>(seek_offset) != old_file_pos &&
!old_file->Seek(seek_offset)) {
- fprintf(stderr, "Error seeking input file to offset %" PRId64 ": %s\n",
- seek_offset, strerror(errno));
+ PLOG(ERROR) << "Error seeking input file to offset: " << seek_offset;
return 1;
}
@@ -311,23 +305,23 @@
size_t read_bytes;
size_t bytes_to_read = std::min(chunk_size, old_buf.size());
if (!old_file->Read(old_buf.data(), bytes_to_read, &read_bytes)) {
- perror("Error reading from input file");
+ PLOG(ERROR) << "Error reading from input file.";
return 1;
}
if (!read_bytes) {
- fprintf(stderr, "EOF reached while reading from input file.\n");
+ LOG(ERROR) << "EOF reached while reading from input file.";
return 2;
}
// Read same amount of bytes from diff block
if (!patch_reader.ReadDiffStream(new_buf.data(), read_bytes)) {
- fprintf(stderr, "Failed to read diff stream.\n");
+ LOG(ERROR) << "Failed to read diff stream.";
return 2;
}
// new_buf already has data from diff block, adds old data to it.
for (size_t k = 0; k < read_bytes; k++)
new_buf[k] += old_buf[k];
if (!WriteAll(new_file, new_buf.data(), read_bytes)) {
- perror("Error writing to new file");
+ PLOG(ERROR) << "Error writing to new file.";
return 1;
}
chunk_size -= read_bytes;
@@ -350,7 +344,7 @@
// Sanity-check.
if (newpos + control_entry.extra_size > patch_reader.new_file_size()) {
- fprintf(stderr, "Corrupt patch.\n");
+ LOG(ERROR) << "Corrupt patch.";
return 2;
}
@@ -371,12 +365,12 @@
old_file->Close();
if (!patch_reader.Finish()) {
- fprintf(stderr, "Failed to finish the patch reader\n");
+ LOG(ERROR) << "Failed to finish the patch reader.";
return 2;
}
if (!new_file->Close()) {
- perror("Error closing new file");
+ PLOG(ERROR) << "Error closing new file.";
return 1;
}
diff --git a/test_utils.cc b/test_utils.cc
index 25a9b8b..eb079d5 100644
--- a/test_utils.cc
+++ b/test_utils.cc
@@ -37,7 +37,7 @@
int mkstemp_fd = mkstemp(result.data());
if (mkstemp_fd < 0) {
- perror("mkstemp()");
+ PLOG(ERROR) << "mkstemp() Failed";
return false;
}
close(mkstemp_fd);
@@ -95,7 +95,7 @@
ScopedTempFile::~ScopedTempFile() {
if (!filename_.empty() && unlink(filename_.c_str()) < 0) {
- perror("Unable to remove temporary file");
+ PLOG(ERROR) << "Unable to remove temporary file.";
}
}
diff --git a/test_utils.h b/test_utils.h
index 7132fe5..8f7de72 100644
--- a/test_utils.h
+++ b/test_utils.h
@@ -9,12 +9,14 @@
#include <string>
#include <vector>
-#define TEST_AND_RETURN_FALSE(_x) \
- do { \
- if (!static_cast<bool>(_x)) { \
- fprintf(stderr, "%s failed.", #_x); \
- return false; \
- } \
+#include "bsdiff/logging.h"
+
+#define TEST_AND_RETURN_FALSE(_x) \
+ do { \
+ if (!static_cast<bool>(_x)) { \
+ LOG(ERROR) << #_x " failed."; \
+ return false; \
+ } \
} while (0)
namespace test_utils {
