From f1f938732403003206a83a641a0a02a7f82125f7 Mon Sep 17 00:00:00 2001 From: jikai Date: Thu, 30 Nov 2023 19:17:37 +0800 Subject: [PATCH 20/22] drop atomic config write for partial file does Signed-off-by: jikai --- src/lcrcontainer_extend.c | 151 +++++++++++++++++++------------------ src/utils.c | 153 -------------------------------------- src/utils.h | 4 - tests/CMakeLists.txt | 3 +- tests/utils_ut.cpp | 137 ---------------------------------- 5 files changed, 81 insertions(+), 367 deletions(-) delete mode 100644 tests/utils_ut.cpp diff --git a/src/lcrcontainer_extend.c b/src/lcrcontainer_extend.c index 0e1d926..321be8c 100644 --- a/src/lcrcontainer_extend.c +++ b/src/lcrcontainer_extend.c @@ -346,7 +346,7 @@ out: return ret; } -static int lcr_spec_write_seccomp_line(FILE *fp, const char *seccomp) +static int lcr_spec_write_seccomp_line(int fd, const char *seccomp) { size_t len; char *line = NULL; @@ -371,17 +371,14 @@ static int lcr_spec_write_seccomp_line(FILE *fp, const char *seccomp) ERROR("Sprintf failed"); goto cleanup; } - if ((size_t)nret > len - 1) { nret = (int)(len - 1); } - line[nret] = '\n'; - if (fwrite(line, 1, len ,fp) != len) { + if (write(fd, line, len) == -1) { SYSERROR("Write file failed"); goto cleanup; } - ret = 0; cleanup: free(line); @@ -392,7 +389,9 @@ static char *lcr_save_seccomp_file(const char *bundle, const char *seccomp_conf) { char seccomp[PATH_MAX] = { 0 }; char *real_seccomp = NULL; + int fd = -1; int nret; + ssize_t written_cnt; nret = snprintf(seccomp, sizeof(seccomp), "%s/seccomp", bundle); if (nret < 0 || (size_t)nret >= sizeof(seccomp)) { @@ -405,9 +404,16 @@ static char *lcr_save_seccomp_file(const char *bundle, const char *seccomp_conf) goto cleanup; } - if (lcr_util_atomic_write_file(real_seccomp, seccomp_conf, strlen(seccomp_conf), - CONFIG_FILE_MODE, ".tmp_seccomp_save") == -1) { - ERROR("write seccomp_conf failed"); + fd = lcr_util_open(real_seccomp, O_CREAT | O_TRUNC | O_CLOEXEC | O_WRONLY, CONFIG_FILE_MODE); + if (fd == -1) { + SYSERROR("Create file %s failed", real_seccomp); + goto cleanup; + } + + written_cnt = write(fd, seccomp_conf, strlen(seccomp_conf)); + close(fd); + if (written_cnt == -1) { + SYSERROR("write seccomp_conf failed"); goto cleanup; } return real_seccomp; @@ -599,52 +605,34 @@ out_free: return NULL; } -static FILE *lcr_open_tmp_config_file(const char *bundle, char **config_file, char **tmp_file) + +static int lcr_open_config_file(const char *bundle) { char config[PATH_MAX] = { 0 }; + char *real_config = NULL; int fd = -1; int nret; - FILE *fp = NULL; nret = snprintf(config, sizeof(config), "%s/config", bundle); if (nret < 0 || (size_t)nret >= sizeof(config)) { goto out; } - nret = lcr_util_ensure_path(config_file, config); + nret = lcr_util_ensure_path(&real_config, config); if (nret < 0) { ERROR("Failed to ensure path %s", config); goto out; } - *tmp_file = lcr_util_get_tmp_file(*config_file, ".tmp_config_save"); - if (*tmp_file == NULL) { - ERROR("Failed to get random tmp file for %s", *config_file); - goto out; - } - - fd = lcr_util_open(*tmp_file, O_CREAT | O_TRUNC | O_CLOEXEC | O_WRONLY, CONFIG_FILE_MODE); + fd = lcr_util_open(real_config, O_CREAT | O_TRUNC | O_CLOEXEC | O_WRONLY, CONFIG_FILE_MODE); if (fd == -1) { - SYSERROR("Create file %s failed", *tmp_file); - lcr_set_error_message(LCR_ERR_RUNTIME, "Create file %s failed", *tmp_file); - goto out; - } - - fp = fdopen(fd, "w"); - if (fp == NULL) { - close(fd); - ERROR("FILE open failed"); + SYSERROR("Create file %s failed", real_config); + lcr_set_error_message(LCR_ERR_RUNTIME, "Create file %s failed", real_config); goto out; } - out: - if (fp == NULL) { - free(*tmp_file); - *tmp_file = NULL; - free(*config_file); - *config_file = NULL; - } - return fp; + free(real_config); + return fd; } // escape_string_encode unzip some escape characters @@ -710,17 +698,18 @@ static char *escape_string_encode(const char *src) return dst; } -static int lcr_spec_write_config(FILE *fp, const struct lcr_list *lcr_conf) +static int lcr_spec_write_config(int fd, const struct lcr_list *lcr_conf) { - size_t len; - int ret = -1; struct lcr_list *it = NULL; - char *line_encode = NULL; + size_t len; char *line = NULL; + char *line_encode = NULL; + int ret = -1; lcr_list_for_each(it, lcr_conf) { lcr_config_item_t *item = it->elem; int nret; + size_t encode_len; if (item != NULL) { if (strlen(item->value) > ((SIZE_MAX - strlen(item->name)) - 4)) { goto cleanup; @@ -733,6 +722,7 @@ static int lcr_spec_write_config(FILE *fp, const struct lcr_list *lcr_conf) } nret = snprintf(line, len, "%s = %s", item->name, item->value); + if (nret < 0 || (size_t)nret >= len) { ERROR("Sprintf failed"); goto cleanup; @@ -744,21 +734,19 @@ static int lcr_spec_write_config(FILE *fp, const struct lcr_list *lcr_conf) goto cleanup; } - len = strlen(line_encode); - line_encode[len] = '\n'; + encode_len = strlen(line_encode); - if (fwrite(line_encode, 1, len + 1, fp) != len + 1) { + line_encode[encode_len] = '\n'; + if (write(fd, line_encode, encode_len + 1) == -1) { SYSERROR("Write file failed"); goto cleanup; } - free(line); line = NULL; free(line_encode); line_encode = NULL; } } - ret = 0; cleanup: free(line); @@ -816,9 +804,7 @@ bool lcr_save_spec(const char *name, const char *lcrpath, const struct lcr_list const char *path = lcrpath ? lcrpath : LCRPATH; char *bundle = NULL; char *seccomp = NULL; - char *config_file = NULL; - char *tmp_file = NULL; - FILE *fp = NULL; + int fd = -1; int nret = 0; if (name == NULL) { @@ -843,48 +829,72 @@ bool lcr_save_spec(const char *name, const char *lcrpath, const struct lcr_list } } - fp = lcr_open_tmp_config_file(bundle, &config_file, &tmp_file); - if (fp == NULL) { + fd = lcr_open_config_file(bundle); + if (fd == -1) { goto out_free; } - if (lcr_spec_write_config(fp, lcr_conf)) { + if (lcr_spec_write_config(fd, lcr_conf)) { goto out_free; } if (seccomp_conf != NULL) { - nret = lcr_spec_write_seccomp_line(fp, seccomp); + nret = lcr_spec_write_seccomp_line(fd, seccomp); if (nret) { goto out_free; } } - fclose(fp); - fp = NULL; + bret = true; - nret = rename(tmp_file, config_file); - if (nret != 0) { - ERROR("Failed to rename old file %s to target %s", tmp_file, config_file); - goto out_free; +out_free: + free(bundle); + free(seccomp); + if (fd != -1) { + close(fd); } - bret = true; + return bret; +} -out_free: - if (fp != NULL) { - fclose(fp); +static int lcr_write_file(const char *path, const char *data, size_t len) +{ + char *real_path = NULL; + int fd = -1; + int ret = -1; + + if (path == NULL || strlen(path) == 0 || data == NULL || len == 0) { + return -1; } - if (!bret && tmp_file != NULL && unlink(tmp_file) != 0 && errno != ENOENT) { - SYSERROR("Failed to remove temp file:%s", tmp_file); + + if (lcr_util_ensure_path(&real_path, path) < 0) { + ERROR("Failed to ensure path %s", path); + goto out_free; } - free(config_file); - free(tmp_file); - free(seccomp); - free(bundle); - return bret; + fd = lcr_util_open(real_path, O_CREAT | O_TRUNC | O_CLOEXEC | O_WRONLY, CONFIG_FILE_MODE); + if (fd == -1) { + ERROR("Create file %s failed", real_path); + lcr_set_error_message(LCR_ERR_RUNTIME, "Create file %s failed", real_path); + goto out_free; + } + + if (write(fd, data, len) == -1) { + SYSERROR("write data to %s failed", real_path); + goto out_free; + } + + ret = 0; + +out_free: + if (fd != -1) { + close(fd); + } + free(real_path); + return ret; } + static bool lcr_write_ocihooks(const char *path, const oci_runtime_spec_hooks *hooks) { bool ret = false; @@ -897,9 +907,8 @@ static bool lcr_write_ocihooks(const char *path, const oci_runtime_spec_hooks *h goto out_free; } - if (lcr_util_atomic_write_file(path, json_hooks, strlen(json_hooks), - CONFIG_FILE_MODE, ".tmp_ocihooks_save") == -1) { - ERROR("write json hooks failed"); + if (lcr_write_file(path, json_hooks, strlen(json_hooks)) == -1) { + SYSERROR("write json hooks failed"); goto out_free; } diff --git a/src/utils.c b/src/utils.c index 85874a7..b999509 100644 --- a/src/utils.c +++ b/src/utils.c @@ -1284,159 +1284,6 @@ out: return ret; } -static char *lcr_util_path_dir(const char *path) -{ - char *dir = NULL; - int len = 0; - int i = 0; - - if (path == NULL) { - ERROR("invalid NULL param"); - return NULL; - } - - len = (int)strlen(path); - if (len == 0) { - return lcr_util_strdup_s("."); - } - - dir = lcr_util_strdup_s(path); - - for (i = len - 1; i > 0; i--) { - if (dir[i] == '/') { - dir[i] = 0; - break; - } - } - - if (i == 0 && dir[0] == '/') { - free(dir); - return lcr_util_strdup_s("/"); - } - - return dir; -} - -static char *lcr_util_path_join(const char *dir, const char *file) -{ - int nret = 0; - char path[PATH_MAX] = { 0 }; - char cleaned[PATH_MAX] = { 0 }; - - if (dir == NULL || file == NULL) { - ERROR("NULL dir or file failed"); - return NULL; - } - - nret = snprintf(path, PATH_MAX, "%s/%s", dir, file); - if (nret < 0 || (size_t)nret >= PATH_MAX) { - ERROR("dir or file too long failed"); - return NULL; - } - - if (cleanpath(path, cleaned, sizeof(cleaned)) == NULL) { - ERROR("Failed to clean path: %s", path); - return NULL; - } - - return lcr_util_strdup_s(cleaned); -} - -char *lcr_util_get_tmp_file(const char *fname, const char *tmp_name) -{ - char *result = NULL; - char *dir = NULL; - - if (fname == NULL || tmp_name == NULL) { - ERROR("Invalid NULL param"); - return NULL; - } - - dir = lcr_util_path_dir(fname); - if (dir == NULL) { - ERROR("Failed to get dir of %s", fname); - goto out; - } - - result = lcr_util_path_join(dir, tmp_name); - -out: - free(dir); - return result; -} - -static int do_atomic_write_file(const char *fname, const char *content, size_t content_len, mode_t mode) -{ - int ret = 0; - int dst_fd = -1; - ssize_t len = 0; - - dst_fd = lcr_util_open(fname, O_WRONLY | O_CREAT | O_TRUNC, mode); - if (dst_fd < 0) { - SYSERROR("Creat file: %s, failed", fname); - ret = -1; - goto free_out; - } - - len = lcr_util_write_nointr(dst_fd, content, content_len); - if (len < 0 || ((size_t)len) != content_len) { - ret = -1; - SYSERROR("Write file failed"); - goto free_out; - } - -free_out: - if (dst_fd >= 0) { - close(dst_fd); - } - return ret; -} - -int lcr_util_atomic_write_file(const char *fname, const char *content, size_t content_len, mode_t mode, const char *tmp_name) -{ - int ret = 0; - char *tmp_file = NULL; - char rpath[PATH_MAX] = { 0x00 }; - - if (fname == NULL) { - return -1; - } - if (content == NULL || content_len == 0) { - return 0; - } - - if (cleanpath(fname, rpath, sizeof(rpath)) == NULL) { - return -1; - } - - tmp_file = lcr_util_get_tmp_file(fname, tmp_name); - if (tmp_file == NULL) { - ERROR("Failed to get tmp file for %s", fname); - return -1; - } - - ret = do_atomic_write_file(tmp_file, content, content_len, mode); - if (ret != 0) { - ERROR("Failed to write content to tmp file for %s", tmp_file); - ret = -1; - goto free_out; - } - - ret = rename(tmp_file, rpath); - if (ret != 0) { - ERROR("Failed to rename old file %s to target %s", tmp_file, rpath); - ret = -1; - goto free_out; - } - -free_out: - if (ret != 0 && unlink(tmp_file) != 0 && errno != ENOENT) { - SYSERROR("Failed to remove temp file:%s", tmp_file); - } - free(tmp_file); - return ret; -} - /* swap in oci is memoy+swap, so here we need to get real swap */ int lcr_util_get_real_swap(int64_t memory, int64_t memory_swap, int64_t *swap) { diff --git a/src/utils.h b/src/utils.h index 69e0c28..2fe4f1e 100644 --- a/src/utils.h +++ b/src/utils.h @@ -217,10 +217,6 @@ int lcr_util_null_stdfds(void); int lcr_util_flock_append_file(const char *filepath, const char *content); -char *lcr_util_get_tmp_file(const char *fname, const char *tmp_name); - -int lcr_util_atomic_write_file(const char *fname, const char *content, size_t content_len, mode_t mode, const char *tmp_name); - int lcr_util_get_real_swap(int64_t memory, int64_t memory_swap, int64_t *swap); int lcr_util_trans_cpushare_to_cpuweight(int64_t cpu_share); uint64_t lcr_util_trans_blkio_weight_to_io_weight(int weight); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index af267d7..fada476 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -69,7 +69,6 @@ endif() _DEFINE_NEW_TEST(log_ut log_testcase) _DEFINE_NEW_TEST(libocispec_ut libocispec_testcase) _DEFINE_NEW_TEST(go_crc64_ut go_crc64_testcase) -_DEFINE_NEW_TEST(utils_ut utils_testcase) # mock test for run lcov to generate html @@ -82,7 +81,7 @@ target_link_libraries(mock_ut ${GTEST_LIBRARY} pthread ) -add_dependencies(mock_ut log_ut libocispec_ut go_crc64_ut utils_ut) +add_dependencies(mock_ut log_ut libocispec_ut go_crc64_ut) IF(ENABLE_GCOV) add_custom_target(coverage diff --git a/tests/utils_ut.cpp b/tests/utils_ut.cpp deleted file mode 100644 index 17f60ed..0000000 --- a/tests/utils_ut.cpp +++ /dev/null @@ -1,137 +0,0 @@ -/****************************************************************************** - * iSula-libutils: utils library for iSula - * - * Copyright (c) Huawei Technologies Co., Ltd. 2020. All rights reserved. - * - * Authors: - * jikai - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - ********************************************************************************/ - -#include - -#include -#include - -#include "utils.h" - -char *test_read_text_file(const char *path) -{ - char *buf = NULL; - long len = 0; - int f_fd = -1; - size_t readlen = 0; - FILE *filp = NULL; - char *rpath = NULL; - const long max_size = 10 * 1024 * 1024; /* 10M */ - - if (path == NULL) { - return NULL; - } - - if (lcr_util_ensure_path(&rpath, path) != 0) { - return NULL; - } - - f_fd = open(rpath, O_RDONLY | O_CLOEXEC, 0666); - if (f_fd < 0) { - goto err_out; - } - - filp = fdopen(f_fd, "r"); - if (filp == NULL) { - close(f_fd); - goto err_out; - } - - if (fseek(filp, 0, SEEK_END)) { - goto err_out; - } - - len = ftell(filp); - if (len > max_size) { - goto err_out; - } - - if (fseek(filp, 0, SEEK_SET)) { - goto err_out; - } - - buf = (char *)lcr_util_common_calloc_s((size_t)(len + 1)); - if (buf == NULL) { - goto err_out; - } - - readlen = fread(buf, 1, (size_t)len, filp); - if (((readlen < (size_t)len) && (!feof(filp))) || (readlen > (size_t)len)) { - free(buf); - buf = NULL; - goto err_out; - } - - buf[(size_t)len] = 0; - -err_out: - - if (filp != NULL) { - fclose(filp); - } - - free(rpath); - - return buf; -} - -TEST(utils_testcase, test_get_tmp_file) -{ - const char *fname = "/tmp/lcr-test/test"; - char *tmp_file = lcr_util_get_tmp_file(nullptr, ".tmp_test_file"); - ASSERT_EQ(tmp_file, nullptr); - - tmp_file = lcr_util_get_tmp_file(fname, ".tmp_test_file"); - ASSERT_NE(tmp_file, nullptr); - - ASSERT_STREQ(tmp_file, "/tmp/lcr-test/.tmp_test_file"); - free(tmp_file); -} - -TEST(utils_testcase, test_atomic_write_file) -{ - const char *fname = "/tmp/lcr-test/test"; - const char *content = "line1\nline2\n"; - const char *new_content = "line1\nline2\nline3\n"; - char *readcontent = nullptr; - - ASSERT_EQ(lcr_util_atomic_write_file(NULL, content, strlen(content), 0644, ".tmp_test"), -1); - ASSERT_EQ(lcr_util_atomic_write_file(fname, NULL, 0, 0644, ".tmp_test"), 0); - - ASSERT_EQ(lcr_util_build_dir(fname), 0); - - ASSERT_EQ(lcr_util_atomic_write_file(fname, content, strlen(content), 0644, ".tmp_test"), 0); - - readcontent = test_read_text_file(fname); - ASSERT_NE(readcontent, nullptr); - ASSERT_STREQ(readcontent, content); - free(readcontent); - - ASSERT_EQ(lcr_util_atomic_write_file(fname, new_content, strlen(new_content), 0644, ".tmp_test"), 0); - readcontent = test_read_text_file(fname); - ASSERT_NE(readcontent, nullptr); - ASSERT_STREQ(readcontent, new_content); - free(readcontent); - - ASSERT_EQ(lcr_util_recursive_rmdir("/tmp/lcr-test/", 1), 0); -} -- 2.34.1