Re: [PATCH 2/4] signaltest: properly test return value from system()
From: John Kacur <jkacur@redhat.com>
Date: 2015-10-07 10:44:10
On Tue, 6 Oct 2015, Henrik Austad wrote:
quoted hunk ↗ jump to hunk
signaltest writes to several /proc/sys/kernel attributes without testing if the system() call succeeds or not. We expect the writes from echo to suceed (i.e. yield '0' in return), so we test explicitly for that. Signed-off-by: Henrik Austad <redacted> Cc: Clark Williams <redacted> Cc: John Kacur <jkacur@redhat.com> --- src/signaltest/signaltest.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c index b80969b..8074b68 100644 --- a/src/signaltest/signaltest.c +++ b/src/signaltest/signaltest.c@@ -65,6 +65,11 @@ struct thread_stat { int threadstarted; int tid; }; +#define SYSTEM_W(x) \ + if (system((x)) != 0) { \ + fprintf(stderr, "Trouble running %s\n", (x)); \ + return NULL; \ + } \ static int shutdown; static int tracelimit = 0;@@ -102,16 +107,16 @@ void *signalthread(void *param) int first = 1; if (tracelimit) { - system("echo 1 > /proc/sys/kernel/trace_all_cpus"); - system("echo 1 > /proc/sys/kernel/trace_enabled"); - system("echo 1 > /proc/sys/kernel/trace_freerunning"); - system("echo 0 > /proc/sys/kernel/trace_print_at_crash"); - system("echo 1 > /proc/sys/kernel/trace_user_triggered"); - system("echo -1 > /proc/sys/kernel/trace_user_trigger_irq"); - system("echo 0 > /proc/sys/kernel/trace_verbose"); - system("echo 0 > /proc/sys/kernel/preempt_thresh"); - system("echo 0 > /proc/sys/kernel/wakeup_timing"); - system("echo 0 > /proc/sys/kernel/preempt_max_latency"); + SYSTEM_W("echo 1 > /proc/sys/kernel/trace_all_cpus"); + SYSTEM_W("echo 1 > /proc/sys/kernel/trace_enabled"); + SYSTEM_W("echo 1 > /proc/sys/kernel/trace_freerunning"); + SYSTEM_W("echo 0 > /proc/sys/kernel/trace_print_at_crash"); + SYSTEM_W("echo 1 > /proc/sys/kernel/trace_user_triggered"); + SYSTEM_W("echo -1 > /proc/sys/kernel/trace_user_trigger_irq"); + SYSTEM_W("echo 0 > /proc/sys/kernel/trace_verbose"); + SYSTEM_W("echo 0 > /proc/sys/kernel/preempt_thresh"); + SYSTEM_W("echo 0 > /proc/sys/kernel/wakeup_timing"); + SYSTEM_W("echo 0 > /proc/sys/kernel/preempt_max_latency"); } stat->tid = gettid();-- 1.9.1
I really don't like patches like this that just exist to shut-up the compiler's overzealous warnings. Are you actually having a problem with this program that you need to debug? If you are, is the message "Trouble running echo num to some /proc" file really useful? Have you carefully read the system man page to see what kind of errors can be returned and why? I'm going to pass on this one for now. Sorry John