the cleanup __attribute__ doesn't get run when a thread is cancelled, so it is only safe in cases where there aren't pthreads or no cancellation points happen in the code block after the variable needs cleaning up.
Signed-off-by: Benjamin Marzinski <[email protected]> --- libmultipath/configure.c | 6 +-- libmultipath/dmparser.c | 2 +- libmultipath/generic.c | 4 +- libmultipath/prioritizers/weightedpath.c | 6 ++- multipathd/cli_handlers.c | 11 ++++-- multipathd/main.c | 49 ++++++++++++++---------- 6 files changed, 48 insertions(+), 30 deletions(-) diff --git a/libmultipath/configure.c b/libmultipath/configure.c index e5249fc1..25708257 100644 --- a/libmultipath/configure.c +++ b/libmultipath/configure.c @@ -1075,7 +1075,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, int ret = CP_FAIL; int k, i, r; int is_daemon = (cmd == CMD_NONE) ? 1 : 0; - char *params __attribute__((cleanup(cleanup_charp))) = NULL; + char *params = NULL; struct multipath * mpp; struct path * pp1 = NULL; struct path * pp2; @@ -1208,6 +1208,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, remove_map(mpp, vecs->pathvec, NULL); continue; } + pthread_cleanup_push(cleanup_free_ptr, ¶ms); if (cmd == CMD_DRY_RUN) mpp->action = ACT_DRY_RUN; @@ -1216,8 +1217,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, force_reload == FORCE_RELOAD_YES ? 1 : 0); r = domap(mpp, params, is_daemon); - free(params); - params = NULL; + pthread_cleanup_pop(1); if (r == DOMAP_FAIL || r == DOMAP_RETRY) { condlog(3, "%s: domap (%u) failure " diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c index 44650aaa..066a33af 100644 --- a/libmultipath/dmparser.c +++ b/libmultipath/dmparser.c @@ -94,8 +94,8 @@ int assemble_map(struct multipath *mp, char **params) } } + condlog(4, "%s: assembled map [%s]", mp->alias, get_strbuf_str(&buff)); *params = steal_strbuf_str(&buff); - condlog(4, "%s: assembled map [%s]", mp->alias, *params); r = 0; out: pthread_cleanup_pop(1); diff --git a/libmultipath/generic.c b/libmultipath/generic.c index 3e2268e6..cdee21d9 100644 --- a/libmultipath/generic.c +++ b/libmultipath/generic.c @@ -24,11 +24,12 @@ int generic_style(const struct gen_multipath* gm, struct strbuf *buf, __attribute__((unused)) int verbosity) { struct strbuf tmp = STRBUF_INIT; - char *alias_buf __attribute__((cleanup(cleanup_charp))); + char *alias_buf = NULL; const char *wwid_buf; int ret; pthread_cleanup_push_cast(reset_strbuf, &tmp); + pthread_cleanup_push(cleanup_free_ptr, &alias_buf); gm->ops->snprint(gm, &tmp, 'n'); alias_buf = steal_strbuf_str(&tmp); gm->ops->snprint(gm, &tmp, 'w'); @@ -37,5 +38,6 @@ int generic_style(const struct gen_multipath* gm, struct strbuf *buf, ret = print_strbuf(buf, "%%n %s[%%G]:%%d %%s", strcmp(alias_buf, wwid_buf) ? "(%w) " : ""); pthread_cleanup_pop(1); + pthread_cleanup_pop(1); return ret; } diff --git a/libmultipath/prioritizers/weightedpath.c b/libmultipath/prioritizers/weightedpath.c index df700bf3..02d40a3d 100644 --- a/libmultipath/prioritizers/weightedpath.c +++ b/libmultipath/prioritizers/weightedpath.c @@ -64,7 +64,7 @@ build_wwn_path(struct path *pp, struct strbuf *buf) int prio_path_weight(struct path *pp, char *prio_args) { struct strbuf path = STRBUF_INIT; - char *arg __attribute__((cleanup(cleanup_charp))) = NULL; + char *arg = NULL; char *temp, *regex, *prio; char split_char[] = " \t"; int priority = DEFAULT_PRIORITY, path_found = 0; @@ -77,11 +77,12 @@ int prio_path_weight(struct path *pp, char *prio_args) arg = strdup(prio_args); temp = arg; + pthread_cleanup_push(cleanup_free_ptr, &arg); regex = get_next_string(&temp, split_char); /* Return default priority if the argument is not parseable */ if (!regex) { - return priority; + goto out; } pthread_cleanup_push_cast(reset_strbuf, &path); @@ -123,6 +124,7 @@ int prio_path_weight(struct path *pp, char *prio_args) } } out: + pthread_cleanup_pop(1); pthread_cleanup_pop(1); return priority; } diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index ddc807a1..2d1c5cfe 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -812,8 +812,9 @@ cli_reload(void *v, struct strbuf *reply, void *data) static int resize_map(struct multipath *mpp, unsigned long long size, struct vectors * vecs) { - char *params __attribute__((cleanup(cleanup_charp))) = NULL; + char *params = NULL; unsigned long long orig_size = mpp->size; + int r = 1; mpp->size = size; update_mpp_paths(mpp, vecs->pathvec); @@ -823,15 +824,19 @@ static int resize_map(struct multipath *mpp, unsigned long long size, mpp->size = orig_size; return 1; } + pthread_cleanup_push(cleanup_free_ptr, ¶ms); mpp->action = ACT_RESIZE; mpp->force_udev_reload = 1; if (domap(mpp, params, 1) == DOMAP_FAIL) { condlog(0, "%s: failed to resize map : %s", mpp->alias, strerror(errno)); mpp->size = orig_size; - return 1; + goto out; } - return 0; + r = 0; +out: + pthread_cleanup_pop(1); + return r; } static int diff --git a/multipathd/main.c b/multipathd/main.c index ba52d393..2517b541 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -589,9 +589,9 @@ static int update_map (struct multipath *mpp, struct vectors *vecs, int new_map) { int retries = 3; - char *params __attribute__((cleanup(cleanup_charp))) = NULL; + char *params = NULL; struct path *pp; - int i; + int i, r; retry: condlog(4, "%s: updating new map", mpp->alias); @@ -622,10 +622,11 @@ retry: retries = -1; goto fail; } - if (domap(mpp, params, 1) == DOMAP_FAIL && retries-- > 0) { + pthread_cleanup_push(cleanup_free_ptr, ¶ms); + r = domap(mpp, params, 1); + pthread_cleanup_pop(1); + if (r == DOMAP_FAIL && retries-- > 0) { condlog(0, "%s: map_udate sleep", mpp->alias); - free(params); - params = NULL; sleep(1); goto retry; } @@ -1185,7 +1186,7 @@ int ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) { struct multipath * mpp; - char *params __attribute((cleanup(cleanup_charp))) = NULL; + char *params = NULL; int retries = 3; int start_waiter = 0; int ret; @@ -1277,6 +1278,7 @@ rescan: /* * reload the map for the multipath mapped device */ + pthread_cleanup_push(cleanup_free_ptr, ¶ms); ret = domap(mpp, params, 1); while (ret == DOMAP_RETRY && retries-- > 0) { condlog(0, "%s: retry domap for addition of new " @@ -1284,6 +1286,7 @@ rescan: sleep(1); ret = domap(mpp, params, 1); } + pthread_cleanup_pop(1); if (ret == DOMAP_FAIL || ret == DOMAP_RETRY) { condlog(0, "%s: failed in domap for addition of new " "path %s", mpp->alias, pp->dev); @@ -1294,8 +1297,6 @@ rescan: condlog(0, "%s: ev_add_path sleep", mpp->alias); sleep(1); update_mpp_paths(mpp, vecs->pathvec); - free(params); - params = NULL; goto rescan; } else if (mpp->action == ACT_RELOAD) @@ -1356,8 +1357,9 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) { struct multipath * mpp; int i, retval = REMOVE_PATH_SUCCESS; - char *params __attribute__((cleanup(cleanup_charp))) = NULL; + char *params = NULL; + pthread_cleanup_push(cleanup_free_ptr, ¶ms); /* * avoid referring to the map of an orphaned path */ @@ -1376,7 +1378,8 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) if (update_mpp_paths(mpp, vecs->pathvec)) { condlog(0, "%s: failed to update paths", mpp->alias); - goto fail; + retval = REMOVE_PATH_MAP_ERROR; + goto out; } /* @@ -1398,7 +1401,8 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) if (setup_map(mpp, ¶ms, vecs)) { condlog(0, "%s: failed to setup map for" " removal of path %s", mpp->alias, pp->dev); - goto fail; + retval = REMOVE_PATH_MAP_ERROR; + goto out; } if (mpp->wait_for_udev) { @@ -1431,8 +1435,10 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) /* setup_multipath will free the path * regardless of whether it succeeds or * fails */ - if (setup_multipath(vecs, mpp)) - return REMOVE_PATH_MAP_ERROR; + if (setup_multipath(vecs, mpp)) { + retval = REMOVE_PATH_MAP_ERROR; + goto fail; + } sync_map_state(mpp); condlog(2, "%s: path removed from map %s", @@ -1445,13 +1451,14 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) free_path(pp); } out: - return retval; - + if (retval == REMOVE_PATH_MAP_ERROR) { + condlog(0, "%s: error removing path. removing map %s", pp->dev, + mpp->alias); + remove_map_and_stop_waiter(mpp, vecs); + } fail: - condlog(0, "%s: error removing path. removing map %s", pp->dev, - mpp->alias); - remove_map_and_stop_waiter(mpp, vecs); - return REMOVE_PATH_MAP_ERROR; + pthread_cleanup_pop(1); + return retval; } int @@ -2067,7 +2074,7 @@ int update_prio(struct path *pp, int refresh_all) static int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh, int is_daemon) { - char *params __attribute__((cleanup(cleanup_charp))) = NULL; + char *params = NULL; struct path *pp; int i, r; @@ -2089,9 +2096,11 @@ static int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh, condlog(0, "%s: failed to setup map", mpp->alias); return 1; } + pthread_cleanup_push(cleanup_free_ptr, ¶ms); select_action(mpp, vecs->mpvec, 1); r = domap(mpp, params, is_daemon); + pthread_cleanup_pop(1); if (r == DOMAP_FAIL || r == DOMAP_RETRY) { condlog(3, "%s: domap (%u) failure " "for reload map", mpp->alias, r); -- 2.17.2 -- dm-devel mailing list [email protected] https://listman.redhat.com/mailman/listinfo/dm-devel
