diff options
| -rw-r--r-- | doc/developer-guide/coding-standard.md | 22 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-brick-ops.c | 32 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd.h | 22 |
3 files changed, 54 insertions, 22 deletions
diff --git a/doc/developer-guide/coding-standard.md b/doc/developer-guide/coding-standard.md index eb9cd9ddc39..9bc15a5cc15 100644 --- a/doc/developer-guide/coding-standard.md +++ b/doc/developer-guide/coding-standard.md @@ -156,6 +156,28 @@ instead. gf_boolean_t port_inuse[65536]; /* 256KB, this actually happened */ ``` +NOTE: Ideal is to limit the stack array to less than 256 bytes. + + +Character array initializing +---------------------------- + +It is recommended to keep the character array initializing to empty string. + +*Good:* +``` +char msg[1024] = ""; +``` + +Not so much recommended, even though it means the same. + +``` +char msg[1024] = {0,}; +``` + +We recommend above to structure initialization. + + Validate all arguments to a function ------------------------------------ diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c index c3ec249e6ba..eb85b3c263a 100644 --- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c @@ -416,7 +416,7 @@ __glusterd_handle_add_brick (rpcsvc_request_t *req) char *volname = NULL; int brick_count = 0; void *cli_rsp = NULL; - char err_str[2048] = {0,}; + char err_str[2048] = ""; gf_cli_rsp rsp = {0,}; glusterd_volinfo_t *volinfo = NULL; xlator_t *this = NULL; @@ -777,8 +777,8 @@ subvol_matcher_destroy (int *subvols) int glusterd_set_detach_bricks(dict_t *dict, glusterd_volinfo_t *volinfo) { - char key[256] = {0,}; - char value[256] = {0,}; + char key[64] = ""; + char value[2048] = ""; /* hostname + path */ int brick_num = 0; int hot_brick_num = 0; glusterd_brickinfo_t *brickinfo; @@ -885,16 +885,16 @@ __glusterd_handle_remove_brick (rpcsvc_request_t *req) dict_t *dict = NULL; int32_t count = 0; char *brick = NULL; - char key[256] = {0,}; + char key[64] = ""; int i = 1; glusterd_volinfo_t *volinfo = NULL; glusterd_brickinfo_t *brickinfo = NULL; glusterd_brickinfo_t **brickinfo_list = NULL; int *subvols = NULL; - char err_str[2048] = {0}; + char err_str[2048] = ""; gf_cli_rsp rsp = {0,}; void *cli_rsp = NULL; - char vol_type[256] = {0,}; + char vol_type[256] = ""; int32_t replica_count = 0; char *volname = 0; xlator_t *this = NULL; @@ -1319,10 +1319,9 @@ glusterd_op_perform_add_bricks (glusterd_volinfo_t *volinfo, int32_t count, glusterd_brickinfo_t *brickinfo = NULL; glusterd_gsync_status_temp_t param = {0, }; gf_boolean_t restart_needed = 0; - char msg[1024] __attribute__((unused)) = {0, }; int caps = 0; int brickid = 0; - char key[PATH_MAX] = ""; + char key[64] = ""; char *brick_mount_dir = NULL; xlator_t *this = NULL; glusterd_conf_t *conf = NULL; @@ -1521,6 +1520,7 @@ glusterd_op_perform_add_bricks (glusterd_volinfo_t *volinfo, int32_t count, if (ret) goto out; #ifdef HAVE_BD_XLATOR + char msg[1024] = ""; /* Check for VG/thin pool if its BD volume */ if (brickinfo->vg[0]) { ret = glusterd_is_valid_vg (brickinfo, 0, msg); @@ -1674,8 +1674,8 @@ glusterd_op_stage_add_brick (dict_t *dict, char **op_errstr, dict_t *rsp_dict) glusterd_brickinfo_t *brickinfo = NULL; glusterd_volinfo_t *volinfo = NULL; xlator_t *this = NULL; - char msg[2048] = {0,}; - char key[PATH_MAX] = ""; + char msg[4096] = ""; + char key[64] = ""; gf_boolean_t brick_alloc = _gf_false; char *all_bricks = NULL; char *str_ret = NULL; @@ -1997,8 +1997,8 @@ glusterd_remove_brick_validate_bricks (gf1_op_commands cmd, int32_t brick_count, gf_cli_defrag_type cmd_defrag) { char *brick = NULL; - char msg[2048] = {0,}; - char key[256] = {0,}; + char msg[2048] = ""; + char key[64] = ""; glusterd_brickinfo_t *brickinfo = NULL; glusterd_peerinfo_t *peerinfo = NULL; int i = 0; @@ -2140,7 +2140,7 @@ glusterd_op_stage_remove_brick (dict_t *dict, char **op_errstr) glusterd_volinfo_t *volinfo = NULL; char *errstr = NULL; int32_t brick_count = 0; - char msg[2048] = {0,}; + char msg[2048] = ""; int32_t flag = 0; gf1_op_commands cmd = GF_OP_CMD_NONE; char *task_id_str = NULL; @@ -2719,9 +2719,9 @@ glusterd_op_remove_brick (dict_t *dict, char **op_errstr) char *brick = NULL; int32_t count = 0; int32_t i = 1; - char key[256] = {0,}; + char key[64] = ""; int32_t flag = 0; - char err_str[4096] = {0,}; + char err_str[4096] = ""; int need_rebalance = 0; int force = 0; gf1_op_commands cmd = 0; @@ -3243,7 +3243,7 @@ __glusterd_handle_add_tier_brick (rpcsvc_request_t *req) char *volname = NULL; int brick_count = 0; void *cli_rsp = NULL; - char err_str[2048] = {0,}; + char err_str[2048] = ""; gf_cli_rsp rsp = {0,}; glusterd_volinfo_t *volinfo = NULL; xlator_t *this = NULL; diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h index c2a69c79419..49a1dc6dc08 100644 --- a/xlators/mgmt/glusterd/src/glusterd.h +++ b/xlators/mgmt/glusterd/src/glusterd.h @@ -141,6 +141,16 @@ struct glusterd_volgen { dict_t *dict; }; +/* Keeping all the paths required in glusterd would + cause many buffer overflow errors, as we append + more defined paths to the brick path, workdir etc etc. + It is better to keep limit on this as lesser value, + so we get an option to continue with all functionalities. + For example, snapname max would be appended on brick-path and + would be stored on workdir... all of these being PATH_MAX, is + not an ideal situation for success. */ +#define VALID_GLUSTERD_PATHMAX (PATH_MAX - (256 + 64)) + typedef struct { struct _volfile_ctx *volfile; pthread_mutex_t mutex; @@ -148,8 +158,8 @@ typedef struct { gf_boolean_t verify_volfile_checksum; gf_boolean_t trace; uuid_t uuid; - char workdir[PATH_MAX]; - char rundir[PATH_MAX]; + char workdir[VALID_GLUSTERD_PATHMAX]; + char rundir[VALID_GLUSTERD_PATHMAX]; rpcsvc_t *rpc; glusterd_svc_t shd_svc; glusterd_svc_t nfs_svc; @@ -212,10 +222,10 @@ typedef enum gf_brick_status { struct glusterd_brickinfo { char hostname[1024]; - char path[PATH_MAX]; - char real_path[PATH_MAX]; - char device_path[PATH_MAX]; - char mount_dir[PATH_MAX]; + char path[VALID_GLUSTERD_PATHMAX]; + char real_path[VALID_GLUSTERD_PATHMAX]; + char device_path[VALID_GLUSTERD_PATHMAX]; + char mount_dir[VALID_GLUSTERD_PATHMAX]; char brick_id[1024];/*Client xlator name, AFR changelog name*/ char fstype [NAME_MAX]; /* Brick file-system type */ char mnt_opts [1024]; /* Brick mount options */ |