Refactor to reduce complexity of dict print funcs

Dictionary print functions (print_valstr, print_valstr_2col,
dcmi_print_strs) are used in various modules and are excessively
complex. One of the reasons is support for loglevel -1, which means
printing to STDOUT. Another reason is support in dcmi_print_strs()
for so called 'horizontal printing' when only strings without
descriptions are printed in one line with separators.

Horizontal printing was in fact broken as dcmi_print_strs() was
always called with loglevel set to LOG_ERR, which resulted in using
lprintf() that adds a '\n' at the end of each printout.

This commit:
* Fixes horizontal printout for dcmi_print_strs();
* Reduces complexity of all three functions by introducing
  a macro helper called uprintf() that automatically chooses
  either printf() or lprintf() based on the selected loglevel;

Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>

include/ipmitool/log.h

@@ -59,5 +59,17 @@ int log_level_get(void);
 void lprintf(int level, const char * format, ...);
 void lperror(int level, const char * format, ...);
 
+/**
+ * @brief Print to log or to STDOUT depending on the \p ll argument.
+ * @param[in] ll  Log level. Negative values meant "print to stdout".
+ * @param[in] fmt The printf format string. No '\n' is needed at the end.
+ */
+#define uprintf(ll, fmt, ...) do { \
+	if (ll < 0) \
+		printf(fmt "\n", ##__VA_ARGS__); \
+	else \
+		lprintf(ll, fmt, ##__VA_ARGS__); \
+} while(0)
+
 #endif /*IPMITOOL_LOG_H*/
 

lib/helper.c

@@ -669,38 +669,22 @@ print_valstr(const struct valstr * vs, const char * title, int loglevel)
 		return;
 
 	if (title) {
-		if (loglevel < 0)
+		uprintf(loglevel, "\n%s:\n", title);
-			printf("\n%s:\n\n", title);
-		else
-			lprintf(loglevel, "\n%s:\n", title);
 	}
 
-	if (loglevel < 0) {
+	uprintf(loglevel, "  VALUE\tHEX\tSTRING");
-		printf("  VALUE\tHEX\tSTRING\n");
+	uprintf(loglevel, "==============================================");
-		printf("==============================================\n");
-	} else {
-		lprintf(loglevel, "  VAL\tHEX\tSTRING");
-		lprintf(loglevel, "==============================================");
-	}
 
 	for (i = 0; vs[i].str; i++) {
-		if (loglevel < 0) {
 		if (vs[i].val < 256)
-				printf("  %d\t0x%02x\t%s\n", vs[i].val, vs[i].val, vs[i].str);
+			uprintf(loglevel, "  %d\t0x%02x\t%s",
+			        vs[i].val, vs[i].val, vs[i].str);
 		else
-				printf("  %d\t0x%04x\t%s\n", vs[i].val, vs[i].val, vs[i].str);
+			uprintf(loglevel, "  %d\t0x%04x\t%s",
-		} else {
+			        vs[i].val, vs[i].val, vs[i].str);
-			if (vs[i].val < 256)
-				lprintf(loglevel, "  %d\t0x%02x\t%s", vs[i].val, vs[i].val, vs[i].str);
-			else
-				lprintf(loglevel, "  %d\t0x%04x\t%s", vs[i].val, vs[i].val, vs[i].str);
-		}
 	}
 
-	if (loglevel < 0)
+	uprintf(loglevel, "");
-		printf("\n");
-	else
-		lprintf(loglevel, "");
 }
 
 /* print_valstr_2col  -  print value string list in two columns to log or stdout
@@ -718,37 +702,22 @@ print_valstr_2col(const struct valstr * vs, const char * title, int loglevel)
 		return;
 
 	if (title) {
-		if (loglevel < 0)
+		uprintf(loglevel, "\n%s:\n", title);
-			printf("\n%s:\n\n", title);
-		else
-			lprintf(loglevel, "\n%s:\n", title);
 	}
 
 	for (i = 0; vs[i].str; i++) {
 		if (!vs[i+1].str) {
 			/* last one */
-			if (loglevel < 0) {
+			uprintf(loglevel, "  %4d  %-32s\n", vs[i].val, vs[i].str);
-				printf("  %4d  %-32s\n", vs[i].val, vs[i].str);
-			} else {
-				lprintf(loglevel, "  %4d  %-32s\n", vs[i].val, vs[i].str);
-			}
 		}
 		else {
-			if (loglevel < 0) {
+			uprintf(loglevel, "  %4d  %-32s    %4d  %-32s\n",
-				printf("  %4d  %-32s    %4d  %-32s\n",
 			        vs[i].val, vs[i].str, vs[i+1].val, vs[i+1].str);
-			} else {
-				lprintf(loglevel, "  %4d  %-32s    %4d  %-32s\n",
-					vs[i].val, vs[i].str, vs[i+1].val, vs[i+1].str);
-			}
 			i++;
 		}
 	}
 
-	if (loglevel < 0)
+	uprintf(loglevel, ""); /* Empty spacer line */
-		printf("\n");
-	else
-		lprintf(loglevel, "");
 }
 
 /* ipmi_csum  -  calculate an ipmi checksum

lib/ipmi_dcmi.c

@@ -332,11 +332,14 @@ const struct dcmi_cmd dcmi_sampling_vals[] = {
  * purpose but with out the extra formatting.  This function simply prints
  * the dcmi_cmd struct provided.  verthorz specifies to print vertically or
  * horizontally.  If the string is printed horizontally then a | will be
- * printed between each instance of vs[i].str until it is NULL
+ * printed between each instance of vs[i].str until it is NULL.
+ * Note that vs[i].desc is not printed in horizontal mode.
  *
  * @vs:         value string list to print
  * @title:      name of this value string list
- * @loglevel:   what log level to print, -1 for stdout
+ * @loglevel:   what log level to print, -1 for stdout.
+ *              if loglevel is -1 _and_ verthorz is non-zero,
+ *              then output is to stderr.
  * @verthorz:   printed vertically or horizontally, 0 or 1
  */
 void
@@ -346,46 +349,38 @@ dcmi_print_strs(const struct dcmi_cmd * vs,
                 int verthorz)
 {
 	int i;
+	char *buf;
 
 	if (!vs)
 		return;
 
 	if (title) {
-		if (loglevel < 0)
+		uprintf(loglevel, "\n%s", title);
-			printf("\n%s\n", title);
-		else
-			lprintf(loglevel, "\n%s", title);
 	}
+
 	for (i = 0; vs[i].str; i++) {
-		if (loglevel < 0) {
+		if (verthorz) {
-			if (vs[i].val < 256)
+			/* Horizontal printing requires that there is no \n
-				if (verthorz == 0)
+			 * at the end of individual printouts, hence we can't
-					printf("    %s    %s\n", vs[i].str, vs[i].desc);
+			 * use uprintf or lprintf here.
-				else
-					printf("%s", vs[i].str);
-			else if (verthorz == 0)
-				printf("    %s    %s\n", vs[i].str, vs[i].desc);
-			else
-				printf("%s", vs[i].str);
-		} else {
-			if (vs[i].val < 256)
-				lprintf(loglevel, "    %s    %s", vs[i].str, vs[i].desc);
-			else
-				lprintf(loglevel, "    %s    %s", vs[i].str, vs[i].desc);
-		}
-		/* Check to see if this is NOT the last element in vs.str if true
-		 * print the | else don't print anything.
 			 */
-		if (verthorz == 1 && vs[i+1].str)
+			fprintf(stderr, "%s", vs[i].str);
-			printf(" | ");
+			/* Check to see if this is NOT the last element in vs.str.
-	}
+			 * If so, print the | else terminate the line.
-	if (verthorz == 0) {
+			 */
-		if (loglevel < 0) {
+			if (vs[i+1].str)
-			printf("\n");
+				fprintf(stderr, " | ");
+			else
+				fprintf(stderr, "\n");
 		} else {
-			lprintf(loglevel, "");
+			uprintf(loglevel, "    %10s    %s", vs[i].str, vs[i].desc);
 		}
 	}
+
+	if (verthorz)
+		fprintf(stderr, "\n");
+	else
+		uprintf(loglevel, "");
 }
 
 /* This was taken from str2val() from helper.c.  It serves the same