| Message ID | 20210715105223.30463-4-mwilck@suse.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | multipath-tools: use variable-size string buffers | expand |
On Thu, Jul 15, 2021 at 12:52:17PM +0200, mwilck@suse.com wrote: > From: Martin Wilck <mwilck@suse.com> > > Instead of using fixed PARAMS_SIZE-sized arrays for parameters, use > dynamically allocated memory. > > The library version needs to be bumped, because setup_map() argument > list has changed. > Looks good. Only minor nits below. > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > libmultipath/configure.c | 18 ++++++------ > libmultipath/configure.h | 3 +- > libmultipath/dmparser.c | 47 ++++++++++--------------------- > libmultipath/dmparser.h | 2 +- > libmultipath/libmultipath.version | 5 +++- > libmultipath/structs.h | 1 - > libmultipath/util.c | 5 ++++ > libmultipath/util.h | 1 + > multipathd/cli_handlers.c | 4 +-- > multipathd/main.c | 20 +++++++------ > 10 files changed, 50 insertions(+), 56 deletions(-) > > diff --git a/libmultipath/configure.c b/libmultipath/configure.c > index a6ae335..1227864 100644 > --- a/libmultipath/configure.c > +++ b/libmultipath/configure.c > @@ -292,8 +292,7 @@ static int wait_for_pending_paths(struct multipath *mpp, > return n_pending; > } > > -int setup_map(struct multipath *mpp, char *params, int params_size, > - struct vectors *vecs) > +int setup_map(struct multipath *mpp, char **params, struct vectors *vecs) > { > struct pathgroup * pgp; > struct config *conf; > @@ -462,7 +461,7 @@ int setup_map(struct multipath *mpp, char *params, int params_size, > * transform the mp->pg vector of vectors of paths > * into a mp->params strings to feed the device-mapper > */ > - if (assemble_map(mpp, params, params_size)) { > + if (assemble_map(mpp, params)) { > condlog(0, "%s: problem assembing map", mpp->alias); > return 1; > } > @@ -811,7 +810,7 @@ void select_action (struct multipath *mpp, const struct _vector *curmp, > remove_feature(&mpp_feat, "retain_attached_hw_handler"); > remove_feature(&cmpp_feat, "queue_if_no_path"); > remove_feature(&cmpp_feat, "retain_attached_hw_handler"); > - if (strncmp(mpp_feat, cmpp_feat, PARAMS_SIZE)) { > + if (strcmp(mpp_feat, cmpp_feat)) { > select_reload_action(mpp, "features change"); > FREE(cmpp_feat); > FREE(mpp_feat); > @@ -1128,14 +1127,14 @@ 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[PARAMS_SIZE]; > + char *params __attribute__((cleanup(cleanup_charp))) = NULL; > struct multipath * mpp; > - struct path * pp1; > + struct path * pp1 = NULL; > struct path * pp2; > vector curmp = vecs->mpvec; > vector pathvec = vecs->pathvec; > vector newmp; > - struct config *conf; > + struct config *conf = NULL; > int allow_queueing; > struct bitfield *size_mismatch_seen; > > @@ -1247,8 +1246,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, > } > verify_paths(mpp); > > - params[0] = '\0'; > - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { > + if (setup_map(mpp, ¶ms, vecs)) { > remove_map(mpp, vecs->pathvec, vecs->mpvec, KEEP_VEC); > continue; > } > @@ -1260,6 +1258,8 @@ 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; > > if (r == DOMAP_FAIL || r == DOMAP_RETRY) { > condlog(3, "%s: domap (%u) failure " > diff --git a/libmultipath/configure.h b/libmultipath/configure.h > index 70cf77a..92a5aba 100644 > --- a/libmultipath/configure.h > +++ b/libmultipath/configure.h > @@ -47,8 +47,7 @@ enum { > > struct vectors; > > -int setup_map (struct multipath * mpp, char * params, int params_size, > - struct vectors *vecs ); > +int setup_map (struct multipath * mpp, char **params, struct vectors *vecs); > void select_action (struct multipath *mpp, const struct _vector *curmp, > int force_reload); > int domap (struct multipath * mpp, char * params, int is_daemon); > diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c > index b306c46..fb09e0a 100644 > --- a/libmultipath/dmparser.c > +++ b/libmultipath/dmparser.c > @@ -15,6 +15,7 @@ > #include "util.h" > #include "debug.h" > #include "dmparser.h" > +#include "strbuf.h" > > #define WORD_SIZE 64 > > @@ -41,40 +42,21 @@ merge_words(char **dst, const char *word) > return 0; > } > > -#define APPEND(p, end, args...) \ > -({ \ > - int ret; \ > - \ > - ret = snprintf(p, end - p, ##args); \ > - if (ret < 0) { \ > - condlog(0, "%s: conversion error", mp->alias); \ > - goto err; \ > - } \ > - p += ret; \ > - if (p >= end) { \ > - condlog(0, "%s: params too small", mp->alias); \ > - goto err; \ > - } \ > -}) > - > /* > * Transforms the path group vector into a proper device map string > */ > -int > -assemble_map (struct multipath * mp, char * params, int len) > +int assemble_map(struct multipath *mp, char **params) > { > + static const char no_path_retry[] = "queue_if_no_path"; > + static const char retain_hwhandler[] = "retain_attached_hw_handler"; > int i, j; > int minio; > int nr_priority_groups, initial_pg_nr; > - char * p; > - const char *const end = params + len; > - char no_path_retry[] = "queue_if_no_path"; > - char retain_hwhandler[] = "retain_attached_hw_handler"; Why not use STRBUF_ON_STACK() here? > + struct strbuf __attribute__((cleanup(reset_strbuf))) buff = STRBUF_INIT; > struct pathgroup * pgp; > struct path * pp; > > minio = mp->minio; > - p = params; > > nr_priority_groups = VECTOR_SIZE(mp->pg); > initial_pg_nr = (nr_priority_groups ? mp->bestpg : 0); > @@ -87,14 +69,15 @@ assemble_map (struct multipath * mp, char * params, int len) > get_linux_version_code() < KERNEL_VERSION(4, 3, 0)) > add_feature(&mp->features, retain_hwhandler); > > - /* mp->features must not be NULL */ > - APPEND(p, end, "%s %s %i %i", mp->features, mp->hwhandler, > - nr_priority_groups, initial_pg_nr); > + if (print_strbuf(&buff, "%s %s %i %i", mp->features, mp->hwhandler, > + nr_priority_groups, initial_pg_nr) < 0) > + goto err; > > vector_foreach_slot (mp->pg, pgp, i) { > pgp = VECTOR_SLOT(mp->pg, i); > - APPEND(p, end, " %s %i 1", mp->selector, > - VECTOR_SIZE(pgp->paths)); > + if (print_strbuf(&buff, " %s %i 1", mp->selector, > + VECTOR_SIZE(pgp->paths)) < 0) > + goto err; > > vector_foreach_slot (pgp->paths, pp, j) { > int tmp_minio = minio; > @@ -106,19 +89,19 @@ assemble_map (struct multipath * mp, char * params, int len) > condlog(0, "dev_t not set for '%s'", pp->dev); > goto err; > } > - APPEND(p, end, " %s %d", pp->dev_t, tmp_minio); > + if (print_strbuf(&buff, " %s %d", pp->dev_t, tmp_minio) < 0) > + goto err; > } > } > > - condlog(4, "%s: assembled map [%s]", mp->alias, params); > + *params = steal_strbuf_str(&buff); > + condlog(4, "%s: assembled map [%s]", mp->alias, *params); > return 0; > > err: > return 1; > } > > -#undef APPEND > - > /* > * Caution callers: If this function encounters yet unkown path devices, it > * adds them uninitialized to the mpp. > diff --git a/libmultipath/dmparser.h b/libmultipath/dmparser.h > index 212fee5..666ae74 100644 > --- a/libmultipath/dmparser.h > +++ b/libmultipath/dmparser.h > @@ -1,3 +1,3 @@ > -int assemble_map (struct multipath *, char *, int); > +int assemble_map (struct multipath *, char **); > int disassemble_map (const struct _vector *, const char *, struct multipath *); > int disassemble_status (const char *, struct multipath *); > diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version > index 7567837..6dd52c2 100644 > --- a/libmultipath/libmultipath.version > +++ b/libmultipath/libmultipath.version > @@ -31,7 +31,7 @@ > * The new version inherits the previous ones. > */ > > -LIBMULTIPATH_6.0.0 { > +LIBMULTIPATH_7.0.0 { > global: > /* symbols referenced by multipath and multipathd */ > add_foreign; > @@ -267,6 +267,9 @@ global: > /* added in 4.5.0 */ > get_vpd_sgio; > trigger_partitions_udev_change; > + > + /* added in 7.0.0 */ > + cleanup_charp; > local: > *; > }; > diff --git a/libmultipath/structs.h b/libmultipath/structs.h > index c52bcee..399540e 100644 > --- a/libmultipath/structs.h > +++ b/libmultipath/structs.h > @@ -13,7 +13,6 @@ > #define SERIAL_SIZE 128 > #define NODE_NAME_SIZE 224 > #define PATH_STR_SIZE 16 > -#define PARAMS_SIZE 4096 > #define FILE_NAME_SIZE 256 > #define CALLOUT_MAX_SIZE 256 > #define BLK_DEV_SIZE 33 > diff --git a/libmultipath/util.c b/libmultipath/util.c > index 0e37f3f..e2fafe8 100644 > --- a/libmultipath/util.c > +++ b/libmultipath/util.c > @@ -455,3 +455,8 @@ int should_exit(void) > { > return 0; > } > + > +void cleanup_charp(char **p) > +{ > + free(*p); > +} > diff --git a/libmultipath/util.h b/libmultipath/util.h > index e9b48f9..89027f8 100644 > --- a/libmultipath/util.h > +++ b/libmultipath/util.h > @@ -123,4 +123,5 @@ static inline void clear_bit_in_bitfield(unsigned int bit, struct bitfield *bf) > ___p; \ > }) > > +void cleanup_charp(char **p); > #endif /* _UTIL_H */ > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > index d70e1db..bce40b1 100644 > --- a/multipathd/cli_handlers.c > +++ b/multipathd/cli_handlers.c > @@ -972,12 +972,12 @@ cli_reload(void *v, char **reply, int *len, void *data) > int resize_map(struct multipath *mpp, unsigned long long size, > struct vectors * vecs) > { > - char params[PARAMS_SIZE] = {0}; > + char *params __attribute__((cleanup(cleanup_charp))) = NULL; > unsigned long long orig_size = mpp->size; > > mpp->size = size; > update_mpp_paths(mpp, vecs->pathvec); > - if (setup_map(mpp, params, PARAMS_SIZE, vecs) != 0) { > + if (setup_map(mpp, ¶ms, vecs) != 0) { > condlog(0, "%s: failed to setup map for resize : %s", > mpp->alias, strerror(errno)); > mpp->size = orig_size; > diff --git a/multipathd/main.c b/multipathd/main.c > index bdd629e..f760643 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -489,7 +489,7 @@ static int > update_map (struct multipath *mpp, struct vectors *vecs, int new_map) > { > int retries = 3; > - char params[PARAMS_SIZE] = {0}; > + char *params __attribute__((cleanup(cleanup_charp))) = NULL; > > retry: > condlog(4, "%s: updating new map", mpp->alias); > @@ -502,13 +502,15 @@ retry: > verify_paths(mpp); > mpp->action = ACT_RELOAD; > > - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { > + if (setup_map(mpp, ¶ms, vecs)) { > condlog(0, "%s: failed to setup new map in update", mpp->alias); > retries = -1; > goto fail; > } > if (domap(mpp, params, 1) == DOMAP_FAIL && retries-- > 0) { > condlog(0, "%s: map_udate sleep", mpp->alias); > + free(params); > + params = NULL; > sleep(1); > goto retry; > } > @@ -1028,7 +1030,7 @@ int > ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) > { > struct multipath * mpp; > - char params[PARAMS_SIZE] = {0}; > + char *params __attribute((cleanup(cleanup_charp))) = NULL; > int retries = 3; > int start_waiter = 0; > int ret; > @@ -1104,7 +1106,9 @@ rescan: > /* > * push the map to the device-mapper > */ > - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { Is there a reason to free params here, instead of just doing it before the "goto rescan"? -Ben > + free(params); > + params = NULL; > + if (setup_map(mpp, ¶ms, vecs)) { > condlog(0, "%s: failed to setup map for addition of new " > "path %s", mpp->alias, pp->dev); > goto fail_map; > @@ -1189,7 +1193,7 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) > { > struct multipath * mpp; > int i, retval = REMOVE_PATH_SUCCESS; > - char params[PARAMS_SIZE] = {0}; > + char *params __attribute__((cleanup(cleanup_charp))) = NULL; > > /* > * avoid referring to the map of an orphaned path > @@ -1250,7 +1254,7 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) > */ > } > > - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { > + if (setup_map(mpp, ¶ms, vecs)) { > condlog(0, "%s: failed to setup map for" > " removal of path %s", mpp->alias, pp->dev); > goto fail; > @@ -1940,7 +1944,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[PARAMS_SIZE] = {0}; > + char *params __attribute__((cleanup(cleanup_charp))) = NULL; > struct path *pp; > int i, r; > > @@ -1958,7 +1962,7 @@ static int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh, > } > } > } > - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { > + if (setup_map(mpp, ¶ms, vecs)) { > condlog(0, "%s: failed to setup map", mpp->alias); > return 1; > } > -- > 2.32.0 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
On Mi, 2021-07-28 at 10:54 -0500, Benjamin Marzinski wrote: > On Thu, Jul 15, 2021 at 12:52:17PM +0200, mwilck@suse.com wrote: > > From: Martin Wilck <mwilck@suse.com> > > > > Instead of using fixed PARAMS_SIZE-sized arrays for parameters, use > > dynamically allocated memory. > > > > The library version needs to be bumped, because setup_map() > > argument > > list has changed. > > > > Looks good. Only minor nits below. > > > - > > /* > > * Transforms the path group vector into a proper device map > > string > > */ > > -int > > -assemble_map (struct multipath * mp, char * params, int len) > > +int assemble_map(struct multipath *mp, char **params) > > { > > + static const char no_path_retry[] = "queue_if_no_path"; > > + static const char retain_hwhandler[] = > > "retain_attached_hw_handler"; > > int i, j; > > int minio; > > int nr_priority_groups, initial_pg_nr; > > - char * p; > > - const char *const end = params + len; > > - char no_path_retry[] = "queue_if_no_path"; > > - char retain_hwhandler[] = "retain_attached_hw_handler"; > > Why not use STRBUF_ON_STACK() here? Good catch. I probably wrote this before I wrote the macro. > > > + struct strbuf __attribute__((cleanup(reset_strbuf))) buff = > > STRBUF_INIT; > > struct pathgroup * pgp; > > struct path * pp; > > > > @@ -1028,7 +1030,7 @@ int > > ev_add_path (struct path * pp, struct vectors * vecs, int > > need_do_map) > > { > > struct multipath * mpp; > > - char params[PARAMS_SIZE] = {0}; > > + char *params __attribute((cleanup(cleanup_charp))) = NULL; > > int retries = 3; > > int start_waiter = 0; > > int ret; > > @@ -1104,7 +1106,9 @@ rescan: > > /* > > * push the map to the device-mapper > > */ > > - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { > > Is there a reason to free params here, instead of just doing it > before > the "goto rescan"? No strong reason. I thought it was more obvious this way, and perhaps less prone to future error. I will change it. Martin -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
diff --git a/libmultipath/configure.c b/libmultipath/configure.c index a6ae335..1227864 100644 --- a/libmultipath/configure.c +++ b/libmultipath/configure.c @@ -292,8 +292,7 @@ static int wait_for_pending_paths(struct multipath *mpp, return n_pending; } -int setup_map(struct multipath *mpp, char *params, int params_size, - struct vectors *vecs) +int setup_map(struct multipath *mpp, char **params, struct vectors *vecs) { struct pathgroup * pgp; struct config *conf; @@ -462,7 +461,7 @@ int setup_map(struct multipath *mpp, char *params, int params_size, * transform the mp->pg vector of vectors of paths * into a mp->params strings to feed the device-mapper */ - if (assemble_map(mpp, params, params_size)) { + if (assemble_map(mpp, params)) { condlog(0, "%s: problem assembing map", mpp->alias); return 1; } @@ -811,7 +810,7 @@ void select_action (struct multipath *mpp, const struct _vector *curmp, remove_feature(&mpp_feat, "retain_attached_hw_handler"); remove_feature(&cmpp_feat, "queue_if_no_path"); remove_feature(&cmpp_feat, "retain_attached_hw_handler"); - if (strncmp(mpp_feat, cmpp_feat, PARAMS_SIZE)) { + if (strcmp(mpp_feat, cmpp_feat)) { select_reload_action(mpp, "features change"); FREE(cmpp_feat); FREE(mpp_feat); @@ -1128,14 +1127,14 @@ 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[PARAMS_SIZE]; + char *params __attribute__((cleanup(cleanup_charp))) = NULL; struct multipath * mpp; - struct path * pp1; + struct path * pp1 = NULL; struct path * pp2; vector curmp = vecs->mpvec; vector pathvec = vecs->pathvec; vector newmp; - struct config *conf; + struct config *conf = NULL; int allow_queueing; struct bitfield *size_mismatch_seen; @@ -1247,8 +1246,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid, } verify_paths(mpp); - params[0] = '\0'; - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { + if (setup_map(mpp, ¶ms, vecs)) { remove_map(mpp, vecs->pathvec, vecs->mpvec, KEEP_VEC); continue; } @@ -1260,6 +1258,8 @@ 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; if (r == DOMAP_FAIL || r == DOMAP_RETRY) { condlog(3, "%s: domap (%u) failure " diff --git a/libmultipath/configure.h b/libmultipath/configure.h index 70cf77a..92a5aba 100644 --- a/libmultipath/configure.h +++ b/libmultipath/configure.h @@ -47,8 +47,7 @@ enum { struct vectors; -int setup_map (struct multipath * mpp, char * params, int params_size, - struct vectors *vecs ); +int setup_map (struct multipath * mpp, char **params, struct vectors *vecs); void select_action (struct multipath *mpp, const struct _vector *curmp, int force_reload); int domap (struct multipath * mpp, char * params, int is_daemon); diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c index b306c46..fb09e0a 100644 --- a/libmultipath/dmparser.c +++ b/libmultipath/dmparser.c @@ -15,6 +15,7 @@ #include "util.h" #include "debug.h" #include "dmparser.h" +#include "strbuf.h" #define WORD_SIZE 64 @@ -41,40 +42,21 @@ merge_words(char **dst, const char *word) return 0; } -#define APPEND(p, end, args...) \ -({ \ - int ret; \ - \ - ret = snprintf(p, end - p, ##args); \ - if (ret < 0) { \ - condlog(0, "%s: conversion error", mp->alias); \ - goto err; \ - } \ - p += ret; \ - if (p >= end) { \ - condlog(0, "%s: params too small", mp->alias); \ - goto err; \ - } \ -}) - /* * Transforms the path group vector into a proper device map string */ -int -assemble_map (struct multipath * mp, char * params, int len) +int assemble_map(struct multipath *mp, char **params) { + static const char no_path_retry[] = "queue_if_no_path"; + static const char retain_hwhandler[] = "retain_attached_hw_handler"; int i, j; int minio; int nr_priority_groups, initial_pg_nr; - char * p; - const char *const end = params + len; - char no_path_retry[] = "queue_if_no_path"; - char retain_hwhandler[] = "retain_attached_hw_handler"; + struct strbuf __attribute__((cleanup(reset_strbuf))) buff = STRBUF_INIT; struct pathgroup * pgp; struct path * pp; minio = mp->minio; - p = params; nr_priority_groups = VECTOR_SIZE(mp->pg); initial_pg_nr = (nr_priority_groups ? mp->bestpg : 0); @@ -87,14 +69,15 @@ assemble_map (struct multipath * mp, char * params, int len) get_linux_version_code() < KERNEL_VERSION(4, 3, 0)) add_feature(&mp->features, retain_hwhandler); - /* mp->features must not be NULL */ - APPEND(p, end, "%s %s %i %i", mp->features, mp->hwhandler, - nr_priority_groups, initial_pg_nr); + if (print_strbuf(&buff, "%s %s %i %i", mp->features, mp->hwhandler, + nr_priority_groups, initial_pg_nr) < 0) + goto err; vector_foreach_slot (mp->pg, pgp, i) { pgp = VECTOR_SLOT(mp->pg, i); - APPEND(p, end, " %s %i 1", mp->selector, - VECTOR_SIZE(pgp->paths)); + if (print_strbuf(&buff, " %s %i 1", mp->selector, + VECTOR_SIZE(pgp->paths)) < 0) + goto err; vector_foreach_slot (pgp->paths, pp, j) { int tmp_minio = minio; @@ -106,19 +89,19 @@ assemble_map (struct multipath * mp, char * params, int len) condlog(0, "dev_t not set for '%s'", pp->dev); goto err; } - APPEND(p, end, " %s %d", pp->dev_t, tmp_minio); + if (print_strbuf(&buff, " %s %d", pp->dev_t, tmp_minio) < 0) + goto err; } } - condlog(4, "%s: assembled map [%s]", mp->alias, params); + *params = steal_strbuf_str(&buff); + condlog(4, "%s: assembled map [%s]", mp->alias, *params); return 0; err: return 1; } -#undef APPEND - /* * Caution callers: If this function encounters yet unkown path devices, it * adds them uninitialized to the mpp. diff --git a/libmultipath/dmparser.h b/libmultipath/dmparser.h index 212fee5..666ae74 100644 --- a/libmultipath/dmparser.h +++ b/libmultipath/dmparser.h @@ -1,3 +1,3 @@ -int assemble_map (struct multipath *, char *, int); +int assemble_map (struct multipath *, char **); int disassemble_map (const struct _vector *, const char *, struct multipath *); int disassemble_status (const char *, struct multipath *); diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version index 7567837..6dd52c2 100644 --- a/libmultipath/libmultipath.version +++ b/libmultipath/libmultipath.version @@ -31,7 +31,7 @@ * The new version inherits the previous ones. */ -LIBMULTIPATH_6.0.0 { +LIBMULTIPATH_7.0.0 { global: /* symbols referenced by multipath and multipathd */ add_foreign; @@ -267,6 +267,9 @@ global: /* added in 4.5.0 */ get_vpd_sgio; trigger_partitions_udev_change; + + /* added in 7.0.0 */ + cleanup_charp; local: *; }; diff --git a/libmultipath/structs.h b/libmultipath/structs.h index c52bcee..399540e 100644 --- a/libmultipath/structs.h +++ b/libmultipath/structs.h @@ -13,7 +13,6 @@ #define SERIAL_SIZE 128 #define NODE_NAME_SIZE 224 #define PATH_STR_SIZE 16 -#define PARAMS_SIZE 4096 #define FILE_NAME_SIZE 256 #define CALLOUT_MAX_SIZE 256 #define BLK_DEV_SIZE 33 diff --git a/libmultipath/util.c b/libmultipath/util.c index 0e37f3f..e2fafe8 100644 --- a/libmultipath/util.c +++ b/libmultipath/util.c @@ -455,3 +455,8 @@ int should_exit(void) { return 0; } + +void cleanup_charp(char **p) +{ + free(*p); +} diff --git a/libmultipath/util.h b/libmultipath/util.h index e9b48f9..89027f8 100644 --- a/libmultipath/util.h +++ b/libmultipath/util.h @@ -123,4 +123,5 @@ static inline void clear_bit_in_bitfield(unsigned int bit, struct bitfield *bf) ___p; \ }) +void cleanup_charp(char **p); #endif /* _UTIL_H */ diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index d70e1db..bce40b1 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -972,12 +972,12 @@ cli_reload(void *v, char **reply, int *len, void *data) int resize_map(struct multipath *mpp, unsigned long long size, struct vectors * vecs) { - char params[PARAMS_SIZE] = {0}; + char *params __attribute__((cleanup(cleanup_charp))) = NULL; unsigned long long orig_size = mpp->size; mpp->size = size; update_mpp_paths(mpp, vecs->pathvec); - if (setup_map(mpp, params, PARAMS_SIZE, vecs) != 0) { + if (setup_map(mpp, ¶ms, vecs) != 0) { condlog(0, "%s: failed to setup map for resize : %s", mpp->alias, strerror(errno)); mpp->size = orig_size; diff --git a/multipathd/main.c b/multipathd/main.c index bdd629e..f760643 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -489,7 +489,7 @@ static int update_map (struct multipath *mpp, struct vectors *vecs, int new_map) { int retries = 3; - char params[PARAMS_SIZE] = {0}; + char *params __attribute__((cleanup(cleanup_charp))) = NULL; retry: condlog(4, "%s: updating new map", mpp->alias); @@ -502,13 +502,15 @@ retry: verify_paths(mpp); mpp->action = ACT_RELOAD; - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { + if (setup_map(mpp, ¶ms, vecs)) { condlog(0, "%s: failed to setup new map in update", mpp->alias); retries = -1; goto fail; } if (domap(mpp, params, 1) == DOMAP_FAIL && retries-- > 0) { condlog(0, "%s: map_udate sleep", mpp->alias); + free(params); + params = NULL; sleep(1); goto retry; } @@ -1028,7 +1030,7 @@ int ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) { struct multipath * mpp; - char params[PARAMS_SIZE] = {0}; + char *params __attribute((cleanup(cleanup_charp))) = NULL; int retries = 3; int start_waiter = 0; int ret; @@ -1104,7 +1106,9 @@ rescan: /* * push the map to the device-mapper */ - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { + free(params); + params = NULL; + if (setup_map(mpp, ¶ms, vecs)) { condlog(0, "%s: failed to setup map for addition of new " "path %s", mpp->alias, pp->dev); goto fail_map; @@ -1189,7 +1193,7 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) { struct multipath * mpp; int i, retval = REMOVE_PATH_SUCCESS; - char params[PARAMS_SIZE] = {0}; + char *params __attribute__((cleanup(cleanup_charp))) = NULL; /* * avoid referring to the map of an orphaned path @@ -1250,7 +1254,7 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) */ } - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { + if (setup_map(mpp, ¶ms, vecs)) { condlog(0, "%s: failed to setup map for" " removal of path %s", mpp->alias, pp->dev); goto fail; @@ -1940,7 +1944,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[PARAMS_SIZE] = {0}; + char *params __attribute__((cleanup(cleanup_charp))) = NULL; struct path *pp; int i, r; @@ -1958,7 +1962,7 @@ static int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh, } } } - if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { + if (setup_map(mpp, ¶ms, vecs)) { condlog(0, "%s: failed to setup map", mpp->alias); return 1; }