diff options
author | Humble Devassy Chirammal <hchiramm@redhat.com> | 2015-03-20 18:57:52 +0530 |
---|---|---|
committer | Kaleb KEITHLEY <kkeithle@redhat.com> | 2015-03-30 05:31:34 -0700 |
commit | 299f6ce2b17216a6c09d5345139b9a78f7505b24 (patch) | |
tree | 9be7929ddc9618a975d4ed3c341957f25d926150 | |
parent | b247ff4b297481148ab2fe4c7b832aac85f6ad72 (diff) |
libgfapi: revamp glfs_new function
Current glfs_new() function is not flexible enough to error out
and destroy the struct members or objects initialized just before the
error path/condition. This make the structs or objects to continue or
left out with partially recorded data in fs and ctx structs and cause
crashes/issues later in the code path. This patch avoid the issue.
Change-Id: Ie4514b82b24723a46681cc7832a08870afc0cb28
BUG: 1202492
Signed-off-by: Humble Devassy Chirammal <hchiramm@redhat.com>
Reviewed-on: http://review.gluster.org/9903
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Poornima G <pgurusid@redhat.com>
Reviewed-by: soumya k <skoduri@redhat.com>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>
-rw-r--r-- | api/src/glfs.c | 107 | ||||
-rw-r--r-- | libglusterfs/src/mem-pool.h | 12 |
2 files changed, 85 insertions, 34 deletions
diff --git a/api/src/glfs.c b/api/src/glfs.c index 8bd410c716d..4ae4d769471 100644 --- a/api/src/glfs.c +++ b/api/src/glfs.c @@ -556,56 +556,105 @@ glfs_poller (void *data) struct glfs * pub_glfs_new (const char *volname) { - struct glfs *fs = NULL; - int ret = -1; - glusterfs_ctx_t *ctx = NULL; + struct glfs *fs = NULL; + int ret = -1; + glusterfs_ctx_t *ctx = NULL; + gf_boolean_t cleanup_fini = _gf_false; if (!volname) { errno = EINVAL; return NULL; } - ctx = glusterfs_ctx_new (); - if (!ctx) { - return NULL; - } + fs = CALLOC (1, sizeof (*fs)); + if (!fs) + return NULL; - /* first globals init, for gf_mem_acct_enable_set () */ - ret = glusterfs_globals_init (ctx); - if (ret) - return NULL; + ret = pthread_mutex_init (&fs->mutex, NULL); + if (ret != 0) + goto freefs; + + ret = pthread_cond_init (&fs->cond, NULL); + if (ret != 0) + goto mutex_destroy; + + ret = pthread_cond_init (&fs->child_down_cond, NULL); + if (ret != 0) + goto cond_destroy; + + ret = pthread_mutex_init (&fs->upcall_list_mutex, NULL); + if (ret != 0) + goto cond_child_destroy; + + cleanup_fini = _gf_true; + + ctx = glusterfs_ctx_new (); + if (!ctx) + goto freefs; + + /* first globals init, for gf_mem_acct_enable_set () */ + + ret = glusterfs_globals_init (ctx); + if (ret) + goto freefs; if (!THIS->ctx) THIS->ctx = ctx; - /* then ctx_defaults_init, for xlator_mem_acct_init(THIS) */ - ret = glusterfs_ctx_defaults_init (ctx); - if (ret) - return NULL; + /* then ctx_defaults_init, for xlator_mem_acct_init(THIS) */ - fs = CALLOC (1, sizeof (*fs)); - if (!fs) - return NULL; - fs->ctx = ctx; - - glfs_set_logging (fs, "/dev/null", 0); + ret = glusterfs_ctx_defaults_init (ctx); + if (ret) + goto freefs; - fs->ctx->cmd_args.volfile_id = gf_strdup (volname); + fs->ctx = ctx; - fs->volname = strdup (volname); + ret = glfs_set_logging (fs, "/dev/null", 0); + if (ret) + goto freefs; - pthread_mutex_init (&fs->mutex, NULL); - pthread_cond_init (&fs->cond, NULL); - pthread_cond_init (&fs->child_down_cond, NULL); + fs->ctx->cmd_args.volfile_id = gf_strdup (volname); + if (!(fs->ctx->cmd_args.volfile_id)) + goto freefs; - INIT_LIST_HEAD (&fs->openfds); + fs->volname = strdup (volname); + if (!fs->volname) + goto freefs; + INIT_LIST_HEAD (&fs->openfds); INIT_LIST_HEAD (&fs->upcall_list); - pthread_mutex_init (&fs->upcall_list_mutex, NULL); fs->pin_refcnt = 0; - return fs; + return fs; + +cond_child_destroy: + pthread_cond_destroy (&fs->child_down_cond); + +cond_destroy: + pthread_cond_destroy (&fs->cond); + +mutex_destroy: + pthread_mutex_destroy (&fs->mutex); + +freefs: + + /* + * When pthread_*init() fails there is no way for other cleanup + * funtions (glfs_fini/glfs_free_from_ctx) to know which of them succeded + * and which did not(unless there is a flag). Hence pthread cleanup is done + * in this funtion.Anything that fails after pthread_*_init() succeeds, should + * directly call glfs_fini() to cleanup the resources. + */ + + if (!cleanup_fini) + FREE(fs); + else + glfs_fini (fs); + fs = NULL; + + return fs; + } GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_new, 3.4.0); diff --git a/libglusterfs/src/mem-pool.h b/libglusterfs/src/mem-pool.h index 88ec9705604..2bbb45ae8a7 100644 --- a/libglusterfs/src/mem-pool.h +++ b/libglusterfs/src/mem-pool.h @@ -111,11 +111,13 @@ void* __gf_default_realloc (void *oldptr, size_t size) #define CALLOC(cnt,size) __gf_default_calloc(cnt,size) #define REALLOC(ptr,size) __gf_default_realloc(ptr,size) -#define FREE(ptr) \ - if (ptr != NULL) { \ - free ((void *)ptr); \ - ptr = (void *)0xeeeeeeee; \ - } +#define FREE(ptr) \ + do { \ + if (ptr != NULL) { \ + free ((void *)ptr); \ + ptr = (void *)0xeeeeeeee; \ + } \ + } while (0) #define GF_CALLOC(nmemb, size, type) __gf_calloc (nmemb, size, type, #type) |