[lvm-devel] [PATCH 09/14] Refactor and add code for (lv) 'lv_name' get function.

[Petr Rockai]

Dave Wysochanski <dwysocha at redhat.com> writes:

> Signed-off-by: Dave Wysochanski <dwysocha at redhat.com>
Reviewed-By: Petr Rockai <prockai at redhat.com>

> --- a/lib/metadata/lv.c
> +++ b/lib/metadata/lv.c
> @@ -20,6 +20,33 @@
>  #include "segtype.h"
>  #include "str_list.h"
>  
> +char *lv_name_dup(struct dm_pool *mem, const struct logical_volume *lv)
> +{
> +	char *repstr, *lvname;
> +	size_t len;
> +
> +	if (lv_is_visible(lv)) {
> +		goto dup;
> +	}
> +
> +	len = strlen(lv->name) + 3;
> +	if (!(repstr = dm_pool_zalloc(mem, len))) {
> +		log_error("dm_pool_alloc failed");
> +		return NULL;
> +	}
> +
> +	if (dm_snprintf(repstr, len, "[%s]", lv->name) < 0) {
> +		log_error("lvname snprintf failed");
> +		return NULL;
> +	}
> +dup:
> +	if (!(lvname = dm_pool_strdup(mem, lv->name))) {
> +		log_error("dm_pool_strdup failed");
> +		return NULL;
> +	}
> +	return lvname;
> +}
Using (proposed) dm_pool_asprintf would simplify this a lot, too.

> --- a/lib/metadata/lv.h
> +++ b/lib/metadata/lv.h
> @@ -60,5 +60,6 @@ int lv_kernel_major(const struct logical_volume *lv);
>  int lv_kernel_minor(const struct logical_volume *lv);
>  char *lv_mirror_log_dup(struct dm_pool *mem, const struct logical_volume *lv);
>  char *lv_modules_dup(struct dm_pool *mem, const struct logical_volume *lv);
> +char *lv_name_dup(struct dm_pool *mem, const struct logical_volume *lv);
>  
>  #endif

> --- a/lib/report/properties.c
> +++ b/lib/report/properties.c
> @@ -99,7 +99,7 @@ GET_PV_NUM_PROPERTY_FN(pv_mda_used_count, pv_mda_used_count(pv))
>  /* LV */
>  GET_LV_STR_PROPERTY_FN(lv_uuid, lv_uuid_dup(lv))
>  #define _lv_uuid_set _not_implemented_set
> -#define _lv_name_get _not_implemented_get
> +GET_LV_STR_PROPERTY_FN(lv_name, lv_name_dup(lv->vg->vgmem, lv))
>  #define _lv_name_set _not_implemented_set
>  GET_LV_STR_PROPERTY_FN(lv_path, lv_path_dup(lv->vg->vgmem, lv))
>  #define _lv_path_set _not_implemented_set

> --- a/lib/report/report.c
> +++ b/lib/report/report.c
> @@ -308,31 +308,13 @@ static int _lvname_disp(struct dm_report *rh, struct dm_pool *mem,
>  			const void *data, void *private __attribute__((unused)))
>  {
>  	const struct logical_volume *lv = (const struct logical_volume *) data;
> -	char *repstr, *lvname;
> -	size_t len;
> -
> -	if (lv_is_visible(lv)) {
> -		repstr = lv->name;
> -		return dm_report_field_string(rh, field, (const char **) &repstr);
> -	}
> -
> -	len = strlen(lv->name) + 3;
> -	if (!(repstr = dm_pool_zalloc(mem, len))) {
> -		log_error("dm_pool_alloc failed");
> -		return 0;
> -	}
> -
> -	if (dm_snprintf(repstr, len, "[%s]", lv->name) < 0) {
> -		log_error("lvname snprintf failed");
> -		return 0;
> -	}
> +	const char *name;
>  
> -	if (!(lvname = dm_pool_strdup(mem, lv->name))) {
> -		log_error("dm_pool_strdup failed");
> -		return 0;
> -	}
> +	name = lv_name_dup(mem, lv);
> +	if (name)
> +		return dm_report_field_string(rh, field, &name);
if ((name = ...)) vs ... ; if (name) ... again

>  
> -	dm_report_field_set_value(field, repstr, lvname);
> +	dm_report_field_set_value(field, "", NULL);
>  
>  	return 1;
>  }

OK.

Yours,
   Petr.