Bug#434241: less intrusive patch

Bastian Blank waldi at debian.org
Sun Aug 12 13:55:07 UTC 2007


On Sun, Aug 12, 2007 at 12:00:28AM +0200, Guido Guenther wrote:
> @@ -108,6 +108,7 @@ enum {
>  	READ_ONLY = 0,
>  	COLS_ARG,
>  	EXEC_ARG,
> +	EXPORT_ARG,
>  	FORCE_ARG,
>  	GID_ARG,
>  	MAJOR_ARG,

Okay.

> @@ -320,8 +321,34 @@ static int _display_info_cols(struct dm_task *dmt, struct dm_info *info)
>  	return r;
>  }
>  
> +

Whitespace changes?

>  static void _display_info_long(struct dm_task *dmt, struct dm_info *info)
>  {
> +	const char* info_names[] = {
> +		"Name:              %s\n",
> +		"State:             %s%s\n",
> +		"Tables present:    %s%s%s\n",
> +		"Open count:        %d\n",
> +		"Event number:      %" PRIu32 "\n",
> +		"Major, minor:      %d, %d\n",
> +		"Number of targets: %d\n",
> +		"UUID: %s\n",
> +		"DM_NAME=%s\n",
> +		"DM_STATE=\"%s%s\"\n",
> +		"DM_TABLE_STATE=\"%s%s%s\"\n",
> +		"DM_OPENCOUNT=%d\n",
> +		"DM_LAST_EVENT_NR=%" PRIu32 "\n",
> +		"DM_MAJOR=%d\nDM_MINOR=%d\n",
> +		"DM_TARGET_COUNT=%d\n",
> +		"DM_UUID=%s\n",
> + 	};

Lacks documentation and proper index definitions or something like

const char *const info_formatstrings = {
	"Name:              %s\n",
	...
}

const char *const export_formatstrings = {
	"DM_NAME=%s\n",
}

> +	const int info_elems = sizeof(info_names)/sizeof(info_names[0])/2;
> +#define FORMAT(x) (info_names[info_elems*mode+x])

Nack. Don't use magic constants.

> +#define MODE_LONG   0
> +#define MODE_EXPORT 1

Nack.

Export can only work with one device, the given code doesn't check this.

Bastian

-- 
Extreme feminine beauty is always disturbing.
		-- Spock, "The Cloud Minders", stardate 5818.4




More information about the pkg-lvm-maintainers mailing list