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
