Re: [PATCH 1/4] tools/power/cpupower: Read energy_perf_bias from sysfs

[Borislav Petkov]

On Thu, Oct 29, 2020 at 04:32:43PM -0600, Shuah Khan wrote:
> > Because a patch should do one thing and one thing only. So a separate
> > patch which converts them all in one go should come ontop. But if you
> > insist for the ones I'm adding to have error handling, I can do that, of
> > course.
> > 
> 
> A separate patch is fine.

Here's how it could look like, I've converted two users for now to get
your opinion first before I convert the rest.

Thx.

---
diff --git a/tools/power/cpupower/bench/Makefile b/tools/power/cpupower/bench/Makefile
index f68b4bc55273..d4ac380f7a56 100644
--- a/tools/power/cpupower/bench/Makefile
+++ b/tools/power/cpupower/bench/Makefile
@@ -9,13 +9,14 @@ endif
 ifeq ($(strip $(STATIC)),true)
 LIBS = -L../ -L$(OUTPUT) -lm
 OBJS = $(OUTPUT)main.o $(OUTPUT)parse.o $(OUTPUT)system.o $(OUTPUT)benchmark.o \
-       $(OUTPUT)../lib/cpufreq.o $(OUTPUT)../lib/cpupower.o
+       $(OUTPUT)../lib/cpufreq.o $(OUTPUT)../lib/cpupower.o $(OUTPUT)../utils/helpers/string.o
 else
 LIBS = -L../ -L$(OUTPUT) -lm -lcpupower
-OBJS = $(OUTPUT)main.o $(OUTPUT)parse.o $(OUTPUT)system.o $(OUTPUT)benchmark.o
+OBJS = $(OUTPUT)main.o $(OUTPUT)parse.o $(OUTPUT)system.o $(OUTPUT)benchmark.o \
+       $(OUTPUT)../utils/helpers/string.o
 endif
 
-CFLAGS += -D_GNU_SOURCE -I../lib -DDEFAULT_CONFIG_FILE=\"$(confdir)/cpufreq-bench.conf\"
+CFLAGS += -D_GNU_SOURCE -I../lib -I../utils -DDEFAULT_CONFIG_FILE=\"$(confdir)/cpufreq-bench.conf\"
 
 $(OUTPUT)%.o : %.c
 	$(ECHO) "  CC      " $@
diff --git a/tools/power/cpupower/bench/parse.c b/tools/power/cpupower/bench/parse.c
index e63dc11fa3a5..54ba48ad0983 100644
--- a/tools/power/cpupower/bench/parse.c
+++ b/tools/power/cpupower/bench/parse.c
@@ -18,6 +18,8 @@
 #include "parse.h"
 #include "config.h"
 
+#include "helpers/helpers.h"
+
 /**
  * converts priority string to priority
  *
@@ -84,11 +86,12 @@ FILE *prepare_output(const char *dirname)
 		}
 
 		filename = filename_tmp;
-		snprintf(filename, len - 1, "%s/benchmark_%s_%s_%li.log",
-			dirname, sysdata.nodename, sysdata.release, time(NULL));
+		if (!cpupower_snprintf(filename, len - 1, "%s/benchmark_%s_%s_%li.log",
+		    dirname, sysdata.nodename, sysdata.release, time(NULL)))
+			return NULL;
 	} else {
-		snprintf(filename, len - 1, "%s/benchmark_%li.log",
-			dirname, time(NULL));
+		if (!cpupower_snprintf(filename, len - 1, "%s/benchmark_%li.log", dirname, time(NULL)))
+			return NULL;
 	}
 
 	dprintf("logfilename: %s\n", filename);
diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
index 37dac161f3fe..dadd919ee599 100644
--- a/tools/power/cpupower/utils/helpers/helpers.h
+++ b/tools/power/cpupower/utils/helpers/helpers.h
@@ -171,4 +171,6 @@ static inline unsigned int cpuid_ecx(unsigned int op) { return 0; };
 static inline unsigned int cpuid_edx(unsigned int op) { return 0; };
 #endif /* defined(__i386__) || defined(__x86_64__) */
 
+extern char *cpupower_snprintf(char *str, int size, const char *fmt, ...);
+
 #endif /* __CPUPOWERUTILS_HELPERS__ */
diff --git a/tools/power/cpupower/utils/helpers/string.c b/tools/power/cpupower/utils/helpers/string.c
new file mode 100644
index 000000000000..bbd5e90bdc6e
--- /dev/null
+++ b/tools/power/cpupower/utils/helpers/string.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdarg.h>
+
+#include "helpers/helpers.h"
+
+/*
+ * A helper with error handling for less code clutter.
+ */
+char *cpupower_snprintf(char *str, int size, const char *fmt, ...)
+{
+	int sz = 0;
+	va_list ap;
+
+	va_start(ap, fmt);
+	sz = vsnprintf(str, size, fmt, ap);
+	va_end(ap);
+
+	if (sz < 0) {
+		perror("vsnprintf");
+		return NULL;
+	}
+
+	if (sz >= size) {
+		fprintf(stderr, "Output truncated: [%s]\n", str);
+		return NULL;
+	}
+
+	return str;
+}

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette