From 652c42da2c6fa31168d5d299c959ab0c86799c4b Mon Sep 17 00:00:00 2001 From: Dean Lee Date: Fri, 29 Oct 2021 19:23:31 +0800 Subject: [PATCH] util: add safe write functions (#22160) * add safe write functions * log error * bzfile safe_fopen&safe_flush * update test_case * trigger ci Co-authored-by: Willem Melching --- selfdrive/common/tests/test_util.cc | 18 ++++++++++++++++++ selfdrive/common/util.cc | 26 ++++++++++++++++++++++++++ selfdrive/common/util.h | 5 +++++ selfdrive/loggerd/logger.h | 3 ++- selfdrive/loggerd/omx_encoder.cc | 10 +++++++--- 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/selfdrive/common/tests/test_util.cc b/selfdrive/common/tests/test_util.cc index 123650922a..06317accd0 100644 --- a/selfdrive/common/tests/test_util.cc +++ b/selfdrive/common/tests/test_util.cc @@ -93,6 +93,24 @@ TEST_CASE("util::read_files_in_dir") { } } + +TEST_CASE("util::safe_fwrite") { + char filename[] = "/tmp/XXXXXX"; + int fd = mkstemp(filename); + close(fd); + std::string dat = random_bytes(1024 * 1024); + + FILE *f = util::safe_fopen(filename, "wb"); + REQUIRE(f != nullptr); + size_t size = util::safe_fwrite(dat.data(), 1, dat.size(), f); + REQUIRE(size == dat.size()); + int ret = util::safe_fflush(f); + REQUIRE(ret == 0); + ret = fclose(f); + REQUIRE(ret == 0); + REQUIRE(dat == util::read_file(filename)); +} + TEST_CASE("util::create_directories") { system("rm /tmp/test_create_directories -rf"); std::string dir = "/tmp/test_create_directories/a/b/c/d/e/f"; diff --git a/selfdrive/common/util.cc b/selfdrive/common/util.cc index e2ad5409e7..3623ac70cc 100644 --- a/selfdrive/common/util.cc +++ b/selfdrive/common/util.cc @@ -107,6 +107,32 @@ int write_file(const char* path, const void* data, size_t size, int flags, mode_ return (n >= 0 && (size_t)n == size) ? 0 : -1; } +FILE* safe_fopen(const char* filename, const char* mode) { + FILE* fp = NULL; + do { + fp = fopen(filename, mode); + } while ((nullptr == fp) && (errno == EINTR)); + return fp; +} + +size_t safe_fwrite(const void* ptr, size_t size, size_t count, FILE* stream) { + size_t written = 0; + do { + size_t ret = ::fwrite((void*)((char *)ptr + written * size), size, count - written, stream); + if (ret == 0 && errno != EINTR) break; + written += ret; + } while (written != count); + return written; +} + +int safe_fflush(FILE *stream) { + int ret = EOF; + do { + ret = fflush(stream); + } while ((EOF == ret) && (errno == EINTR)); + return ret; +} + std::string readlink(const std::string &path) { char buff[4096]; ssize_t len = ::readlink(path.c_str(), buff, sizeof(buff)-1); diff --git a/selfdrive/common/util.h b/selfdrive/common/util.h index 1fcb23b443..5df271df67 100644 --- a/selfdrive/common/util.h +++ b/selfdrive/common/util.h @@ -75,6 +75,11 @@ std::string dir_name(std::string const& path); std::string read_file(const std::string& fn); std::map read_files_in_dir(const std::string& path); int write_file(const char* path, const void* data, size_t size, int flags = O_WRONLY, mode_t mode = 0664); + +FILE* safe_fopen(const char* filename, const char* mode); +size_t safe_fwrite(const void * ptr, size_t size, size_t count, FILE * stream); +int safe_fflush(FILE *stream); + std::string readlink(const std::string& path); bool file_exists(const std::string& fn); bool create_directories(const std::string &dir, mode_t mode); diff --git a/selfdrive/loggerd/logger.h b/selfdrive/loggerd/logger.h index 1c2a25df69..bdda9d6917 100644 --- a/selfdrive/loggerd/logger.h +++ b/selfdrive/loggerd/logger.h @@ -23,7 +23,7 @@ const std::string LOG_ROOT = Path::log_root(); class BZFile { public: BZFile(const char* path) { - file = fopen(path, "wb"); + file = util::safe_fopen(path, "wb"); assert(file != nullptr); int bzerror; bz_file = BZ2_bzWriteOpen(&bzerror, file, 9, 0, 30); @@ -35,6 +35,7 @@ class BZFile { if (bzerror != BZ_OK) { LOGE("BZ2_bzWriteClose error, bzerror=%d", bzerror); } + util::safe_fflush(file); int err = fclose(file); assert(err == 0); } diff --git a/selfdrive/loggerd/omx_encoder.cc b/selfdrive/loggerd/omx_encoder.cc index 3ccbbff57b..3205aac08a 100644 --- a/selfdrive/loggerd/omx_encoder.cc +++ b/selfdrive/loggerd/omx_encoder.cc @@ -344,7 +344,10 @@ void OmxEncoder::handle_out_buf(OmxEncoder *e, OMX_BUFFERHEADERTYPE *out_buf) { if (e->of) { //printf("write %d flags 0x%x\n", out_buf->nFilledLen, out_buf->nFlags); - fwrite(buf_data, out_buf->nFilledLen, 1, e->of); + size_t written = util::safe_fwrite(buf_data, 1, out_buf->nFilledLen, e->of); + if (written != out_buf->nFilledLen) { + LOGE("failed to write file.errno=%d", errno); + } } if (e->remuxing) { @@ -505,11 +508,11 @@ void OmxEncoder::encoder_open(const char* path) { this->wrote_codec_config = false; } else { if (this->write) { - this->of = fopen(this->vid_path, "wb"); + this->of = util::safe_fopen(this->vid_path, "wb"); assert(this->of); #ifndef QCOM2 if (this->codec_config_len > 0) { - fwrite(this->codec_config, this->codec_config_len, 1, this->of); + util::safe_fwrite(this->codec_config, 1, this->codec_config_len, this->of); } #endif } @@ -557,6 +560,7 @@ void OmxEncoder::encoder_close() { avformat_free_context(this->ofmt_ctx); } else { if (this->of) { + util::safe_fflush(this->of); fclose(this->of); this->of = nullptr; }