packages/iSulad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
iSulad/0107-improve-coding.patch
openeuler-sync-bot ac7f14ac9b !607 [sync] PR-606: code improvements and bugfix for code review 
* code improvements and bugfix for code review
2023-08-26 10:10:17 +00:00

491 lines
18 KiB
Diff
Raw Blame History

From a305fe5feaf32e5d72c0951b6ef0a522f7a5830d Mon Sep 17 00:00:00 2001
From: haozi007 <liuhao27@huawei.com>
Date: Tue, 15 Aug 2023 19:08:34 +0800
Subject: [PATCH 01/10] improve coding
Signed-off-by: haozi007 <liuhao27@huawei.com>
---
.../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 <fstream>
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
Reference in New Issue View Git Blame Copy Permalink