glusterfs icon indicating copy to clipboard operation
glusterfs copied to clipboard

clang-format issue

Open Sheetalpamecha opened this issue 3 years ago • 10 comments

Problem - clang-format passes successfully but still shows a warning msg on some of the Pull Requests and suggest edits in the code that is not been changed by respective PR.

Testing - clang format — OK - clang format

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

Solution - I ran the formatting script locally and all these files require formatting changes. No code change has been made.

Updates: #1000 Change-Id: I2499dcaa1337a49191363c210e482fb43de1f5ce Signed-off-by: Sheetal Pamecha [email protected]

Sheetalpamecha avatar Nov 14 '21 12:11 Sheetalpamecha

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

index e3c84705b..4a03d95ba 100644
--- a/libglusterfs/src/glusterfs/common-utils.h
+++ b/libglusterfs/src/glusterfs/common-utils.h
@@ -1104,7 +1104,7 @@ __gf_thread_set_name(pthread_t thread, const char *name)
 #elif defined(HAVE_PTHREAD_SET_NAME_NP)
     pthread_set_name_np(thread, name);
     return 0;
-#else /* none found */
+#else  /* none found */
     return -ENOSYS;
 #endif /* set name */
 }
diff --git a/libglusterfs/src/glusterfs/compat.h b/libglusterfs/src/glusterfs/compat.h
index 364b6f6f0..3d1a0e479 100644
--- a/libglusterfs/src/glusterfs/compat.h
+++ b/libglusterfs/src/glusterfs/compat.h
@@ -188,10 +188,10 @@ enum {
 #define F_GETLK64 F_GETLK
 #define F_SETLK64 F_SETLK
 #define F_SETLKW64 F_SETLKW
-#define FALLOC_FL_KEEP_SIZE 0x01 /* default is extend size */
-#define FALLOC_FL_PUNCH_HOLE 0x02 /* de-allocates range */
-#define FALLOC_FL_ZERO_RANGE 0x10 /* zeroes out range */
-#define FALLOC_FL_INSERT_RANGE 0x20 /* Expands the size */
+#define FALLOC_FL_KEEP_SIZE 0x01      /* default is extend size */
+#define FALLOC_FL_PUNCH_HOLE 0x02     /* de-allocates range */
+#define FALLOC_FL_ZERO_RANGE 0x10     /* zeroes out range */
+#define FALLOC_FL_INSERT_RANGE 0x20   /* Expands the size */
 #define FALLOC_FL_COLLAPSE_RANGE 0x08 /* Reduces the size */
 
 #ifndef _PATH_UMOUNT
@@ -498,7 +498,7 @@ dirname_r(char *path);
 #if defined(__GNUC__) && !defined(RELAX_POISONING)
 /* Use run API, see run.h */
 #include <stdlib.h> /* system(), mkostemp() */
-#include <stdio.h> /* popen() */
+#include <stdio.h>  /* popen() */
 #ifdef GF_LINUX_HOST_OS
 #include <sys/sysmacros.h>
 #endif
diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h
index 422524f03..2c0bc463c 100644
--- a/libglusterfs/src/glusterfs/glusterfs.h
+++ b/libglusterfs/src/glusterfs/glusterfs.h
@@ -284,7 +284,7 @@ enum gf_internal_fop_indicator {
 /* GlusterFS's maximum supported Auxiliary GIDs */
 #define GF_MAX_AUX_GROUPS 65535
 
-#define GF_UUID_BUF_SIZE 37 /* UUID_CANONICAL_FORM_LEN + NULL */
+#define GF_UUID_BUF_SIZE 37          /* UUID_CANONICAL_FORM_LEN + NULL */
 #define GF_UUID_BNAME_BUF_SIZE (320) /* (64 + 256) */
 
 #define GF_REBALANCE_TID_KEY "rebalance-id"
@@ -350,7 +350,7 @@ enum gf_internal_fop_indicator {
 #define GF_BACKTRACE_LEN 4096
 #define GF_BACKTRACE_FRAME_COUNT 7
 
-#define GF_LK_ADVISORY 0 /* maps to GLFS_LK_ADVISORY from libgfapi*/
+#define GF_LK_ADVISORY 0  /* maps to GLFS_LK_ADVISORY from libgfapi*/
 #define GF_LK_MANDATORY 1 /* maps to GLFS_LK_MANDATORY from libgfapi*/
 #define GF_LOCK_MODE "glusterfs.lk.lkmode"
 
@@ -442,9 +442,9 @@ const char *
 fop_enum_to_pri_string(glusterfs_fop_t fop);
 
 #define GF_SET_IF_NOT_PRESENT 0x1 /* default behaviour */
-#define GF_SET_OVERWRITE 0x2 /* Overwrite with the buf given */
+#define GF_SET_OVERWRITE 0x2      /* Overwrite with the buf given */
 #define GF_SET_DIR_ONLY 0x4
-#define GF_SET_EPOCH_TIME 0x8 /* used by afr dir lookup selfheal */
+#define GF_SET_EPOCH_TIME 0x8    /* used by afr dir lookup selfheal */
 #define GF_AUXILLARY_PARGFID 0xd /* RIO dummy parent gfid */
 
 /* key value which quick read uses to get small files in lookup cbk */
diff --git a/libglusterfs/src/glusterfs/mem-pool.h b/libglusterfs/src/glusterfs/mem-pool.h
index 370f5ca22..29208dffa 100644
--- a/libglusterfs/src/glusterfs/mem-pool.h
+++ b/libglusterfs/src/glusterfs/mem-pool.h
@@ -242,10 +242,10 @@ struct mem_pool {
     unsigned long count; /* requested pool size (unused) */
     char *name;
     char *xl_name;
-    gf_atomic_t active; /* current allocations */
+    gf_atomic_t active;     /* current allocations */
 #ifdef DEBUG
-    gf_atomic_t hit;  /* number of allocations served from pt_pool */
-    gf_atomic_t miss; /* number of std allocs due to miss */
+    gf_atomic_t hit;        /* number of allocations served from pt_pool */
+    gf_atomic_t miss;       /* number of std allocs due to miss */
 #endif
     struct list_head owner; /* glusterfs_ctx_t->mempool_list */
     glusterfs_ctx_t *ctx;   /* take ctx->lock when updating owner */
diff --git a/rpc/rpc-lib/src/protocol-common.h b/rpc/rpc-lib/src/protocol-common.h
index 0d0fc45e2..e98ae33fc 100644
--- a/rpc/rpc-lib/src/protocol-common.h
+++ b/rpc/rpc-lib/src/protocol-common.h
@@ -330,16 +330,16 @@ enum gf_getspec_flags_type { GF_GETSPEC_FLAG_SERVERS_LIST = 1 };
 typedef enum gf_getspec_flags_type gf_getspec_flags_type;
 
 #define GLUSTER_HNDSK_PROGRAM 14398633 /* Completely random */
-#define GLUSTER_HNDSK_VERSION 2 /* 0.0.2 */
+#define GLUSTER_HNDSK_VERSION 2        /* 0.0.2 */
 
 #define GLUSTER_PMAP_PROGRAM 34123456
 #define GLUSTER_PMAP_VERSION 1
 
 #define GLUSTER_CBK_PROGRAM 52743234 /* Completely random */
-#define GLUSTER_CBK_VERSION 1 /* 0.0.1 */
+#define GLUSTER_CBK_VERSION 1        /* 0.0.1 */
 
 #define GLUSTER_FOP_PROGRAM 1298437 /* Completely random */
-#define GLUSTER_FOP_VERSION 330 /* 3.3.0 */
+#define GLUSTER_FOP_VERSION 330     /* 3.3.0 */
 #define GLUSTER_FOP_PROCCNT GFS3_OP_MAXVALUE
 
 #define GLUSTER_FOP_VERSION_v2 400 /* 4.0.0 */
@@ -350,13 +350,13 @@ typedef enum gf_getspec_flags_type gf_getspec_flags_type;
 
 /* Second version */
 #define GD_MGMT_PROGRAM 1238433 /* Completely random */
-#define GD_MGMT_VERSION 2 /* 0.0.2 */
+#define GD_MGMT_VERSION 2       /* 0.0.2 */
 
 #define GD_FRIEND_PROGRAM 1238437 /* Completely random */
-#define GD_FRIEND_VERSION 2 /* 0.0.2 */
+#define GD_FRIEND_VERSION 2       /* 0.0.2 */
 
 #define GLUSTER_CLI_PROGRAM 1238463 /* Completely random */
-#define GLUSTER_CLI_VERSION 2 /* 0.0.2 */
+#define GLUSTER_CLI_VERSION 2       /* 0.0.2 */
 
 #define GD_BRICK_PROGRAM 4867634 /*Completely random*/
 #define GD_BRICK_VERSION 2
diff --git a/rpc/rpc-lib/src/rpcsvc.h b/rpc/rpc-lib/src/rpcsvc.h
index dfd527159..783289276 100644
--- a/rpc/rpc-lib/src/rpcsvc.h
+++ b/rpc/rpc-lib/src/rpcsvc.h
@@ -106,13 +106,13 @@ typedef enum {
 
 #define AUTH_NONE 0 /* no authentication */
 #define AUTH_NULL 0 /* backward compatibility */
-#define AUTH_SYS 1 /* unix style (uid, gids) */
+#define AUTH_SYS 1  /* unix style (uid, gids) */
 #define AUTH_UNIX AUTH_SYS
-#define AUTH_SHORT 2 /* short hand unix style */
-#define AUTH_DES 3 /* des style (encrypted timestamps) */
+#define AUTH_SHORT 2     /* short hand unix style */
+#define AUTH_DES 3       /* des style (encrypted timestamps) */
 #define AUTH_DH AUTH_DES /* Diffie-Hellman (this is DES) */
-#define AUTH_KERB 4 /* kerberos style */
-#endif /* */
+#define AUTH_KERB 4      /* kerberos style */
+#endif                   /* */
 
 typedef struct rpcsvc_program rpcsvc_program_t;
 
diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c
index 1f22b5e65..b79d56404 100644
--- a/rpc/rpc-transport/socket/src/socket.c
+++ b/rpc/rpc-transport/socket/src/socket.c
@@ -4102,7 +4102,7 @@ threadid_func(CRYPTO_THREADID *id)
      */
     CRYPTO_THREADID_set_numeric(id, (unsigned long)pthread_self());
 }
-#else /* older openssl */
+#else  /* older openssl */
 static unsigned long
 legacy_threadid_func(void)
 {
@@ -4358,7 +4358,7 @@ ssl_setup_connection_params(rpc_transport_t *this)
                        "DH ciphers are disabled.",
                        dh_param, ERR_error_string(err, NULL));
             }
-#else /* HAVE_OPENSSL_DH_H */
+#else  /* HAVE_OPENSSL_DH_H */
             BIO_free(bio);
             gf_log(this->name, GF_LOG_ERROR, "OpenSSL has no DH support");
 #endif /* HAVE_OPENSSL_DH_H */
@@ -4385,7 +4385,7 @@ ssl_setup_connection_params(rpc_transport_t *this)
                        "ECDH ciphers are disabled.",
                        ec_curve, ERR_error_string(err, NULL));
             }
-#else /* HAVE_OPENSSL_ECDH_H */
+#else  /* HAVE_OPENSSL_ECDH_H */
             gf_log(this->name, GF_LOG_ERROR, "OpenSSL has no ECDH support");
 #endif /* HAVE_OPENSSL_ECDH_H */
         }
diff --git a/xlators/features/compress/src/cdc.h b/xlators/features/compress/src/cdc.h
index f164a9ca7..33e0bffd9 100644
--- a/xlators/features/compress/src/cdc.h
+++ b/xlators/features/compress/src/cdc.h
@@ -50,7 +50,7 @@ typedef struct cdc_info {
 
 /* Gzip defaults */
 #define GF_CDC_DEF_WINDOWSIZE -15 /* default value */
-#define GF_CDC_MAX_WINDOWSIZE -8 /* max value     */
+#define GF_CDC_MAX_WINDOWSIZE -8  /* max value     */
 
 #define GF_CDC_DEF_MEMLEVEL 8
 #define GF_CDC_DEF_BUFFERSIZE 262144  // 256K - default compression buffer size
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index 6b7af6422..7d4bb5de7 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -935,7 +935,7 @@ out:
     return ret ? -1 : 0;
 }
 #undef RUN_GSYNCD_CMD
-#else /* SYNCDAEMON_COMPILE */
+#else  /* SYNCDAEMON_COMPILE */
 static int
 configure_syncdaemon(glusterd_conf_t *conf)
 {
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
index 78f39de02..110af8e3d 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -1321,7 +1321,7 @@ posix_fhandle_pair(call_frame_t *frame, xlator_t *this, int fd, char *key,
                        "fd=%d: key:%s", fd, key);
             }
 
-#else /* ! DARWIN */
+#else  /* ! DARWIN */
             gf_msg(this->name, GF_LOG_ERROR, errno, P_MSG_XATTR_FAILED,
                    "fd=%d: key:%s", fd, key);
 #endif /* DARWIN */
diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c
index 26ab6b055..fb7b7f004 100644
--- a/xlators/storage/posix/src/posix-inode-fd-ops.c
+++ b/xlators/storage/posix/src/posix-inode-fd-ops.c
@@ -1108,7 +1108,7 @@ posix_discard(call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset,
 #ifndef FALLOC_FL_KEEP_SIZE
     ret = EOPNOTSUPP;
 
-#else /* FALLOC_FL_KEEP_SIZE */
+#else  /* FALLOC_FL_KEEP_SIZE */
     int32_t flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE;
     struct iatt statpre = {
         0,

gluster-ant avatar Nov 14 '21 12:11 gluster-ant

/run regression

Sheetalpamecha avatar Nov 14 '21 16:11 Sheetalpamecha

@Sheetalpamecha as per the clang format result on this patch, it is asking for few more changes and reverting some of the changes done by this patch. May be due to the version mismatch between your local system and the one running on the builders.

karthik-us avatar Nov 16 '21 07:11 karthik-us

Yes that can be possible cause. I am tracking it in build-jobs repo too if we need to fail this job as and when change is made. Issue no- https://github.com/gluster/build-jobs/issues/112

Sheetalpamecha avatar Nov 17 '21 03:11 Sheetalpamecha

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

index 77531fbf7..bf338b0e7 100644
--- a/libglusterfs/src/glusterfs/common-utils.h
+++ b/libglusterfs/src/glusterfs/common-utils.h
@@ -1091,7 +1091,7 @@ __gf_thread_set_name(pthread_t thread, const char *name)
 #elif defined(HAVE_PTHREAD_SET_NAME_NP)
     pthread_set_name_np(thread, name);
     return 0;
-#else /* none found */
+#else  /* none found */
     return -ENOSYS;
 #endif /* set name */
 }
diff --git a/libglusterfs/src/glusterfs/compat.h b/libglusterfs/src/glusterfs/compat.h
index 364b6f6f0..3d1a0e479 100644
--- a/libglusterfs/src/glusterfs/compat.h
+++ b/libglusterfs/src/glusterfs/compat.h
@@ -188,10 +188,10 @@ enum {
 #define F_GETLK64 F_GETLK
 #define F_SETLK64 F_SETLK
 #define F_SETLKW64 F_SETLKW
-#define FALLOC_FL_KEEP_SIZE 0x01 /* default is extend size */
-#define FALLOC_FL_PUNCH_HOLE 0x02 /* de-allocates range */
-#define FALLOC_FL_ZERO_RANGE 0x10 /* zeroes out range */
-#define FALLOC_FL_INSERT_RANGE 0x20 /* Expands the size */
+#define FALLOC_FL_KEEP_SIZE 0x01      /* default is extend size */
+#define FALLOC_FL_PUNCH_HOLE 0x02     /* de-allocates range */
+#define FALLOC_FL_ZERO_RANGE 0x10     /* zeroes out range */
+#define FALLOC_FL_INSERT_RANGE 0x20   /* Expands the size */
 #define FALLOC_FL_COLLAPSE_RANGE 0x08 /* Reduces the size */
 
 #ifndef _PATH_UMOUNT
@@ -498,7 +498,7 @@ dirname_r(char *path);
 #if defined(__GNUC__) && !defined(RELAX_POISONING)
 /* Use run API, see run.h */
 #include <stdlib.h> /* system(), mkostemp() */
-#include <stdio.h> /* popen() */
+#include <stdio.h>  /* popen() */
 #ifdef GF_LINUX_HOST_OS
 #include <sys/sysmacros.h>
 #endif
diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h
index 5e6c57891..709c0ea6e 100644
--- a/libglusterfs/src/glusterfs/glusterfs.h
+++ b/libglusterfs/src/glusterfs/glusterfs.h
@@ -287,7 +287,7 @@ enum gf_internal_fop_indicator {
 /* GlusterFS's maximum supported Auxiliary GIDs */
 #define GF_MAX_AUX_GROUPS 65535
 
-#define GF_UUID_BUF_SIZE 37 /* UUID_CANONICAL_FORM_LEN + NULL */
+#define GF_UUID_BUF_SIZE 37          /* UUID_CANONICAL_FORM_LEN + NULL */
 #define GF_UUID_BNAME_BUF_SIZE (320) /* (64 + 256) */
 
 #define GF_REBALANCE_TID_KEY "rebalance-id"
@@ -353,7 +353,7 @@ enum gf_internal_fop_indicator {
 #define GF_BACKTRACE_LEN 4096
 #define GF_BACKTRACE_FRAME_COUNT 7
 
-#define GF_LK_ADVISORY 0 /* maps to GLFS_LK_ADVISORY from libgfapi*/
+#define GF_LK_ADVISORY 0  /* maps to GLFS_LK_ADVISORY from libgfapi*/
 #define GF_LK_MANDATORY 1 /* maps to GLFS_LK_MANDATORY from libgfapi*/
 #define GF_LOCK_MODE "glusterfs.lk.lkmode"
 
@@ -445,9 +445,9 @@ const char *
 fop_enum_to_pri_string(glusterfs_fop_t fop);
 
 #define GF_SET_IF_NOT_PRESENT 0x1 /* default behaviour */
-#define GF_SET_OVERWRITE 0x2 /* Overwrite with the buf given */
+#define GF_SET_OVERWRITE 0x2      /* Overwrite with the buf given */
 #define GF_SET_DIR_ONLY 0x4
-#define GF_SET_EPOCH_TIME 0x8 /* used by afr dir lookup selfheal */
+#define GF_SET_EPOCH_TIME 0x8    /* used by afr dir lookup selfheal */
 #define GF_AUXILLARY_PARGFID 0xd /* RIO dummy parent gfid */
 
 /* key value which quick read uses to get small files in lookup cbk */
diff --git a/libglusterfs/src/glusterfs/mem-pool.h b/libglusterfs/src/glusterfs/mem-pool.h
index 370f5ca22..29208dffa 100644
--- a/libglusterfs/src/glusterfs/mem-pool.h
+++ b/libglusterfs/src/glusterfs/mem-pool.h
@@ -242,10 +242,10 @@ struct mem_pool {
     unsigned long count; /* requested pool size (unused) */
     char *name;
     char *xl_name;
-    gf_atomic_t active; /* current allocations */
+    gf_atomic_t active;     /* current allocations */
 #ifdef DEBUG
-    gf_atomic_t hit;  /* number of allocations served from pt_pool */
-    gf_atomic_t miss; /* number of std allocs due to miss */
+    gf_atomic_t hit;        /* number of allocations served from pt_pool */
+    gf_atomic_t miss;       /* number of std allocs due to miss */
 #endif
     struct list_head owner; /* glusterfs_ctx_t->mempool_list */
     glusterfs_ctx_t *ctx;   /* take ctx->lock when updating owner */
diff --git a/rpc/rpc-lib/src/protocol-common.h b/rpc/rpc-lib/src/protocol-common.h
index 0d0fc45e2..e98ae33fc 100644
--- a/rpc/rpc-lib/src/protocol-common.h
+++ b/rpc/rpc-lib/src/protocol-common.h
@@ -330,16 +330,16 @@ enum gf_getspec_flags_type { GF_GETSPEC_FLAG_SERVERS_LIST = 1 };
 typedef enum gf_getspec_flags_type gf_getspec_flags_type;
 
 #define GLUSTER_HNDSK_PROGRAM 14398633 /* Completely random */
-#define GLUSTER_HNDSK_VERSION 2 /* 0.0.2 */
+#define GLUSTER_HNDSK_VERSION 2        /* 0.0.2 */
 
 #define GLUSTER_PMAP_PROGRAM 34123456
 #define GLUSTER_PMAP_VERSION 1
 
 #define GLUSTER_CBK_PROGRAM 52743234 /* Completely random */
-#define GLUSTER_CBK_VERSION 1 /* 0.0.1 */
+#define GLUSTER_CBK_VERSION 1        /* 0.0.1 */
 
 #define GLUSTER_FOP_PROGRAM 1298437 /* Completely random */
-#define GLUSTER_FOP_VERSION 330 /* 3.3.0 */
+#define GLUSTER_FOP_VERSION 330     /* 3.3.0 */
 #define GLUSTER_FOP_PROCCNT GFS3_OP_MAXVALUE
 
 #define GLUSTER_FOP_VERSION_v2 400 /* 4.0.0 */
@@ -350,13 +350,13 @@ typedef enum gf_getspec_flags_type gf_getspec_flags_type;
 
 /* Second version */
 #define GD_MGMT_PROGRAM 1238433 /* Completely random */
-#define GD_MGMT_VERSION 2 /* 0.0.2 */
+#define GD_MGMT_VERSION 2       /* 0.0.2 */
 
 #define GD_FRIEND_PROGRAM 1238437 /* Completely random */
-#define GD_FRIEND_VERSION 2 /* 0.0.2 */
+#define GD_FRIEND_VERSION 2       /* 0.0.2 */
 
 #define GLUSTER_CLI_PROGRAM 1238463 /* Completely random */
-#define GLUSTER_CLI_VERSION 2 /* 0.0.2 */
+#define GLUSTER_CLI_VERSION 2       /* 0.0.2 */
 
 #define GD_BRICK_PROGRAM 4867634 /*Completely random*/
 #define GD_BRICK_VERSION 2
diff --git a/rpc/rpc-lib/src/rpcsvc.h b/rpc/rpc-lib/src/rpcsvc.h
index a9ba19b79..83656b403 100644
--- a/rpc/rpc-lib/src/rpcsvc.h
+++ b/rpc/rpc-lib/src/rpcsvc.h
@@ -106,13 +106,13 @@ typedef enum {
 
 #define AUTH_NONE 0 /* no authentication */
 #define AUTH_NULL 0 /* backward compatibility */
-#define AUTH_SYS 1 /* unix style (uid, gids) */
+#define AUTH_SYS 1  /* unix style (uid, gids) */
 #define AUTH_UNIX AUTH_SYS
-#define AUTH_SHORT 2 /* short hand unix style */
-#define AUTH_DES 3 /* des style (encrypted timestamps) */
+#define AUTH_SHORT 2     /* short hand unix style */
+#define AUTH_DES 3       /* des style (encrypted timestamps) */
 #define AUTH_DH AUTH_DES /* Diffie-Hellman (this is DES) */
-#define AUTH_KERB 4 /* kerberos style */
-#endif /* */
+#define AUTH_KERB 4      /* kerberos style */
+#endif                   /* */
 
 typedef struct rpcsvc_program rpcsvc_program_t;
 
diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c
index 8e73e1380..3ecc1f71a 100644
--- a/rpc/rpc-transport/socket/src/socket.c
+++ b/rpc/rpc-transport/socket/src/socket.c
@@ -4097,7 +4097,7 @@ threadid_func(CRYPTO_THREADID *id)
      */
     CRYPTO_THREADID_set_numeric(id, (unsigned long)pthread_self());
 }
-#else /* older openssl */
+#else  /* older openssl */
 static unsigned long
 legacy_threadid_func(void)
 {
@@ -4356,7 +4356,7 @@ ssl_setup_connection_params(rpc_transport_t *this)
                        "DH ciphers are disabled.",
                        dh_param, ERR_error_string(err, NULL));
             }
-#else /* HAVE_OPENSSL_DH_H */
+#else  /* HAVE_OPENSSL_DH_H */
             BIO_free(bio);
             gf_log(this->name, GF_LOG_ERROR, "OpenSSL has no DH support");
 #endif /* HAVE_OPENSSL_DH_H */
@@ -4386,7 +4386,7 @@ ssl_setup_connection_params(rpc_transport_t *this)
                        "ECDH ciphers are disabled.",
                        ec_curve, ERR_error_string(err, NULL));
             }
-#else /* HAVE_OPENSSL_ECDH_H */
+#else  /* HAVE_OPENSSL_ECDH_H */
             gf_log(this->name, GF_LOG_ERROR, "OpenSSL has no ECDH support");
 #endif /* HAVE_OPENSSL_ECDH_H */
         }
diff --git a/xlators/features/compress/src/cdc.h b/xlators/features/compress/src/cdc.h
index f164a9ca7..33e0bffd9 100644
--- a/xlators/features/compress/src/cdc.h
+++ b/xlators/features/compress/src/cdc.h
@@ -50,7 +50,7 @@ typedef struct cdc_info {
 
 /* Gzip defaults */
 #define GF_CDC_DEF_WINDOWSIZE -15 /* default value */
-#define GF_CDC_MAX_WINDOWSIZE -8 /* max value     */
+#define GF_CDC_MAX_WINDOWSIZE -8  /* max value     */
 
 #define GF_CDC_DEF_MEMLEVEL 8
 #define GF_CDC_DEF_BUFFERSIZE 262144  // 256K - default compression buffer size
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index 0676cd57a..f7bd7f265 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -935,7 +935,7 @@ out:
     return ret ? -1 : 0;
 }
 #undef RUN_GSYNCD_CMD
-#else /* SYNCDAEMON_COMPILE */
+#else  /* SYNCDAEMON_COMPILE */
 static int
 configure_syncdaemon(glusterd_conf_t *conf)
 {
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
index db40c367d..0150eda66 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -1321,7 +1321,7 @@ posix_fhandle_pair(call_frame_t *frame, xlator_t *this, int fd, char *key,
                        "fd=%d: key:%s", fd, key);
             }
 
-#else /* ! DARWIN */
+#else  /* ! DARWIN */
             gf_msg(this->name, GF_LOG_ERROR, errno, P_MSG_XATTR_FAILED,
                    "fd=%d: key:%s", fd, key);
 #endif /* DARWIN */
diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c
index 122ef94f4..a57e3e02b 100644
--- a/xlators/storage/posix/src/posix-inode-fd-ops.c
+++ b/xlators/storage/posix/src/posix-inode-fd-ops.c
@@ -1108,7 +1108,7 @@ posix_discard(call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset,
 #ifndef FALLOC_FL_KEEP_SIZE
     ret = EOPNOTSUPP;
 
-#else /* FALLOC_FL_KEEP_SIZE */
+#else  /* FALLOC_FL_KEEP_SIZE */
     int32_t flags = FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE;
     struct iatt statpre = {
         0,

gluster-ant avatar Jan 18 '22 11:01 gluster-ant

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

index 4001d6c2c..9ee5935aa 100644
--- a/xlators/cluster/ec/src/ec.c
+++ b/xlators/cluster/ec/src/ec.c
@@ -43,8 +43,8 @@ static char *ec_read_policies[EC_READ_POLICY_MAX + 1] = {
         }                                                                      \
     } while (0)
 
-    int32_t
-    ec_parse_options(xlator_t *this)
+int32_t
+ec_parse_options(xlator_t *this)
 {
     ec_t *ec = this->private;
     int32_t error = EINVAL;

gluster-ant avatar Jan 18 '22 17:01 gluster-ant

/run regression

Sheetalpamecha avatar Jan 18 '22 18:01 Sheetalpamecha

Some of the changes from clang-format actually make the code uglier and/or harder to read. I think we shouldn't apply the rules automatically (we actually made the clang-format test non mandatory before merging a patch just because of this).

On each PR an automatic comment is added if there are clang-format mismatches. If the owner or reviewers consider that the formatting is important, it will be asked and the PR updated. Otherwise I think we are ok and we shouldn't apply clang-format massively and unconditionally.

Hi Xavi, A few points that I have observed recently -

  • As it is made non-mandatory, a lot of times it tends to be ignored by developers and new contributors which actually makes code harder to read.
  • Also, as glusterfs has for long stick to clang-formatting and now if its ignored, then it does make the code uglier.
  • Additionally It spams the PR for the changes not done in particular PR, as it check the whole file.

And that's the reason for picking it up and correct them and make the job mandatory.

Sheetalpamecha avatar Jan 19 '22 05:01 Sheetalpamecha

Some of the changes from clang-format actually make the code uglier and/or harder to read. I think we shouldn't apply the rules automatically (we actually made the clang-format test non mandatory before merging a patch just because of this). On each PR an automatic comment is added if there are clang-format mismatches. If the owner or reviewers consider that the formatting is important, it will be asked and the PR updated. Otherwise I think we are ok and we shouldn't apply clang-format massively and unconditionally.

Hi Xavi, A few points that I have observed recently -

  • As it is made non-mandatory, a lot of times it tends to be ignored by developers and new contributors which actually makes code harder to read.

I think this should also be enforced by reviewers if they consider that the clang-format changes are significant. That's why the additional message was added, so that the reviewers (and owner) can easily see any potential formatting issue.

Blindly formatting all the code makes the code ugly, specially for structures initialized inline, like xlator options. They are a complete mess hard to read. But there are other cases where the code gets worse or even against the rules, like putting the ''{'' in the same line for loops (this doesn't work for list_for_each()-like loops).

  • Also, as glusterfs has for long stick to clang-formatting and now if its ignored, then it does make the code uglier.

Yes, but it was decided to make it not mandatory just because of the issues it was raising. Even different clang-format versions format code in a different way, specially for some comments.

IMO there are some core rules that should be enforced, but not all the clang-format changes.

  • Additionally It spams the PR for the changes not done in particular PR, as it check the whole file.

Yes, that's the worst part. I think we should try to limit the reported changes to things introduced by the PR only. However I'm not sure about the feasibility of this...

xhernandez avatar Jan 19 '22 09:01 xhernandez

Thank you for your contributions. Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity. It will be closed in 2 weeks if no one responds with a comment here.

stale[bot] avatar Sep 20 '22 19:09 stale[bot]

Closing this issue as there was no update since my last update on issue. If this is an issue which is still valid, feel free to open it.

stale[bot] avatar Oct 16 '22 03:10 stale[bot]