On Mi, 2021-07-28 at 10:54 -0500, Benjamin Marzinski wrote:
> On Thu, Jul 15, 2021 at 12:52:17PM +0200, mwilck@xxxxxxxx wrote:
> > From: Martin Wilck <mwilck@xxxxxxxx>
> >
> > 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@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
