From 0340a8248e8a4fb133ab3638679755d8590dafae Mon Sep 17 00:00:00 2001
From: xuxuepeng <xuxuepeng1@huawei.com>
Date: Wed, 11 Dec 2024 13:03:21 +0800
Subject: [PATCH 04/11] fix some bad code

Signed-off-by: xuxuepeng <xuxuepeng1@huawei.com>
---
 .../storage/layer_store/graphdriver/driver.c  | 19 +++++----------
 .../graphdriver/overlay2/driver_overlay2.c    | 24 ++++++++++++++-----
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/src/daemon/modules/image/oci/storage/layer_store/graphdriver/driver.c b/src/daemon/modules/image/oci/storage/layer_store/graphdriver/driver.c
index 94235b80..99fd573c 100644
--- a/src/daemon/modules/image/oci/storage/layer_store/graphdriver/driver.c
+++ b/src/daemon/modules/image/oci/storage/layer_store/graphdriver/driver.c
@@ -121,36 +121,31 @@ static inline void driver_unlock()
 
 int graphdriver_init(const struct storage_module_init_options *opts)
 {
-    int ret = 0;
     size_t i = 0;
     char driver_home[PATH_MAX] = { 0 };
 
     if (opts == NULL || opts->storage_root == NULL || opts->driver_name == NULL) {
-        ret = -1;
-        goto out;
+        return -1;
     }
 
     int nret = snprintf(driver_home, PATH_MAX, "%s/%s", opts->storage_root, opts->driver_name);
     if (nret < 0 || (size_t)nret >= PATH_MAX) {
         ERROR("Sprintf graph driver path failed");
-        ret = -1;
-        goto out;
+        return -1;
     }
 
     for (i = 0; i < g_numdrivers; i++) {
         if (strcmp(opts->driver_name, g_drivers[i].name) == 0) {
             if (pthread_rwlock_init(&(g_drivers[i].rwlock), NULL) != 0) {
                 ERROR("Failed to init driver rwlock");
-                ret = -1;
-                goto out;
+                return -1;
             }
 #ifdef ENABLE_REMOTE_LAYER_STORE
             g_drivers[i].enable_remote_layer = opts->enable_remote_layer;
 #endif
             if (g_drivers[i].ops->init(&g_drivers[i], driver_home, (const char **)opts->driver_opts,
                                        opts->driver_opts_len) != 0) {
-                ret = -1;
-                goto out;
+                return -1;
             }
             g_graphdriver = &g_drivers[i];
             break;
@@ -159,12 +154,10 @@ int graphdriver_init(const struct storage_module_init_options *opts)
 
     if (i == g_numdrivers) {
         ERROR("unsupported driver %s", opts->driver_name);
-        ret = -1;
-        goto out;
+        return -1;
     }
 
-out:
-    return ret;
+    return 0;
 }
 
 int graphdriver_create_rw(const char *id, const char *parent, struct driver_create_opts *create_opts)
diff --git a/src/daemon/modules/image/oci/storage/layer_store/graphdriver/overlay2/driver_overlay2.c b/src/daemon/modules/image/oci/storage/layer_store/graphdriver/overlay2/driver_overlay2.c
index 6d45f463..cc24909a 100644
--- a/src/daemon/modules/image/oci/storage/layer_store/graphdriver/overlay2/driver_overlay2.c
+++ b/src/daemon/modules/image/oci/storage/layer_store/graphdriver/overlay2/driver_overlay2.c
@@ -64,6 +64,9 @@ struct io_read_wrapper;
 
 #define QUOTA_SIZE_OPTION "overlay2.size"
 #define QUOTA_BASESIZE_OPTIONS "overlay2.basesize"
+#define OVERRIDE_KERNELCHECK_OPTIONS "overlay2.override_kernel_check"
+#define SKIP_MOUNT_HOME_OPTIONS "overlay2.skip_mount_home"
+#define MOUNT_OPTIONS "overlay2.mountopt"
 // MAX_LAYER_ID_LENGTH represents the number of random characters which can be used to create the unique link identifer
 // for every layer. If this value is too long then the page size limit for the mount command may be exceeded.
 // The idLength should be selected such that following equation is true (512 is a buffer for label metadata).
@@ -150,7 +153,7 @@ static int overlay2_parse_options(struct graphdriver *driver, const char **optio
                 goto out;
             }
             overlay_opts->default_quota = converted;
-        } else if (strcasecmp(dup, "overlay2.override_kernel_check") == 0) {
+        } else if (strcasecmp(dup, OVERRIDE_KERNELCHECK_OPTIONS) == 0) {
             bool converted_bool = 0;
             ret = util_str_to_bool(val, &converted_bool);
             if (ret != 0) {
@@ -160,7 +163,7 @@ static int overlay2_parse_options(struct graphdriver *driver, const char **optio
                 goto out;
             }
             overlay_opts->override_kernelcheck = converted_bool;
-        } else if (strcasecmp(dup, "overlay2.skip_mount_home") == 0) {
+        } else if (strcasecmp(dup, SKIP_MOUNT_HOME_OPTIONS) == 0) {
             bool converted_bool = 0;
             ret = util_str_to_bool(val, &converted_bool);
             if (ret != 0) {
@@ -170,7 +173,7 @@ static int overlay2_parse_options(struct graphdriver *driver, const char **optio
                 goto out;
             }
             overlay_opts->skip_mount_home = converted_bool;
-        } else if (strcasecmp(dup, "overlay2.mountopt") == 0) {
+        } else if (strcasecmp(dup, MOUNT_OPTIONS) == 0) {
             overlay_opts->mount_options = util_strdup_s(val);
         } else {
             ERROR("Overlay2: unknown option: '%s'", dup);
@@ -693,6 +696,10 @@ static char *get_lower(const char *parent, const char *driver_home)
         goto out;
     }
 
+    /*
+     * lower format: "l/5697636c0104156cb2bd94be25", so "/" and "\0" must be
+     * counted in the size for snprintf.
+     */
     lower_len = strlen(OVERLAY_LINK_DIR) + 1 + strlen(parent_link) + 1;
 
     parent_lower_file = util_path_join(parent_dir, OVERLAY_LAYER_LOWER);
@@ -707,6 +714,11 @@ static char *get_lower(const char *parent, const char *driver_home)
             ERROR("parent lower %s too large", parent_link_file);
             goto out;
         }
+        /*
+         * with parent link, the lower format will be like
+         * "l/5697636c0104156cb2bd94be25:l/df53b618a57bb50a61755b5623",
+         * so ":" must be counted.
+         */
         lower_len = lower_len + strlen(parent_lowers) + 1;
     }
 
@@ -911,7 +923,7 @@ static int do_create_remote_ro(const char *id, const char *parent, const struct
 #ifdef ENABLE_USERNS_REMAP
     if (set_file_owner_for_userns_remap(layer_dir, userns_remap) != 0) {
         ERROR("Unable to change directory %s owner for user remap.", layer_dir);
-        goto out;
+        goto err_out;
     }
 #endif
 
@@ -977,7 +989,7 @@ static int do_create(const char *id, const char *parent, const struct graphdrive
     if (set_file_owner_for_userns_remap(layer_dir, userns_remap) != 0) {
         ERROR("Unable to change directory %s owner for user remap.", layer_dir);
         ret = -1;
-        goto out;
+        goto err_out;
     }
 #endif
 
@@ -1790,7 +1802,7 @@ out:
     return ret;
 }
 
-bool is_valid_layer_link(const char *link_id, const struct graphdriver *driver)
+static bool is_valid_layer_link(const char *link_id, const struct graphdriver *driver)
 {
     bool valid = false;
     char *link_dir = NULL;
-- 
2.23.0