From 8e4459f3dc16db07f7c5efa53b30af21f276098e Mon Sep 17 00:00:00 2001 From: Alexander Amelkin Date: Tue, 26 Feb 2019 15:17:34 +0300 Subject: [PATCH] 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 --- include/ipmitool/log.h | 12 +++++++++ lib/helper.c | 61 +++++++++++------------------------------- lib/ipmi_dcmi.c | 57 ++++++++++++++++++--------------------- 3 files changed, 53 insertions(+), 77 deletions(-) diff --git a/include/ipmitool/log.h b/include/ipmitool/log.h index 7199fdb..f8524af 100644 --- a/include/ipmitool/log.h +++ b/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*/ diff --git a/lib/helper.c b/lib/helper.c index f279a50..d7c02b6 100644 --- a/lib/helper.c +++ b/lib/helper.c @@ -669,38 +669,22 @@ print_valstr(const struct valstr * vs, const char * title, int loglevel) return; if (title) { - if (loglevel < 0) - printf("\n%s:\n\n", title); - else - lprintf(loglevel, "\n%s:\n", title); + uprintf(loglevel, "\n%s:\n", title); } - if (loglevel < 0) { - printf(" VALUE\tHEX\tSTRING\n"); - printf("==============================================\n"); - } else { - lprintf(loglevel, " VAL\tHEX\tSTRING"); - lprintf(loglevel, "=============================================="); - } + uprintf(loglevel, " VALUE\tHEX\tSTRING"); + uprintf(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); - else - printf(" %d\t0x%04x\t%s\n", vs[i].val, vs[i].val, vs[i].str); - } else { - 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 (vs[i].val < 256) + uprintf(loglevel, " %d\t0x%02x\t%s", + vs[i].val, vs[i].val, vs[i].str); + else + uprintf(loglevel, " %d\t0x%04x\t%s", + vs[i].val, vs[i].val, vs[i].str); } - if (loglevel < 0) - printf("\n"); - else - lprintf(loglevel, ""); + uprintf(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) - printf("\n%s:\n\n", title); - else - lprintf(loglevel, "\n%s:\n", title); + uprintf(loglevel, "\n%s:\n", title); } for (i = 0; vs[i].str; i++) { if (!vs[i+1].str) { /* last one */ - if (loglevel < 0) { - printf(" %4d %-32s\n", vs[i].val, vs[i].str); - } else { - lprintf(loglevel, " %4d %-32s\n", vs[i].val, vs[i].str); - } + uprintf(loglevel, " %4d %-32s\n", vs[i].val, vs[i].str); } else { - if (loglevel < 0) { - 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); - } + uprintf(loglevel, " %4d %-32s %4d %-32s\n", + vs[i].val, vs[i].str, vs[i+1].val, vs[i+1].str); i++; } } - if (loglevel < 0) - printf("\n"); - else - lprintf(loglevel, ""); + uprintf(loglevel, ""); /* Empty spacer line */ } /* ipmi_csum - calculate an ipmi checksum diff --git a/lib/ipmi_dcmi.c b/lib/ipmi_dcmi.c index a8a8b27..dd47303 100755 --- a/lib/ipmi_dcmi.c +++ b/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) - printf("\n%s\n", title); - else - lprintf(loglevel, "\n%s", title); + uprintf(loglevel, "\n%s", title); } + for (i = 0; vs[i].str; i++) { - if (loglevel < 0) { - if (vs[i].val < 256) - if (verthorz == 0) - printf(" %s %s\n", vs[i].str, vs[i].desc); - else - printf("%s", vs[i].str); - else if (verthorz == 0) - printf(" %s %s\n", vs[i].str, vs[i].desc); + if (verthorz) { + /* Horizontal printing requires that there is no \n + * at the end of individual printouts, hence we can't + * use uprintf or lprintf here. + */ + fprintf(stderr, "%s", vs[i].str); + /* Check to see if this is NOT the last element in vs.str. + * If so, print the | else terminate the line. + */ + if (vs[i+1].str) + fprintf(stderr, " | "); else - printf("%s", vs[i].str); + fprintf(stderr, "\n"); } 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) - printf(" | "); - } - if (verthorz == 0) { - if (loglevel < 0) { - printf("\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