public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	David Ahern <dsahern@gmail.com>, Davidlohr Bueso <dbueso@suse.de>,
	Jiri Olsa <jolsa@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Wang Nan <wangnan0@huawei.com>
Subject: [PATCH 04/14] Revert "perf bench futex: Sanitize numeric parameters"
Date: Wed, 15 Feb 2017 16:04:14 -0300	[thread overview]
Message-ID: <20170215190424.18687-5-acme@kernel.org> (raw)
In-Reply-To: <20170215190424.18687-1-acme@kernel.org>

From: Arnaldo Carvalho de Melo <acme@redhat.com>

This reverts commit 60758d6668b3e2fa8e5fd143d24d0425203d007e.

Now that libsubcmd makes sure that OPT_UINTEGER options will not
return negative values, we can revert this patch while addressing
the problem it solved:

  # perf bench futex hash -t  -4
  # Running 'futex/hash' benchmark:
   Error: switch `t' expects an unsigned numerical value
   Usage: perf bench futex hash <options>

      -t, --threads <n>     Specify amount of threads
  # perf bench futex hash -t-4
  # Running 'futex/hash' benchmark:
   Error: switch `t' expects an unsigned numerical value
   Usage: perf bench futex hash <options>

      -t, --threads <n>     Specify amount of threads
  #

IMO it is more reasonable to flat out refuse to process a negative
number than to silently turn it into an absolute value.

This also helps in silencing clang's complaint about asking for an
absolute value of an unsigned integer:

  bench/futex-hash.c:133:10: error: taking the absolute value of unsigned type 'unsigned int' has no effect [-Werror,-Wabsolute-value]
          nsecs = futexbench_sanitize_numeric(nsecs);
                ^
  bench/futex.h:104:42: note: expanded from macro 'futexbench_sanitize_numeric'
  #define futexbench_sanitize_numeric(__n) abs((__n))
                                           ^
  bench/futex-hash.c:133:10: note: remove the call to 'abs' since unsigned values cannot be negative

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-2kl68v22or31vw643m2exz8x@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/bench/futex-hash.c          | 4 ----
 tools/perf/bench/futex-lock-pi.c       | 3 ---
 tools/perf/bench/futex-requeue.c       | 2 --
 tools/perf/bench/futex-wake-parallel.c | 4 ----
 tools/perf/bench/futex-wake.c          | 3 ---
 tools/perf/bench/futex.h               | 4 ----
 6 files changed, 20 deletions(-)

diff --git a/tools/perf/bench/futex-hash.c b/tools/perf/bench/futex-hash.c
index bfbb6b5f609c..da04b8c5568a 100644
--- a/tools/perf/bench/futex-hash.c
+++ b/tools/perf/bench/futex-hash.c
@@ -130,8 +130,6 @@ int bench_futex_hash(int argc, const char **argv,
 	}
 
 	ncpus = sysconf(_SC_NPROCESSORS_ONLN);
-	nsecs = futexbench_sanitize_numeric(nsecs);
-	nfutexes = futexbench_sanitize_numeric(nfutexes);
 
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
@@ -139,8 +137,6 @@ int bench_futex_hash(int argc, const char **argv,
 
 	if (!nthreads) /* default to the number of CPUs */
 		nthreads = ncpus;
-	else
-		nthreads = futexbench_sanitize_numeric(nthreads);
 
 	worker = calloc(nthreads, sizeof(*worker));
 	if (!worker)
diff --git a/tools/perf/bench/futex-lock-pi.c b/tools/perf/bench/futex-lock-pi.c
index 6d9d6c40a916..91877777ec6e 100644
--- a/tools/perf/bench/futex-lock-pi.c
+++ b/tools/perf/bench/futex-lock-pi.c
@@ -152,7 +152,6 @@ int bench_futex_lock_pi(int argc, const char **argv,
 		goto err;
 
 	ncpus = sysconf(_SC_NPROCESSORS_ONLN);
-	nsecs = futexbench_sanitize_numeric(nsecs);
 
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
@@ -160,8 +159,6 @@ int bench_futex_lock_pi(int argc, const char **argv,
 
 	if (!nthreads)
 		nthreads = ncpus;
-	else
-		nthreads = futexbench_sanitize_numeric(nthreads);
 
 	worker = calloc(nthreads, sizeof(*worker));
 	if (!worker)
diff --git a/tools/perf/bench/futex-requeue.c b/tools/perf/bench/futex-requeue.c
index fd4ee95b689a..2b9705a8734c 100644
--- a/tools/perf/bench/futex-requeue.c
+++ b/tools/perf/bench/futex-requeue.c
@@ -128,8 +128,6 @@ int bench_futex_requeue(int argc, const char **argv,
 
 	if (!nthreads)
 		nthreads = ncpus;
-	else
-		nthreads = futexbench_sanitize_numeric(nthreads);
 
 	worker = calloc(nthreads, sizeof(*worker));
 	if (!worker)
diff --git a/tools/perf/bench/futex-wake-parallel.c b/tools/perf/bench/futex-wake-parallel.c
index beaa6c142477..2c8fa67ad537 100644
--- a/tools/perf/bench/futex-wake-parallel.c
+++ b/tools/perf/bench/futex-wake-parallel.c
@@ -217,12 +217,8 @@ int bench_futex_wake_parallel(int argc, const char **argv,
 	sigaction(SIGINT, &act, NULL);
 
 	ncpus = sysconf(_SC_NPROCESSORS_ONLN);
-	nwaking_threads = futexbench_sanitize_numeric(nwaking_threads);
-
 	if (!nblocked_threads)
 		nblocked_threads = ncpus;
-	else
-		nblocked_threads = futexbench_sanitize_numeric(nblocked_threads);
 
 	/* some sanity checks */
 	if (nwaking_threads > nblocked_threads || !nwaking_threads)
diff --git a/tools/perf/bench/futex-wake.c b/tools/perf/bench/futex-wake.c
index 46efcb98b5a4..e246b1b8388a 100644
--- a/tools/perf/bench/futex-wake.c
+++ b/tools/perf/bench/futex-wake.c
@@ -129,7 +129,6 @@ int bench_futex_wake(int argc, const char **argv,
 	}
 
 	ncpus = sysconf(_SC_NPROCESSORS_ONLN);
-	nwakes = futexbench_sanitize_numeric(nwakes);
 
 	sigfillset(&act.sa_mask);
 	act.sa_sigaction = toggle_done;
@@ -137,8 +136,6 @@ int bench_futex_wake(int argc, const char **argv,
 
 	if (!nthreads)
 		nthreads = ncpus;
-	else
-		nthreads = futexbench_sanitize_numeric(nthreads);
 
 	worker = calloc(nthreads, sizeof(*worker));
 	if (!worker)
diff --git a/tools/perf/bench/futex.h b/tools/perf/bench/futex.h
index ba7c735c0c62..b2e06d1190d0 100644
--- a/tools/perf/bench/futex.h
+++ b/tools/perf/bench/futex.h
@@ -7,7 +7,6 @@
 #ifndef _FUTEX_H
 #define _FUTEX_H
 
-#include <stdlib.h>
 #include <unistd.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
@@ -100,7 +99,4 @@ static inline int pthread_attr_setaffinity_np(pthread_attr_t *attr,
 }
 #endif
 
-/* User input sanitation */
-#define futexbench_sanitize_numeric(__n) abs((__n))
-
 #endif /* _FUTEX_H */
-- 
2.9.3

  parent reply	other threads:[~2017-02-15 19:05 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-15 19:04 [GIT PULL 00/14] perf/core clang fixes Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 01/14] tools: Suppress request for warning options not existent in clang Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 02/14] tools: Set the maximum optimization level according to the compiler being used Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 03/14] tools lib subcmd: Make it an error to pass a signed value to OPTION_UINTEGER Arnaldo Carvalho de Melo
2017-02-15 19:04 ` Arnaldo Carvalho de Melo [this message]
2017-02-15 19:04 ` [PATCH 05/14] perf bench numa: Make sure dprintf() is not defined Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 06/14] perf tests: Synthesize struct instead of using field after variable sized type Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 07/14] perf record: Do not put a variable sized type not at the end of a struct Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 08/14] perf tools: " Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 09/14] perf probe: Avoid accessing uninitialized 'map' variable Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 10/14] perf evsel: Do not put a variable sized type not at the end of a struct Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 11/14] perf intel pt decoder: clang has no -Wno-override-init Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 12/14] perf tools: Be consistent on the type of map->symbols[] interator Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 13/14] perf pmu: Fix check for unset alias->unit array Arnaldo Carvalho de Melo
2017-02-15 19:04 ` [PATCH 14/14] perf tools: Add missing parse_events_error() prototype Arnaldo Carvalho de Melo
2017-02-16 19:54 ` [GIT PULL 00/14] perf/core clang fixes Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170215190424.18687-5-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=dbueso@suse.de \
    --cc=dsahern@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox