Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang warning fixes #4482

Open
wants to merge 17 commits into
base: devel
Choose a base branch
from
Open

Clang warning fixes #4482

wants to merge 17 commits into from

Conversation

mykaul
Copy link
Contributor

@mykaul mykaul commented Feb 17, 2025

This is a series of independent patches (can be cherry-picked independently if needed) that fix warnings that clang (19.1.7) complained when Gluster was compiled with it.

Signed-off-by: Yaniv Kaul [email protected]

Fix:
make[4]: Entering directory '/home/ykaul/github/glusterfs/xlators/storage/posix/src'
  CC       posix-entry-ops.lo
posix-entry-ops.c:102:5: warning: Value stored to 'dir_handle' is never read [deadcode.DeadStores]
  102 |     dir_handle = alloca0(handle_size);
      |     ^            ~~~~~~~~~~~~~~~~~~~~
posix-entry-ops.c:222:9: warning: Value stored to 'op_ret' is never read [deadcode.DeadStores]
  222 |         op_ret = op_errno = errno = 0;
      |         ^        ~~~~~~~~~~~~~~~~~~~~
posix-entry-ops.c:222:18: warning: Although the value stored to 'op_errno' is used in the enclosing expression, the value is never actually read from 'op_errno' [deadcode.DeadStores]
  222 |         op_ret = op_errno = errno = 0;
      |                  ^          ~~~~~~~~~
posix-entry-ops.c:1217:9: warning: Value stored to 'locked' is never read [deadcode.DeadStores]
 1217 |         locked = _gf_false;
      |         ^        ~~~~~~~~~
posix-entry-ops.c:2067:9: warning: Value stored to 'locked' is never read [deadcode.DeadStores]
 2067 |         locked = _gf_false;
      |         ^        ~~~~~~~~~
posix-entry-ops.c:2083:9: warning: Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]
 2083 |         LOCK(&newloc->inode->lock);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../libglusterfs/src/glusterfs/locking.h:28:17: note: expanded from macro 'LOCK'
   28 | #define LOCK(x) pthread_mutex_lock(x)
      |                 ^~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
glusterd-snapshot-utils.c:304:13: warning: Access to field 'brick_path' results in a dereference of a null pointer (loaded from variable 'snap_ops') [core.NullDereference]
  304 |             snap_ops->brick_path(snap_mount_dir, brickinfo->origin_path, 0,
      |             ^~~~~~~~~~~~~~~~~~~~

glusterd-snapshot-utils.c:3345:19: warning: Access to field 'deactivate' results in a dereference of a null pointer (loaded from variable 'snap_ops') [core.NullDereference]
 3345 |             ret = snap_ops->deactivate(brickinfo, volinfo->snapshot->snapname,
      |                   ^~~~~~~~~~~~~~~~~~~~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
client.c:2747:48: warning: Although the value stored to 'use_try_lock' is used in the enclosing expression, the value is never actually read from 'use_try_lock' [deadcode.DeadStores]
 2747 |     ret = client_fd_lk_list_empty(lk_ctx_ref, (use_try_lock = _gf_true));
      |                                                ^              ~~~~~~~~

While at it - simplify the code. There's no need for a whole function just to TRY_LOCK.

Fix:
client.c:2318:13: warning: Value stored to 'ret' is never read [deadcode.DeadStores]
 2318 |             ret = client_fini_complete(this);
      |             ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
afr-self-heal-common.c:2465:13: warning: Dereference of null pointer (loaded from variable 'op_errno') [core.NullDereference]
 2465 |     local = AFR_FRAME_INIT(frame, (*op_errno));
      |             ^                       ~~~~~~~~
./afr.h:1075:22: note: expanded from macro 'AFR_FRAME_INIT'
 1075 |             op_errno = ENOMEM;                                                 \
      |             ~~~~~~~~ ^

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
glusterfsd-mgmt.c:1587:46: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
 1587 |             gf_log(THIS->name, GF_LOG_ERROR, "%s", msg);
      |                                              ^~~~
../../libglusterfs/src/glusterfs/logging.h:248:65: note: in definition of macro ‘gf_log’
  248 |         _gf_log(dom, __FILE__, __FUNCTION__, __LINE__, level, ##fmt);          \
      |                                                                 ^~~
glusterfsd-mgmt.c:1587:47: note: format string is defined here
 1587 |             gf_log(THIS->name, GF_LOG_ERROR, "%s", msg);
      |                                               ^~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
glusterd-locks.c: In function 'gd_mgmt_v3_unlock_timer_cbk':
glusterd-locks.c:668:9: warning: 'bt_key_len' may be used uninitialized [-Wmaybe-uninitialized]
  668 |         dict_deln(conf->mgmt_v3_lock_timer, bt_key, bt_key_len);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
glusterd-locks.c:642:9: note: 'bt_key_len' was declared here
  642 |     int bt_key_len = 0;
      |         ^~~~~~~~~~

As well as:
glusterd-locks.c:604:9: warning: Value stored to 'ret' is never read [deadcode.DeadStores]
  604 |         ret = 0;

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
inode.c:1734:9: warning: Value stored to 'mem_pool_size' is never read [deadcode.DeadStores]
 1734 |         mem_pool_size = DEFAULT_INODE_MEMPOOL_ENTRIES;
      |         ^               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fix:
Add a check to ensure the pointer pointing to the gfid is not NULL in __is_root_gfid(), as pointed out by scan-analyzer

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
make[4]: Entering directory '/home/ykaul/github/glusterfs/libglusterfs/src'
  CC       libglusterfs_la-dict.lo
dict.c:384:5: warning: Null pointer passed to 2nd parameter expecting 'nonnull' [core.NonNullParamChecker]
  384 |     memcpy(pair->key, key, key_len + 1);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
cli-cmd.c:166:9: warning: variable 'i' set but not used [-Wunused-but-set-variable]
  166 |     int i = 0;
      |         ^

Signed-off-by: Yaniv Kaul <[email protected]>
@gluster-ant
Copy link
Collaborator

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index 9e1999613..bf77d88d9 100644
--- a/xlators/features/shard/src/shard.c
+++ b/xlators/features/shard/src/shard.c
@@ -1289,7 +1289,8 @@ shard_refresh_internal_dir_cbk(call_frame_t *frame, void *cookie,
 {
     shard_local_t *local = NULL;
     inode_t *linked_inode = NULL;
-    shard_internal_dir_type_t type = (shard_internal_dir_type_t)(unsigned long)cookie;
+    shard_internal_dir_type_t type = (shard_internal_dir_type_t)(unsigned long)
+        cookie;
 
     local = frame->local;
 
@@ -1370,7 +1371,8 @@ shard_lookup_internal_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
 {
     inode_t *link_inode = NULL;
     shard_local_t *local = NULL;
-    shard_internal_dir_type_t type = (shard_internal_dir_type_t)(unsigned long)cookie;
+    shard_internal_dir_type_t type = (shard_internal_dir_type_t)(unsigned long)
+        cookie;
 
     local = frame->local;
 
@@ -5889,7 +5891,8 @@ shard_mkdir_internal_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
 {
     inode_t *link_inode = NULL;
     shard_local_t *local = NULL;
-    shard_internal_dir_type_t type = (shard_internal_dir_type_t)(unsigned long)cookie;
+    shard_internal_dir_type_t type = (shard_internal_dir_type_t)(unsigned long)
+        cookie;
 
     local = frame->local;
 
diff --git a/xlators/features/simple-quota/src/simple-quota.c b/xlators/features/simple-quota/src/simple-quota.c
index 244458a99..5a928fc71 100644
--- a/xlators/features/simple-quota/src/simple-quota.c
+++ b/xlators/features/simple-quota/src/simple-quota.c
@@ -129,7 +129,7 @@ sq_set_ns_hardlimit(xlator_t *this, inode_t *inode, int64_t limit, int64_t size,
     ret = inode_ctx_put(inode, this, (uint64_t)(uintptr_t)sq_ctx);
     if (IS_ERROR(ret)) {
         GF_FREE(sq_ctx);
-	sq_ctx = NULL;
+        sq_ctx = NULL;
         goto out;
     }
 
@@ -217,7 +217,9 @@ sq_update_brick_usage(xlator_t *this, inode_t *inode)
     if (!tmp_mq) {
         goto out;
     }
-    loc_t loc = { 0, };
+    loc_t loc = {
+        0,
+    };
     loc.inode = inode_ref(inode);
     int ret = syncop_getxattr(FIRST_CHILD(this), &loc, &dict, SQUOTA_SIZE_KEY,
                               NULL, NULL);
@@ -262,7 +264,7 @@ sq_update_hard_limit(xlator_t *this, inode_t *ns, int64_t limit, int64_t size)
            limit, size);
     sq_ctx->hard_lim = limit;
 
-    //GF_ASSERT(size > 0);
+    // GF_ASSERT(size > 0);
 
 out:
     return;
diff --git a/xlators/protocol/client/src/client.h b/xlators/protocol/client/src/client.h
index d974650fa..96bf86fc8 100644
--- a/xlators/protocol/client/src/client.h
+++ b/xlators/protocol/client/src/client.h
@@ -95,8 +95,8 @@ typedef struct clnt_conf {
     rpc_clnt_prog_t *dump;
 
     int client_id;
-    int event_threads; /* # of event threads
-                        * configured */
+    int event_threads;        /* # of event threads
+                               * configured */
     uint64_t reopen_fd_count; /* Count of fds reopened after a
                                  connection is established */
     gf_lock_t rec_lock;
@@ -123,7 +123,7 @@ typedef struct clnt_conf {
                                   */
     gf_boolean_t filter_o_direct; /* if set, filter O_DIRECT from
                                      the flags list of open() */
-    gf_boolean_t send_gids; /* let the server resolve gids */
+    gf_boolean_t send_gids;       /* let the server resolve gids */
 
     gf_boolean_t destroy; /* if enabled implies fini was called
                            * on @this xlator instance */
@@ -136,7 +136,7 @@ typedef struct clnt_conf {
                                       * logged
                                       */
 
-    gf_boolean_t old_protocol;         /* used only for old-protocol testing */
+    gf_boolean_t old_protocol; /* used only for old-protocol testing */
     gf_boolean_t fini_completed;
     gf_boolean_t strict_locks; /* When set, doesn't reopen saved fds after
                                   reconnect if POSIX locks are held on them.
@@ -145,8 +145,8 @@ typedef struct clnt_conf {
                                   complaince as bricks cleanup any granted
                                   locks when a client disconnects.
                                */
-    gf_boolean_t connection_to_brick; /*True from attempt to connect to brick
-                                        till disconnection to brick*/
+    gf_boolean_t connection_to_brick;  /*True from attempt to connect to brick
+                                         till disconnection to brick*/
     pthread_cond_t fini_complete_cond; /* Used to wait till we finsh the fini
                                           compltely, ie client_fini_complete
                                           to return*/

@gluster-ant
Copy link
Collaborator

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index 244458a99..5a928fc71 100644
--- a/xlators/features/simple-quota/src/simple-quota.c
+++ b/xlators/features/simple-quota/src/simple-quota.c
@@ -129,7 +129,7 @@ sq_set_ns_hardlimit(xlator_t *this, inode_t *inode, int64_t limit, int64_t size,
     ret = inode_ctx_put(inode, this, (uint64_t)(uintptr_t)sq_ctx);
     if (IS_ERROR(ret)) {
         GF_FREE(sq_ctx);
-	sq_ctx = NULL;
+        sq_ctx = NULL;
         goto out;
     }
 
@@ -217,7 +217,9 @@ sq_update_brick_usage(xlator_t *this, inode_t *inode)
     if (!tmp_mq) {
         goto out;
     }
-    loc_t loc = { 0, };
+    loc_t loc = {
+        0,
+    };
     loc.inode = inode_ref(inode);
     int ret = syncop_getxattr(FIRST_CHILD(this), &loc, &dict, SQUOTA_SIZE_KEY,
                               NULL, NULL);
@@ -262,7 +264,7 @@ sq_update_hard_limit(xlator_t *this, inode_t *ns, int64_t limit, int64_t size)
            limit, size);
     sq_ctx->hard_lim = limit;
 
-    //GF_ASSERT(size > 0);
+    // GF_ASSERT(size > 0);
 
 out:
     return;
diff --git a/xlators/protocol/client/src/client.h b/xlators/protocol/client/src/client.h
index d974650fa..96bf86fc8 100644
--- a/xlators/protocol/client/src/client.h
+++ b/xlators/protocol/client/src/client.h
@@ -95,8 +95,8 @@ typedef struct clnt_conf {
     rpc_clnt_prog_t *dump;
 
     int client_id;
-    int event_threads; /* # of event threads
-                        * configured */
+    int event_threads;        /* # of event threads
+                               * configured */
     uint64_t reopen_fd_count; /* Count of fds reopened after a
                                  connection is established */
     gf_lock_t rec_lock;
@@ -123,7 +123,7 @@ typedef struct clnt_conf {
                                   */
     gf_boolean_t filter_o_direct; /* if set, filter O_DIRECT from
                                      the flags list of open() */
-    gf_boolean_t send_gids; /* let the server resolve gids */
+    gf_boolean_t send_gids;       /* let the server resolve gids */
 
     gf_boolean_t destroy; /* if enabled implies fini was called
                            * on @this xlator instance */
@@ -136,7 +136,7 @@ typedef struct clnt_conf {
                                       * logged
                                       */
 
-    gf_boolean_t old_protocol;         /* used only for old-protocol testing */
+    gf_boolean_t old_protocol; /* used only for old-protocol testing */
     gf_boolean_t fini_completed;
     gf_boolean_t strict_locks; /* When set, doesn't reopen saved fds after
                                   reconnect if POSIX locks are held on them.
@@ -145,8 +145,8 @@ typedef struct clnt_conf {
                                   complaince as bricks cleanup any granted
                                   locks when a client disconnects.
                                */
-    gf_boolean_t connection_to_brick; /*True from attempt to connect to brick
-                                        till disconnection to brick*/
+    gf_boolean_t connection_to_brick;  /*True from attempt to connect to brick
+                                         till disconnection to brick*/
     pthread_cond_t fini_complete_cond; /* Used to wait till we finsh the fini
                                           compltely, ie client_fini_complete
                                           to return*/

Fix:
make[5]: Entering directory '/home/ykaul/github/glusterfs/xlators/features/simple-quota/src'
  CC       simple-quota.lo
simple-quota.c:532:20: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
  532 |         if ((nlink == 1)) {
      |              ~~~~~~^~~~
simple-quota.c:532:20: note: remove extraneous parentheses around the comparison to silence this warning
  532 |         if ((nlink == 1)) {
      |             ~      ^   ~
simple-quota.c:532:20: note: use '=' to turn this equality comparison into an assignment
  532 |         if ((nlink == 1)) {
      |                    ^~
      |                    =
simple-quota.c:1017:20: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
 1017 |         if ((nlink == 1)) {
      |              ~~~~~~^~~~
simple-quota.c:1017:20: note: remove extraneous parentheses around the comparison to silence this warning
 1017 |         if ((nlink == 1)) {
      |             ~      ^   ~
simple-quota.c:1017:20: note: use '=' to turn this equality comparison into an assignment
 1017 |         if ((nlink == 1)) {
      |                    ^~
      |                    =

Signed-off-by: Yaniv Kaul <[email protected]>

simple-quota.c: fix clang-format
Fix:
make[5]: Entering directory '/home/ykaul/github/glusterfs/xlators/performance/write-behind/src'
  CC       write-behind.lo
write-behind.c:537:34: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
  537 |             req->ordering.append = 1;
      |                                  ^ ~
write-behind.c:719:29: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
  719 |     req->ordering.fulfilled = 1;
      |                             ^ ~
write-behind.c:1270:32: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
 1270 |             req->ordering.lied = 1;
      |                                ^ ~
write-behind.c:1407:41: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
 1407 |                     holder->ordering.go = 1;
      |                                         ^ ~
write-behind.c:1420:33: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
 1420 |             holder->ordering.go = 1;
      |                                 ^ ~
write-behind.c:1426:33: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
 1426 |             holder->ordering.go = 1;
      |                                 ^ ~
write-behind.c:1432:33: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
 1432 |             holder->ordering.go = 1;
      |                                 ^ ~
write-behind.c:1440:33: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
 1440 |             holder->ordering.go = 1;
      |                                 ^ ~
write-behind.c:1470:29: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
 1470 |         holder->ordering.go = 1;
      |                             ^ ~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
make[5]: Entering directory '/home/ykaul/github/glusterfs/xlators/protocol/server/src'
  CC       server-rpc-fops_v2.lo
server-rpc-fops_v2.c:6198:48: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6197 |     [GFS3_OP_COPY_FILE_RANGE] = {"COPY-FILE-RANGE", server4_0_copy_file_range,
      |                                 ~
 6198 |                                  NULL, DRC_NA, GFS3_OP_COPY_FILE_RANGE, 0},
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~
server-rpc-fops_v2.c:6196:27: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6195 |     [GFS3_OP_NAMELINK] = {"NAMELINK", server4_0_namelink, NULL, DRC_NA,
      |                          ~
 6196 |                           GFS3_OP_NAMELINK, 0},
      |                           ^~~~~~~~~~~~~~~~
server-rpc-fops_v2.c:6194:26: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6193 |     [GFS3_OP_ICREATE] = {"ICREATE", server4_0_icreate, NULL, DRC_NA,
      |                         ~
 6194 |                          GFS3_OP_ICREATE, 0},
      |                          ^~~~~~~~~~~~~~~
server-rpc-fops_v2.c:6190:30: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6189 |     [GFS3_OP_SETACTIVELK] = {"SETACTIVELK", server4_0_setactivelk, NULL, DRC_NA,
      |                             ~
 6190 |                              GFS3_OP_SETACTIVELK, 0},
      |                              ^~~~~~~~~~~~~~~~~~~
server-rpc-fops_v2.c:6188:30: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6187 |     [GFS3_OP_GETACTIVELK] = {"GETACTIVELK", server4_0_getactivelk, NULL, DRC_NA,
      |                             ~
 6188 |                              GFS3_OP_GETACTIVELK, 0},
      |                              ^~~~~~~~~~~~~~~~~~~
server-rpc-fops_v2.c:6185:64: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6185 |     [GFS3_OP_LEASE] = {"LEASE", server4_0_lease, NULL, DRC_NA, GFS3_OP_LEASE,
      |                       ~                                        ^~~~~~~~~~~~~
server-rpc-fops_v2.c:6192:27: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6191 |     [GFS3_OP_COMPOUND] = {"COMPOUND", server4_0_compound, NULL, DRC_NA,
      |                          ~
 6192 |                           GFS3_OP_COMPOUND, 0},
      |                           ^~~~~~~~~~~~~~~~
server-rpc-fops_v2.c:6184:61: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6184 |     [GFS3_OP_SEEK] = {"SEEK", server4_0_seek, NULL, DRC_NA, GFS3_OP_SEEK, 0},
      |                      ~                                      ^~~~~~~~~~~~
server-rpc-fops_v2.c:6183:58: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6183 |     [GFS3_OP_IPC] = {"IPC", server4_0_ipc, NULL, DRC_NA, GFS3_OP_IPC, 0},
      |                     ~                                    ^~~~~~~~~~~
server-rpc-fops_v2.c:6182:27: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6181 |     [GFS3_OP_ZEROFILL] = {"ZEROFILL", server4_0_zerofill, NULL, DRC_NA,
      |                          ~
 6182 |                           GFS3_OP_ZEROFILL, 0},
      |                           ^~~~~~~~~~~~~~~~
server-rpc-fops_v2.c:6180:26: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6179 |     [GFS3_OP_DISCARD] = {"DISCARD", server4_0_discard, NULL, DRC_NA,
      |                         ~
 6180 |                          GFS3_OP_DISCARD, 0},
      |                          ^~~~~~~~~~~~~~~
server-rpc-fops_v2.c:6178:28: warning: implicit conversion from enumeration type 'enum gf_fop_procnum' to different enumeration type 'drc_op_type_t' (aka 'enum drc_op_type') [-Wenum-conversion]
 6177 |     [GFS3_OP_FALLOCATE] = {"FALLOCATE", server4_0_fallocate, NULL, DRC_NA,
      |                           ~
 6178 |                            GFS3_OP_FALLOCATE, 0},
      |                            ^~~~~~~~~~~~~~~~~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
dht-rebalance.c:2259:33: warning: implicit conversion from enumeration type 'enum entrylk_cmd' to different enumeration type 'entrylk_type' (aka 'enum entrylk_type') [-Wenum-conversion]
 2257 |         lk_ret = syncop_entrylk(hashed_subvol, DHT_ENTRY_SYNC_DOMAIN,
      |                  ~~~~~~~~~~~~~~
 2258 |                                 &parent_loc, loc->name, ENTRYLK_UNLOCK,
 2259 |                                 ENTRYLK_UNLOCK, NULL, NULL);
      |                                 ^~~~~~~~~~~~~~

This is not very clear - there's a very clear copy-pasta here, but is the fix to turn it into ENTRYLK_WRLCK ?
Every other caller seem to think so, so this is the change.

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
glfs-fops.c:5965:26: warning: implicit conversion from enumeration type 'gf_upcall_event_t' to different enumeration type 'enum glfs_upcall_reason' [-Wenum-conversion]
 5965 |         up_arg->reason = GF_UPCALL_EVENT_NULL;
      |                        ~ ^~~~~~~~~~~~~~~~~~~~

  CC       libgfapi_la-glfs-fops.lo
glfs-fops.c:6621:28: warning: implicit conversion from enumeration type 'gf_lease_cmds_t' (aka 'enum gf_lease_cmds_t') to different enumeration type 'glfs_lease_cmds_t' (aka 'enum glfs_lease_cmds') [-Wenum-conversion]
 6621 |     lease->cmd = gf_lease->cmd;
      |                ~ ~~~~~~~~~~^~~
glfs-fops.c:6630:28: warning: implicit conversion from enumeration type 'glfs_lease_cmds_t' (aka 'enum glfs_lease_cmds') to different enumeration type 'gf_lease_cmds_t' (aka 'enum gf_lease_cmds_t') [-Wenum-conversion]
 6630 |     gf_lease->cmd = lease->cmd;
      |                   ~ ~~~~~~~^~~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix:
client_t.c:601:25: warning: address of array 'client->client_uid' will always evaluate to 'true' [-Wpointer-bool-conversion]
  601 |             if (client->client_uid) {
      |             ~~  ~~~~~~~~^~~~~~~~~~

Signed-off-by: Yaniv Kaul <[email protected]>

client.c/h: clang-format fixes
Fix:
common-utils.c:1590:10: warning: implicit conversion from 'unsigned long' to 'double' changes value from 18446744073709551615 to 18446744073709551616 [-Wimplicit-const-int-float-conversion]
 1590 |     if ((UINT64_MAX - value) < 0) {
      |          ^~~~~~~~~~ ~
/usr/include/stdint.h:119:23: note: expanded from macro 'UINT64_MAX'
  119 | # define UINT64_MAX             (__UINT64_C(18446744073709551615))
      |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/stdint.h:96:25: note: expanded from macro '__UINT64_C'
   96 | #  define __UINT64_C(c) c ## UL
      |                         ^~~~~~~
<scratch space>:38:1: note: expanded from here
   38 | 18446744073709551615UL
      | ^~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Yaniv Kaul <[email protected]>
Fix the following warning that are seen when compiling with clang:

make[4]: Entering directory '/home/ykaul/github/glusterfs/xlators/features/shard/src'
  CC       shard.lo
shard.c:1292:38: warning: cast to smaller integer type 'shard_internal_dir_type_t' from 'void *' [-Wvoid-pointer-to-enum-cast]
 1292 |     shard_internal_dir_type_t type = (shard_internal_dir_type_t)cookie;
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
shard.c:1373:38: warning: cast to smaller integer type 'shard_internal_dir_type_t' from 'void *' [-Wvoid-pointer-to-enum-cast]
 1373 |     shard_internal_dir_type_t type = (shard_internal_dir_type_t)cookie;
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
shard.c:5892:38: warning: cast to smaller integer type 'shard_internal_dir_type_t' from 'void *' [-Wvoid-pointer-to-enum-cast]
 5892 |     shard_internal_dir_type_t type = (shard_internal_dir_type_t)cookie;
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Yaniv Kaul <[email protected]>
@gluster-ant
Copy link
Collaborator

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index d974650fa..96bf86fc8 100644
--- a/xlators/protocol/client/src/client.h
+++ b/xlators/protocol/client/src/client.h
@@ -95,8 +95,8 @@ typedef struct clnt_conf {
     rpc_clnt_prog_t *dump;
 
     int client_id;
-    int event_threads; /* # of event threads
-                        * configured */
+    int event_threads;        /* # of event threads
+                               * configured */
     uint64_t reopen_fd_count; /* Count of fds reopened after a
                                  connection is established */
     gf_lock_t rec_lock;
@@ -123,7 +123,7 @@ typedef struct clnt_conf {
                                   */
     gf_boolean_t filter_o_direct; /* if set, filter O_DIRECT from
                                      the flags list of open() */
-    gf_boolean_t send_gids; /* let the server resolve gids */
+    gf_boolean_t send_gids;       /* let the server resolve gids */
 
     gf_boolean_t destroy; /* if enabled implies fini was called
                            * on @this xlator instance */
@@ -136,7 +136,7 @@ typedef struct clnt_conf {
                                       * logged
                                       */
 
-    gf_boolean_t old_protocol;         /* used only for old-protocol testing */
+    gf_boolean_t old_protocol; /* used only for old-protocol testing */
     gf_boolean_t fini_completed;
     gf_boolean_t strict_locks; /* When set, doesn't reopen saved fds after
                                   reconnect if POSIX locks are held on them.
@@ -145,8 +145,8 @@ typedef struct clnt_conf {
                                   complaince as bricks cleanup any granted
                                   locks when a client disconnects.
                                */
-    gf_boolean_t connection_to_brick; /*True from attempt to connect to brick
-                                        till disconnection to brick*/
+    gf_boolean_t connection_to_brick;  /*True from attempt to connect to brick
+                                         till disconnection to brick*/
     pthread_cond_t fini_complete_cond; /* Used to wait till we finsh the fini
                                           compltely, ie client_fini_complete
                                           to return*/

@mykaul
Copy link
Contributor Author

mykaul commented Feb 20, 2025

@pranithk - can you please review this? I fixed all clang issues, have no idea what it's complaining about here. Some are benign, but some may be real issues that we wish to get fixed.
As I use clang, it'd be easier to work without seeing all the warnings during compilation (of course there are many many more, mainly on dead assignments)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants