LVM2 ./WHATS_NEW_DM libdm/libdm-report.c

LVM2 ./WHATS_NEW_DM libdm/libdm-report.c

[prajnoha]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	prajnoha@sourceware.org	2010-11-01 13:31:55

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-report.c 

Log message:
	Allocate buffer for reporting functions dynamically to support long outputs.
	Fix memory leak of field_id in _output_field function.
	
	There's been a patch added recently to use dynamic allocation for metadata
	tags buffer to remove the 4k limit (for writing metadata out). However, when
	using reporting commands like vgs and lvs, we still need to fix libdm reporting
	functions themselves to support such long outputs. So the buffer used in those
	reporting functions is dynamic now.
	
	The patch also includes a fix for field_id memory leak which was found in
	the _output_field function.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.420&r2=1.421
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-report.c.diff?cvsroot=lvm2&r1=1.37&r2=1.38

--- LVM2/WHATS_NEW_DM	2010/10/25 16:38:20	1.420
+++ LVM2/WHATS_NEW_DM	2010/11/01 13:31:55	1.421
@@ -1,5 +1,7 @@
 Version 1.02.57
 ===================================
+  Fix memory leak of field_id in _output_field function.
+  Allocate buffer for reporting functions dynamically to support long outputs.
 
 Version 1.02.56 - 25th October 2010
 ===================================
--- LVM2/libdm/libdm-report.c	2010/09/30 21:06:52	1.37
+++ LVM2/libdm/libdm-report.c	2010/11/01 13:31:55	1.38
@@ -757,7 +757,8 @@
 {
 	struct field_properties *fp;
 	const char *heading;
-	char buf[1024];
+	char *buf = NULL;
+	size_t buf_size = 0;
 
 	if (rh->flags & RH_HEADINGS_PRINTED)
 		return 1;
@@ -773,6 +774,18 @@
 		return 0;
 	}
 
+	dm_list_iterate_items(fp, &rh->field_props) {
+		if (buf_size < fp->width)
+			buf_size = fp->width;
+	}
+	/* Including trailing '\0'! */
+	buf_size++;
+
+	if (!(buf = dm_malloc(buf_size))) {
+		log_error("dm_report: Could not allocate memory for heading buffer.");
+		goto bad;
+	}
+
 	/* First heading line */
 	dm_list_iterate_items(fp, &rh->field_props) {
 		if (fp->flags & FLD_HIDDEN)
@@ -780,7 +793,7 @@
 
 		heading = rh->fields[fp->field_num].heading;
 		if (rh->flags & DM_REPORT_OUTPUT_ALIGNED) {
-			if (dm_snprintf(buf, sizeof(buf), "%-*.*s",
+			if (dm_snprintf(buf, buf_size, "%-*.*s",
 					 fp->width, fp->width, heading) < 0) {
 				log_error("dm_report: snprintf heading failed");
 				goto bad;
@@ -806,9 +819,12 @@
 	}
 	log_print("%s", (char *) dm_pool_end_object(rh->mem));
 
+	dm_free(buf);
+
 	return 1;
 
       bad:
+	dm_free(buf);
 	dm_pool_abandon_object(rh->mem);
 	return 0;
 }
@@ -892,7 +908,8 @@
 	int32_t width;
 	uint32_t align;
 	const char *repstr;
-	char buf[4096];
+	char *buf = NULL;
+	size_t buf_size = 0;
 
 	if (rh->flags & DM_REPORT_OUTPUT_FIELD_NAME_PREFIX) {
 		if (!(field_id = strdup(rh->fields[field->props->field_num].id))) {
@@ -902,11 +919,13 @@
 
 		if (!dm_pool_grow_object(rh->mem, rh->output_field_name_prefix, 0)) {
 			log_error("dm_report: Unable to extend output line");
+			free(field_id);
 			return 0;
 		}
 
 		if (!dm_pool_grow_object(rh->mem, _toupperstr(field_id), 0)) {
 			log_error("dm_report: Unable to extend output line");
+			free(field_id);
 			return 0;
 		}
 
@@ -935,25 +954,33 @@
 		if (!(align = field->props->flags & DM_REPORT_FIELD_ALIGN_MASK))
 			align = (field->props->flags & DM_REPORT_FIELD_TYPE_NUMBER) ? 
 				DM_REPORT_FIELD_ALIGN_RIGHT : DM_REPORT_FIELD_ALIGN_LEFT;
+
+		/* Including trailing '\0'! */
+		buf_size = width + 1;
+		if (!(buf = dm_malloc(buf_size))) {
+			log_error("dm_report: Could not allocate memory for output line buffer.");
+			return 0;
+		}
+
 		if (align & DM_REPORT_FIELD_ALIGN_LEFT) {
-			if (dm_snprintf(buf, sizeof(buf), "%-*.*s",
+			if (dm_snprintf(buf, buf_size, "%-*.*s",
 					 width, width, repstr) < 0) {
 				log_error("dm_report: left-aligned snprintf() failed");
-				return 0;
+				goto bad;
 			}
 			if (!dm_pool_grow_object(rh->mem, buf, width)) {
 				log_error("dm_report: Unable to extend output line");
-				return 0;
+				goto bad;
 			}
 		} else if (align & DM_REPORT_FIELD_ALIGN_RIGHT) {
-			if (dm_snprintf(buf, sizeof(buf), "%*.*s",
+			if (dm_snprintf(buf, buf_size, "%*.*s",
 					 width, width, repstr) < 0) {
 				log_error("dm_report: right-aligned snprintf() failed");
-				return 0;
+				goto bad;
 			}
 			if (!dm_pool_grow_object(rh->mem, buf, width)) {
 				log_error("dm_report: Unable to extend output line");
-				return 0;
+				goto bad;
 			}
 		}
 	}
@@ -962,10 +989,15 @@
 	    !(rh->flags & DM_REPORT_OUTPUT_FIELD_UNQUOTED))
 		if (!dm_pool_grow_object(rh->mem, "\'", 1)) {
 			log_error("dm_report: Unable to extend output line");
-			return 0;
+			goto bad;
 		}
 
+	dm_free(buf);
 	return 1;
+
+bad:
+	dm_free(buf);
+	return 0;
 }
 
 static int _output_as_rows(struct dm_report *rh)

LVM2 ./WHATS_NEW_DM libdm/libdm-report.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2012-02-10 14:00:08

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-report.c 

Log message:
	Do not crash for NULL sort_key
	
	Guard against NULL pointer for sort_key and let it behave like an empty
	string would have been passed in (i.e. no key).

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.545&r2=1.546
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-report.c.diff?cvsroot=lvm2&r1=1.46&r2=1.47

--- LVM2/WHATS_NEW_DM	2012/02/10 13:56:19	1.545
+++ LVM2/WHATS_NEW_DM	2012/02/10 14:00:07	1.546
@@ -1,5 +1,6 @@
 Version 1.02.70 - 
 ===================================
+  Do not crash for dm_report_init() sort_key == NULL and behave like "".
   Return error for failing allocation in dm_asprintf().
   Add missing test for failing allocation in dm_realloc() code.
   Add test for memory allocation failures in regex matcher code.
--- LVM2/libdm/libdm-report.c	2011/08/04 14:30:52	1.46
+++ LVM2/libdm/libdm-report.c	2012/02/10 14:00:07	1.47
@@ -541,6 +541,9 @@
 	const char *ws;		/* Word start */
 	const char *we = keys;	/* Word end */
 
+	if (!keys)
+		return 1;
+
 	while (*we) {
 		/* Allow consecutive commas */
 		while (*we && *we == ',')

LVM2 ./WHATS_NEW_DM libdm/libdm-report.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk@sourceware.org	2011-06-12 19:49:41

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-report.c 

Log message:
	Fix 'pvs -o pv_all' to include label fields.  (Also removed recursion.)

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.470&r2=1.471
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-report.c.diff?cvsroot=lvm2&r1=1.44&r2=1.45

--- LVM2/WHATS_NEW_DM	2011/06/11 00:03:06	1.470
+++ LVM2/WHATS_NEW_DM	2011/06/12 19:49:40	1.471
@@ -1,5 +1,6 @@
 Version 1.02.65 - 
 ==================================
+  Fix "all" report field prefix matching to include label fields with pv_all.
   Delay resuming new preloaded mirror devices with core logs in deptree code.
   Accept new kernel version 3 uname formats in initialisation.
 
--- LVM2/libdm/libdm-report.c	2011/04/08 14:40:20	1.44
+++ LVM2/libdm/libdm-report.c	2011/06/12 19:49:41	1.45
@@ -367,33 +367,32 @@
 {
 	size_t prefix_len;
 	const struct dm_report_object_type *t;
-	char prefixed_all[32];
+	uint32_t report_types = 0;
+	unsigned unprefixed_all_matched = 0;
 
 	if (!strncasecmp(field, "all", 3) && flen == 3) {
-		if (strlen(rh->field_prefix)) {
-			strcpy(prefixed_all, rh->field_prefix);
-			strcat(prefixed_all, "all");
-			/*
-			 * Add also prefix to receive all attributes
-			 * (e.g.LABEL/PVS use the same prefix)
-			 */
-			return rh->report_types |
-			       _all_match(rh, prefixed_all,
-					  strlen(prefixed_all));
-		} else
-			return (rh->report_types)
-				? rh->report_types : REPORT_TYPES_ALL;
+		/* If there's no report prefix, match all report types */
+		if (!(flen = strlen(rh->field_prefix)))
+			return rh->report_types ? : REPORT_TYPES_ALL;
+
+		/* otherwise include all fields beginning with the report prefix. */
+		unprefixed_all_matched = 1;
+		field = rh->field_prefix;
+		report_types = rh->report_types;
 	}
 
+	/* Combine all report types that have a matching prefix. */
 	for (t = rh->types; t->data_fn; t++) {
 		prefix_len = strlen(t->prefix);
+
 		if (!strncasecmp(t->prefix, field, prefix_len) &&
-		    !strncasecmp(field + prefix_len, "all", 3) &&
-		    flen == prefix_len + 3)
-			return t->id;
+		    ((unprefixed_all_matched && (flen == prefix_len)) ||
+		     (!strncasecmp(field + prefix_len, "all", 3) &&
+		      (flen == prefix_len + 3))))
+			report_types |= t->id;
 	}
 
-	return 0;
+	return report_types;
 }
 
 /*

LVM2 ./WHATS_NEW_DM libdm/libdm-report.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2011-01-25 21:51:31

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-report.c 

Log message:
	Initialize pool object for each row
	
	Fix assert abort of dmsetup (when compiled with pool debug)
	dmsetup splitname --nameprefixes --noheadings --rows gvg-a2
	
	Move pool begin in the inner loop - otherwise it would using
	already 'ended' pool object.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.438&r2=1.439
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-report.c.diff?cvsroot=lvm2&r1=1.40&r2=1.41

--- LVM2/WHATS_NEW_DM	2011/01/10 14:51:33	1.438
+++ LVM2/WHATS_NEW_DM	2011/01/25 21:51:30	1.439
@@ -1,5 +1,6 @@
 Version 1.02.62 - 
 ===================================
+  Initialize pool object for each row in _output_as_rows().
 
 Version 1.02.61 - 10th January 2011
 ===================================
--- LVM2/libdm/libdm-report.c	2010/11/19 13:17:27	1.40
+++ LVM2/libdm/libdm-report.c	2011/01/25 21:51:31	1.41
@@ -1006,11 +1006,6 @@
 	struct dm_report_field *field;
 	struct row *row;
 
-	if (!dm_pool_begin_object(rh->mem, 512)) {
-		log_error("dm_report: Unable to allocate output line");
-		return 0;
-	}
-
 	dm_list_iterate_items(fp, &rh->field_props) {
 		if (fp->flags & FLD_HIDDEN) {
 			dm_list_iterate_items(row, &rh->rows) {
@@ -1020,6 +1015,11 @@
 			continue;
 		}
 
+		if (!dm_pool_begin_object(rh->mem, 512)) {
+			log_error("dm_report: Unable to allocate output line");
+			return 0;
+		}
+
 		if ((rh->flags & DM_REPORT_OUTPUT_HEADINGS)) {
 			if (!dm_pool_grow_object(rh->mem, rh->fields[fp->field_num].heading, 0)) {
 				log_error("dm_report: Failed to extend row for field name");

LVM2 ./WHATS_NEW_DM libdm/libdm-report.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk@sourceware.org	2010-11-19 13:17:28

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-report.c 

Log message:
	Fix _output_field crash from field_id free with DEBUG_MEM. (Phillip Susi)

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.423&r2=1.424
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-report.c.diff?cvsroot=lvm2&r1=1.39&r2=1.40

--- LVM2/WHATS_NEW_DM	2010/11/09 02:58:06	1.423
+++ LVM2/WHATS_NEW_DM	2010/11/19 13:17:27	1.424
@@ -1,5 +1,6 @@
 Version 1.02.58 - 
 ===================================
+  Fix _output_field crash from field_id free with DEBUG_MEM. (1.02.57)
 
 Version 1.02.57 - 8th November 2010
 ===================================
--- LVM2/libdm/libdm-report.c	2010/11/01 13:50:51	1.39
+++ LVM2/libdm/libdm-report.c	2010/11/19 13:17:27	1.40
@@ -929,7 +929,7 @@
 			return 0;
 		}
 
-		free(field_id);
+		dm_free(field_id);
 
 		if (!dm_pool_grow_object(rh->mem, "=", 1)) {
 			log_error("dm_report: Unable to extend output line");

LVM2 ./WHATS_NEW_DM libdm/libdm-report.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2010-01-07 14:30:47

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-report.c 

Log message:
	Show all fields for 'dmsetup info -c -o all'.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.323&r2=1.324
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-report.c.diff?cvsroot=lvm2&r1=1.30&r2=1.31

--- LVM2/WHATS_NEW_DM	2010/01/05 21:32:59	1.323
+++ LVM2/WHATS_NEW_DM	2010/01/07 14:30:47	1.324
@@ -1,5 +1,6 @@
 Version 1.02.41 -
 ====================================
+  Update code to show all fields for 'dmsetup info -c -o all'.
   Return error from dm_tree_deactivate_children().
   Return error from dm_tree_suspend_children().
   Return error from dm_tree_preload_children() and dm_tree_activate_children().
--- LVM2/libdm/libdm-report.c	2009/02/09 09:45:49	1.30
+++ LVM2/libdm/libdm-report.c	2010/01/07 14:30:47	1.31
@@ -26,6 +26,8 @@
 struct dm_report {
 	struct dm_pool *mem;
 
+	/* To report all available types */
+#define REPORT_TYPES_ALL	UINT32_MAX
 	uint32_t report_types;
 	const char *output_field_name_prefix;
 	const char *field_prefix;
@@ -379,7 +381,8 @@
 			       _all_match(rh, prefixed_all,
 					  strlen(prefixed_all));
 		} else
-			return rh->report_types;
+			return (rh->report_types)
+				? rh->report_types : REPORT_TYPES_ALL;
 	}
 
 	for (t = rh->types; t->data_fn; t++) {

LVM2 ./WHATS_NEW_DM libdm/libdm-report.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk@sourceware.org	2009-01-10 03:01:36

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-report.c 

Log message:
	Add an "all" field which expands to all fields of the report type.
	
	For example in LVM2, "pv_all" gives all PV fields.
	"seg_all" gives all LV segment fields.
	
	"all" gives all fields of the final report type.  I think this is more
	useful than just adding the current prefix.
	
	So "lvs -o seg_all" gives all the LV segment fields, whilst
	"lvs --segments -o all" adds in LV and VG fields too.
	
	"lvs -o all -O vg_name" has report type LVS+VGS so includes all LV and all
	VG fields.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.259&r2=1.260
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-report.c.diff?cvsroot=lvm2&r1=1.26&r2=1.27

--- LVM2/WHATS_NEW_DM	2009/01/07 12:17:40	1.259
+++ LVM2/WHATS_NEW_DM	2009/01/10 03:01:35	1.260
@@ -1,5 +1,6 @@
 Version 1.02.30 -
 ====================================
+  Add "all" field to reports expanding to all fields of report type.
   Add checks for device names in dmsetup and show proper error messages.
   Replace _dm_snprintf with EMIT_PARAMS macro for creating target lines
 
--- LVM2/libdm/libdm-report.c	2008/11/03 22:14:29	1.26
+++ LVM2/libdm/libdm-report.c	2009/01/10 03:01:35	1.27
@@ -300,8 +300,6 @@
 {
 	struct field_properties *fp;
 
-	rh->report_types |= rh->fields[field_num].type;
-
 	if (!(fp = dm_pool_zalloc(rh->mem, sizeof(struct field_properties)))) {
 		log_error("dm_report: struct field_properties allocation "
 			  "failed");
@@ -352,23 +350,73 @@
 	return 0;
 }
 
-static int _field_match(struct dm_report *rh, const char *field, size_t flen)
+/*
+ * Check for a report type prefix + "all" match.
+ */
+static uint32_t _all_match(struct dm_report *rh, const char *field, size_t flen)
+{
+	size_t prefix_len;
+	const struct dm_report_object_type *t;
+
+	if (!strncasecmp(field, "all", 3) && flen == 3)
+		return rh->report_types;
+
+	for (t = rh->types; t->data_fn; t++) {
+		prefix_len = strlen(t->prefix);
+		if (!strncasecmp(t->prefix, field, prefix_len) &&
+		    !strncasecmp(field + prefix_len, "all", 3) &&
+		    flen == prefix_len + 3)
+			return t->id;
+	}
+
+	return 0;
+}
+
+/*
+ * Add all fields with a matching type.
+ */
+static int _add_all_fields(struct dm_report *rh, uint32_t type)
 {
 	uint32_t f;
 
+	for (f = 0; rh->fields[f].report_fn; f++)
+		if ((rh->fields[f].type & type) && !_add_field(rh, f, 0))
+			return 0;
+
+	return 1;
+}
+
+static int _field_match(struct dm_report *rh, const char *field, size_t flen,
+			unsigned report_type_only)
+{
+	uint32_t f, type;
+
 	if (!flen)
 		return 0;
 
 	for (f = 0; rh->fields[f].report_fn; f++)
 		if (_is_same_field(rh->fields[f].id, field, flen,
-				   rh->field_prefix))
-			return _add_field(rh, f, 0) ? 1 : 0;
+				   rh->field_prefix)) {
+			if (report_type_only) {
+				rh->report_types |= rh->fields[f].type;
+				return 1;
+			} else
+				return _add_field(rh, f, 0) ? 1 : 0;
+		}
+
+	if ((type = _all_match(rh, field, flen))) {
+		if (report_type_only) {
+			rh->report_types |= type;
+			return 1;
+		} else
+			return  _add_all_fields(rh, type);
+	}
 
 	return 0;
 }
 
 static int _add_sort_key(struct dm_report *rh, uint32_t field_num,
-			 uint32_t flags)
+			 uint32_t flags, unsigned report_type_only)
 {
 	struct field_properties *fp, *found = NULL;
 
@@ -379,8 +427,15 @@
 		}
 	}
 
-	if (!found && !(found = _add_field(rh, field_num, FLD_HIDDEN)))
-		return_0;
+	if (!found) {
+		if (report_type_only)
+			rh->report_types |= rh->fields[field_num].type;
+		else if (!(found = _add_field(rh, field_num, FLD_HIDDEN)))
+			return_0;
+	}
+
+	if (report_type_only)
+		return 1;
 
 	if (found->flags & FLD_SORT_KEY) {
 		log_error("dm_report: Ignoring duplicate sort field: %s",
@@ -395,7 +450,8 @@
 	return 1;
 }
 
-static int _key_match(struct dm_report *rh, const char *key, size_t len)
+static int _key_match(struct dm_report *rh, const char *key, size_t len,
+		      unsigned report_type_only)
 {
 	uint32_t f;
 	uint32_t flags;
@@ -422,12 +478,13 @@
 	for (f = 0; rh->fields[f].report_fn; f++)
 		if (_is_same_field(rh->fields[f].id, key, len,
 				   rh->field_prefix))
-			return _add_sort_key(rh, f, flags);
+			return _add_sort_key(rh, f, flags, report_type_only);
 
 	return 0;
 }
 
-static int _parse_options(struct dm_report *rh, const char *format)
+static int _parse_options(struct dm_report *rh, const char *format,
+			  unsigned report_type_only)
 {
 	const char *ws;		/* Word start */
 	const char *we = format;	/* Word end */
@@ -442,7 +499,7 @@
 		while (*we && *we != ',')
 			we++;
 
-		if (!_field_match(rh, ws, (size_t) (we - ws))) {
+		if (!_field_match(rh, ws, (size_t) (we - ws), report_type_only)) {
 			_display_fields(rh);
 			log_warn(" ");
 			if (strcasecmp(ws, "help") && strcmp(ws, "?"))
@@ -455,7 +512,8 @@
 	return 1;
 }
 
-static int _parse_keys(struct dm_report *rh, const char *keys)
+static int _parse_keys(struct dm_report *rh, const char *keys,
+		       unsigned report_type_only)
 {
 	const char *ws;		/* Word start */
 	const char *we = keys;	/* Word end */
@@ -467,7 +525,7 @@
 		ws = we;
 		while (*we && *we != ',')
 			we++;
-		if (!_key_match(rh, ws, (size_t) (we - ws))) {
+		if (!_key_match(rh, ws, (size_t) (we - ws), report_type_only)) {
 			log_error("dm_report: Unrecognised field: %.*s",
 				  (int) (we - ws), ws);
 			return 0;
@@ -535,13 +593,20 @@
 		return NULL;
 	}
 
-	/* Generate list of fields for output based on format string & flags */
-	if (!_parse_options(rh, output_fields)) {
+	/*
+	 * To keep the code needed to add the "all" field to a minimum, we parse
+	 * the field lists twice.  The first time we only update the report type.
+	 * FIXME Use one pass instead and expand the "all" field afterwards.
+	 */
+	if (!_parse_options(rh, output_fields, 1) ||
+	    !_parse_keys(rh, sort_keys, 1)) {
 		dm_report_free(rh);
 		return NULL;
 	}
 
-	if (!_parse_keys(rh, sort_keys)) {
+	/* Generate list of fields for output based on format string & flags */
+	if (!_parse_options(rh, output_fields, 0) ||
+	    !_parse_keys(rh, sort_keys, 0)) {
 		dm_report_free(rh);
 		return NULL;
 	}