From a305fe5feaf32e5d72c0951b6ef0a522f7a5830d Mon Sep 17 00:00:00 2001 From: haozi007 Date: Tue, 15 Aug 2023 19:08:34 +0800 Subject: [PATCH 01/10] improve coding Signed-off-by: haozi007 --- .../connect/grpc/grpc_containers_client.cc | 2 +- .../container/container_events_handler.c | 4 ++-- src/daemon/modules/container/container_unix.c | 2 +- .../modules/runtime/isula/isula_rt_ops.c | 2 +- src/utils/buffer/buffer.c | 6 +----- src/utils/cutils/error.h | 2 +- src/utils/cutils/util_atomic.h | 3 ++- src/utils/cutils/utils.c | 2 +- src/utils/cutils/utils_base64.c | 10 +++++++--- src/utils/cutils/utils_file.c | 18 ++++++++++++------ src/utils/cutils/utils_fs.c | 2 +- src/utils/cutils/utils_mount_spec.c | 3 +-- src/utils/cutils/utils_string.c | 7 +++---- src/utils/cutils/utils_string.h | 2 +- src/utils/cutils/utils_timestamp.c | 4 ++-- src/utils/http/http.c | 2 +- src/utils/tar/isulad_tar.c | 4 +--- src/utils/tar/isulad_tar.h | 2 -- src/utils/tar/util_archive.c | 16 ++++++++++------ src/utils/tar/util_gzip.c | 6 +++--- 20 files changed, 52 insertions(+), 47 deletions(-) diff --git a/src/client/connect/grpc/grpc_containers_client.cc b/src/client/connect/grpc/grpc_containers_client.cc index 301e172b..314f381f 100644 --- a/src/client/connect/grpc/grpc_containers_client.cc +++ b/src/client/connect/grpc/grpc_containers_client.cc @@ -17,7 +17,7 @@ #include "container.grpc.pb.h" #include "isula_libutils/container_copy_to_request.h" #include "isula_libutils/container_exec_request.h" -#include "isulad_tar.h" +#include "util_archive.h" #include "stoppable_thread.h" #include "utils.h" #include diff --git a/src/daemon/modules/container/container_events_handler.c b/src/daemon/modules/container/container_events_handler.c index 55dbfbe6..d78e6fc1 100644 --- a/src/daemon/modules/container/container_events_handler.c +++ b/src/daemon/modules/container/container_events_handler.c @@ -131,7 +131,7 @@ static int container_state_changed(container_t *cont, const struct isulad_events pid = container_state_get_pid(cont->state); if (pid != (int)events->pid) { - DEBUG("Container's pid \'%d\' is not equal to event's pid \'%d\', ignore STOPPED event", pid, + DEBUG("Container's pid \'%d\' is not equal to event's pid \'%u\', ignore STOPPED event", pid, events->pid); container_unlock(cont); ret = 0; @@ -212,7 +212,7 @@ static int handle_one(container_t *cont, container_events_handler_t *handler) events_handler_unlock(handler); events = (struct isulad_events_format *)it->elem; - INFO("Received event %s with pid %d", events->id, events->pid); + INFO("Received event %s with pid %u", events->id, events->pid); if (container_state_changed(cont, events)) { ERROR("Failed to change container %s state", cont->common_config->id); diff --git a/src/daemon/modules/container/container_unix.c b/src/daemon/modules/container/container_unix.c index 9910b3c8..9392cf0d 100644 --- a/src/daemon/modules/container/container_unix.c +++ b/src/daemon/modules/container/container_unix.c @@ -438,7 +438,7 @@ out: int container_v2_spec_merge_contaner_spec(container_config_v2_common_config *v2_spec) { int ret = 0; - int i = 0; + size_t i = 0; container_config *container_spec = NULL; if (v2_spec == NULL) { diff --git a/src/daemon/modules/runtime/isula/isula_rt_ops.c b/src/daemon/modules/runtime/isula/isula_rt_ops.c index 0f18926a..817d663f 100644 --- a/src/daemon/modules/runtime/isula/isula_rt_ops.c +++ b/src/daemon/modules/runtime/isula/isula_rt_ops.c @@ -704,7 +704,7 @@ static int status_to_exit_code(int status) shim_exit_code records the exit code of isulad-shim, obtained through waitpid; */ static int shim_create(bool fg, const char *id, const char *workdir, const char *bundle, const char *runtime_cmd, - int *exit_code, const char* timeout, int* shim_exit_code) + int *exit_code, const char *timeout, int *shim_exit_code) { pid_t pid = 0; int shim_stderr_pipe[2] = { -1, -1 }; diff --git a/src/utils/buffer/buffer.c b/src/utils/buffer/buffer.c index 19a933cd..7f6bc527 100644 --- a/src/utils/buffer/buffer.c +++ b/src/utils/buffer/buffer.c @@ -36,11 +36,7 @@ Buffer *buffer_alloc(size_t initial_size) return NULL; } - if (initial_size > SIZE_MAX / sizeof(char)) { - free(buf); - return NULL; - } - tmp = calloc(1, initial_size * sizeof(char)); + tmp = util_smart_calloc_s(sizeof(char), initial_size); if (tmp == NULL) { free(buf); return NULL; diff --git a/src/utils/cutils/error.h b/src/utils/cutils/error.h index e3946cf2..537f4d12 100644 --- a/src/utils/cutils/error.h +++ b/src/utils/cutils/error.h @@ -60,11 +60,11 @@ static inline void format_errorf(char **err, const char *format, ...) char errbuf[BUFSIZ + 1] = { 0 }; va_list argp; - va_start(argp, format); if (err == NULL) { return; } + va_start(argp, format); ret = vsnprintf(errbuf, BUFSIZ, format, argp); va_end(argp); diff --git a/src/utils/cutils/util_atomic.h b/src/utils/cutils/util_atomic.h index 6fa2a662..5fa2c3d6 100644 --- a/src/utils/cutils/util_atomic.h +++ b/src/utils/cutils/util_atomic.h @@ -129,7 +129,8 @@ static inline bool atomic_int_compare_exchange(volatile uint64_t *atomic, uint64 atomic_mutex_lock(&g_atomic_lock); - if ((success = (*atomic == oldval))) { + success = (*atomic == oldval); + if (success) { *atomic = newval; } diff --git a/src/utils/cutils/utils.c b/src/utils/cutils/utils.c index 3cede76a..a29de20e 100644 --- a/src/utils/cutils/utils.c +++ b/src/utils/cutils/utils.c @@ -1389,7 +1389,7 @@ static char *get_cpu_variant() int util_normalized_host_os_arch(char **host_os, char **host_arch, char **host_variant) { int ret = 0; - int i = 0; + size_t i; struct utsname uts; char *tmp_variant = NULL; diff --git a/src/utils/cutils/utils_base64.c b/src/utils/cutils/utils_base64.c index 8301e4f9..3871140e 100644 --- a/src/utils/cutils/utils_base64.c +++ b/src/utils/cutils/utils_base64.c @@ -173,13 +173,13 @@ out: return ret; } -size_t util_base64_decode_len(const char *input, size_t len) +static size_t util_base64_decode_len(const char *input, size_t len) { size_t padding_count = 0; if (input == NULL || len < 4 || len % 4 != 0) { ERROR("Invalid param for base64 decode length, length is %zu", len); - return -1; + return 0; } if (input[len - 1] == '=') { @@ -189,7 +189,7 @@ size_t util_base64_decode_len(const char *input, size_t len) } } - return (strlen(input) / 4 * 3) - padding_count; + return (((strlen(input) / 4) * 3) - padding_count); } int util_base64_decode(const char *input, size_t len, unsigned char **out, size_t *out_len) @@ -219,6 +219,10 @@ int util_base64_decode(const char *input, size_t len, unsigned char **out, size_ io = BIO_push(base64, io); out_put_len = util_base64_decode_len(input, len); + if (out_put_len == 0) { + ret = -1; + goto out; + } out_put = util_common_calloc_s(out_put_len + 1); // '+1' for '\0' if (out_put == NULL) { ERROR("out of memory"); diff --git a/src/utils/cutils/utils_file.c b/src/utils/cutils/utils_file.c index 4c62aaa6..9000b0dc 100644 --- a/src/utils/cutils/utils_file.c +++ b/src/utils/cutils/utils_file.c @@ -120,14 +120,19 @@ int util_path_remove(const char *path) ssize_t util_write_nointr_in_total(int fd, const char *buf, size_t count) { - ssize_t nret = 0; - ssize_t nwritten; + size_t nwritten; if (buf == NULL) { return -1; } + if (count > SSIZE_MAX) { + ERROR("Too large data to write"); + return -1; + } + for (nwritten = 0; nwritten < count;) { + ssize_t nret; nret = write(fd, buf + nwritten, count - nwritten); if (nret < 0) { if (errno == EINTR || errno == EAGAIN) { @@ -140,7 +145,7 @@ ssize_t util_write_nointr_in_total(int fd, const char *buf, size_t count) } } - return nwritten; + return (ssize_t)nwritten; } ssize_t util_write_nointr(int fd, const void *buf, size_t count) @@ -1700,9 +1705,10 @@ int util_set_file_group(const char *fname, const char *group) grp = getgrnam(group); if (grp != NULL) { gid = grp->gr_gid; - DEBUG("Group %s found, gid: %d", group, gid); + DEBUG("Group %s found, gid: %u", group, gid); + // set owner to -1, will not change owner if (chown(fname, -1, gid) != 0) { - ERROR("Failed to chown %s to gid: %d", fname, gid); + ERROR("Failed to chown %s to gid: %u", fname, gid); ret = -1; goto out; } @@ -2032,7 +2038,7 @@ static int copy_file(char *copy_dst, char *copy_src, struct stat *src_stat, map_ } else if (S_ISCHR(src_stat->st_mode) || S_ISBLK(src_stat->st_mode)) { ret = copy_device(copy_dst, copy_src, src_stat); } else { // fifo and socket - ERROR("copy %s failed, unsupported type %d", copy_src, src_stat->st_mode); + ERROR("copy %s failed, unsupported type %u", copy_src, src_stat->st_mode); return -1; } if (ret != 0) { diff --git a/src/utils/cutils/utils_fs.c b/src/utils/cutils/utils_fs.c index e7165f26..a8c65f86 100644 --- a/src/utils/cutils/utils_fs.c +++ b/src/utils/cutils/utils_fs.c @@ -111,7 +111,7 @@ static struct fs_element const g_fs_names[] = { struct mount_option_element { const char *option; bool clear; - int flag; + unsigned long flag; }; static struct mount_option_element const g_mount_options[] = { diff --git a/src/utils/cutils/utils_mount_spec.c b/src/utils/cutils/utils_mount_spec.c index 6793f93b..5386c115 100644 --- a/src/utils/cutils/utils_mount_spec.c +++ b/src/utils/cutils/utils_mount_spec.c @@ -67,8 +67,6 @@ static int parse_mount_item_type(const char *value, char *mount_str, mount_spec static int parse_mount_item_src(const char *value, char *mount_str, mount_spec *m, char *errmsg) { - char srcpath[PATH_MAX] = { 0 }; - /* If value of source is NULL, ignore it */ if (value == NULL) { return 0; @@ -87,6 +85,7 @@ static int parse_mount_item_src(const char *value, char *mount_str, mount_spec * #endif if (value[0] == '/') { + char srcpath[PATH_MAX] = { 0 }; if (!util_clean_path(value, srcpath, sizeof(srcpath))) { CACHE_ERRMSG(errmsg, "Invalid mount specification '%s'.Can't translate source path to clean path", mount_str); diff --git a/src/utils/cutils/utils_string.c b/src/utils/cutils/utils_string.c index de1cc60e..ba7dd5b4 100644 --- a/src/utils/cutils/utils_string.c +++ b/src/utils/cutils/utils_string.c @@ -83,11 +83,10 @@ bool util_strings_contains_word(const char *str, const char *substr) return false; } -int util_strings_count(const char *str, unsigned char c) +size_t util_strings_count(const char *str, unsigned char c) { - size_t i = 0; - int res = 0; - size_t len = 0; + size_t i, len; + size_t res = 0; if (str == NULL) { return 0; diff --git a/src/utils/cutils/utils_string.h b/src/utils/cutils/utils_string.h index 4e97c574..0de2266c 100644 --- a/src/utils/cutils/utils_string.h +++ b/src/utils/cutils/utils_string.h @@ -28,7 +28,7 @@ bool util_strings_contains_any(const char *str, const char *substr); bool util_strings_contains_word(const char *str, const char *substr); -int util_strings_count(const char *str, unsigned char c); +size_t util_strings_count(const char *str, unsigned char c); bool util_strings_in_slice(const char **strarray, size_t alen, const char *str); diff --git a/src/utils/cutils/utils_timestamp.c b/src/utils/cutils/utils_timestamp.c index 85551d51..3a440ca9 100644 --- a/src/utils/cutils/utils_timestamp.c +++ b/src/utils/cutils/utils_timestamp.c @@ -495,7 +495,7 @@ bool util_get_tm_from_str(const char *str, struct tm *tm, int32_t *nanos) if (util_strings_contains_any(str, ".")) { format = rFC339NanoLocal; } else if (util_strings_contains_any(str, "T")) { - int tcolons = util_strings_count(str, ':'); + size_t tcolons = util_strings_count(str, ':'); switch (tcolons) { case 0: format = "2016-01-02T15"; @@ -952,7 +952,7 @@ err_out: int util_to_unix_nanos_from_str(const char *str, int64_t *nanos) { struct tm tm = { 0 }; - struct types_timezone tz; + struct types_timezone tz = { 0 }; int32_t nano = 0; types_timestamp_t ts; const int s_hour = 3600; diff --git a/src/utils/http/http.c b/src/utils/http/http.c index 2b514666..6759a28d 100644 --- a/src/utils/http/http.c +++ b/src/utils/http/http.c @@ -266,7 +266,6 @@ static void free_rpath(char *rpath) static void check_buf_len(struct http_get_options *options, char *errbuf, CURLcode curl_result) { - int nret = 0; size_t len = 0; if (options == NULL || options->errmsg != NULL) { @@ -275,6 +274,7 @@ static void check_buf_len(struct http_get_options *options, char *errbuf, CURLco len = strlen(errbuf); if (len == 0) { + int nret = 0; nret = snprintf(errbuf, CURL_ERROR_SIZE, "curl response error code %d", curl_result); if (nret < 0 || (size_t)nret >= CURL_ERROR_SIZE) { ERROR("Failed to print string for error buffer, errcode %d", curl_result); diff --git a/src/utils/tar/isulad_tar.c b/src/utils/tar/isulad_tar.c index 228e091a..d7d69eb2 100644 --- a/src/utils/tar/isulad_tar.c +++ b/src/utils/tar/isulad_tar.c @@ -307,10 +307,8 @@ struct archive_copy_info *copy_info_destination_path(const char *path, char **er nret = copy_info_destination_path_ret(info, st, err, ret, path); if (nret == 0) { return info; - } else { - goto cleanup; } -cleanup: + free(info); return NULL; } diff --git a/src/utils/tar/isulad_tar.h b/src/utils/tar/isulad_tar.h index 31d2d24a..ec085c25 100644 --- a/src/utils/tar/isulad_tar.h +++ b/src/utils/tar/isulad_tar.h @@ -31,8 +31,6 @@ struct io_read_wrapper; extern "C" { #endif -#define ARCHIVE_BLOCK_SIZE (32 * 1024) - struct archive_copy_info { char *path; bool exists; diff --git a/src/utils/tar/util_archive.c b/src/utils/tar/util_archive.c index c72e63b8..c63fd00b 100644 --- a/src/utils/tar/util_archive.c +++ b/src/utils/tar/util_archive.c @@ -776,9 +776,8 @@ int archive_unpack(const struct io_read_wrapper *content, const char *dstdir, co child_out: if (ret != 0) { exit(EXIT_FAILURE); - } else { - exit(EXIT_SUCCESS); } + exit(EXIT_SUCCESS); } close(pipe_stderr[1]); pipe_stderr[1] = -1; @@ -1037,6 +1036,12 @@ static ssize_t stream_write_data(struct archive *a, void *client_data, const voi struct io_write_wrapper *writer = (struct io_write_wrapper *)client_data; size_t written_length = 0; size_t size = 0; + + if (length > SSIZE_MAX) { + ERROR("Too large data to write."); + return -1; + } + while (length > written_length) { if (length - written_length > ARCHIVE_WRITE_BUFFER_SIZE) { size = ARCHIVE_WRITE_BUFFER_SIZE; @@ -1050,7 +1055,7 @@ static ssize_t stream_write_data(struct archive *a, void *client_data, const voi written_length += size; } - return size; + return (ssize_t)written_length; } static int tar_all(const struct io_write_wrapper *writer, const char *tar_dir, const char *src_base, @@ -1264,7 +1269,7 @@ static int close_wait_pid(struct archive_context *ctx, int *status) if (ctx->pid > 0) { if (waitpid(ctx->pid, status, 0) != ctx->pid) { - ERROR("Failed to wait pid %u", ctx->pid); + ERROR("Failed to wait pid %d", ctx->pid); ret = -1; } } @@ -1409,9 +1414,8 @@ int archive_chroot_untar_stream(const struct io_read_wrapper *context, const cha child_out: if (ret != 0) { exit(EXIT_FAILURE); - } else { - exit(EXIT_SUCCESS); } + exit(EXIT_SUCCESS); } close(pipe_stderr[1]); diff --git a/src/utils/tar/util_gzip.c b/src/utils/tar/util_gzip.c index 2f4750be..2665e6df 100644 --- a/src/utils/tar/util_gzip.c +++ b/src/utils/tar/util_gzip.c @@ -32,7 +32,6 @@ int util_gzip_z(const char *srcfile, const char *dstfile, const mode_t mode) int srcfd = 0; gzFile stream = NULL; ssize_t size = 0; - size_t n = 0; void *buffer = 0; const char *gzerr = NULL; int errnum = 0; @@ -58,6 +57,7 @@ int util_gzip_z(const char *srcfile, const char *dstfile, const mode_t mode) } while (true) { + int n; size = util_read_nointr(srcfd, buffer, BLKSIZE); if (size < 0) { ERROR("read file %s failed: %s", srcfile, strerror(errno)); @@ -68,7 +68,7 @@ int util_gzip_z(const char *srcfile, const char *dstfile, const mode_t mode) } n = gzwrite(stream, buffer, size); - if (n <= 0 || n != (size_t)size) { + if (n <= 0 || n != size) { gzerr = gzerror(stream, &errnum); if (gzerr != NULL && strcmp(gzerr, "") != 0) { ERROR("gzread error: %s", gzerr); @@ -104,7 +104,6 @@ int util_gzip_d(const char *srcfile, const FILE *dstfp) int ret = 0; size_t size = 0; void *buffer = NULL; - size_t n = 0; stream = gzopen(srcfile, "r"); if (stream == NULL) { @@ -120,6 +119,7 @@ int util_gzip_d(const char *srcfile, const FILE *dstfp) } while (true) { + size_t n; n = gzread(stream, buffer, BLKSIZE); if (n <= 0) { gzerr = gzerror(stream, &errnum); -- 2.25.1