| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 22 Jan 2010 10:26:07 -0200
From: Arnaldo Carvalho de Melo <acme@...radead.org>
To: Tomasz Fujak <t.fujak@...sung.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
jamie.iles@...ochip.com, will.deacon@....com, jpihet@...sta.com,
mingo@...e.hu, peterz@...radead.org, p.osciak@...sung.com,
m.szyprowski@...sung.com, kyungmin.park@...sung.com
Subject: Re: [PATCH v1 1/1] Extended events (platform-specific) support in
perf
Em Fri, Jan 22, 2010 at 01:08:59PM +0100, Tomasz Fujak escreveu:
> Signed-off-by: Tomasz Fujak <t.fujak@...sung.com>
> Reviewed-by: Pawel Osciak <p.osciak@...sung.com>
> Reviewed-by: Marek Szyprowski <m.szyprowski@...sung.com>
> Reviewed-by: Kyungmin Park <kuyngmin.park@...sung.com>
"Extended" seems vague, I think in this context "platform_" would be a
better namespace specifier.
> ---
> tools/perf/util/parse-events.c | 224 ++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 213 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 05d0c5c..3e32198 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -9,6 +9,9 @@
> #include "header.h"
> #include "debugfs.h"
>
> +#define EXTENDED_EVENT_PATH \
> + "/sys/devices/system/cpu/perf_events/extevents"
> +
> int nr_counters;
>
> struct perf_event_attr attrs[MAX_COUNTERS];
> @@ -60,6 +63,10 @@ static struct event_symbol event_symbols[] = {
> #define PERF_EVENT_TYPE(config) __PERF_EVENT_FIELD(config, TYPE)
> #define PERF_EVENT_ID(config) __PERF_EVENT_FIELD(config, EVENT)
>
> +static struct event_symbol *extended_event_symbols;
> +static unsigned int extended_event_count;
> +static int extended_events_initialized;
> +
> static const char *hw_event_names[] = {
> "cycles",
> "instructions",
> @@ -241,6 +248,157 @@ static const char *tracepoint_id_to_name(u64 config)
> return buf;
> }
>
> +static unsigned count_lines(const char *str, unsigned size)
> +{
> + unsigned count = 0;
> +
> + while (size--)
> + count += ('\n' == *str++);
> + return count;
> +}
> +
> +#define BYTES_PER_CHUNK 4096
> +/* returns the number of lines read;
> + on fail the return is negative and no memory is allocated
> + otherwise the *buf contains a memory chunk of which first
> + *size bytes are used for file contents
> + */
> +static int read_file(int fd, char **buf, unsigned *size)
> +{
> + unsigned bytes = BYTES_PER_CHUNK;
> + int lines = 0;
> + char *ptr = malloc(bytes);
malloc can fail... Also why is that you can't process the lines one by
one instead of reading the whole (albeit small) file at once?
> + *size = 0;
> + do {
> + int ret = read(fd, ptr + *size, bytes - *size);
> + if (ret < 0) {
> + if (EINTR == errno)
> + continue;
> + else {
> + free(ptr);
> + return -1;
> + }
> + }
> +
> + if (!ret)
> + break;
> +
> + lines += count_lines(ptr + *size, ret);
> + *size += ret;
> +
> + if (*size == bytes) {
> + char *tmp = realloc(ptr, bytes <<= 1);
> + if (!tmp) {
> + free(ptr);
> + return -1;
> + }
> + ptr = tmp;
> + }
> + } while (1);
> +
> + *buf = ptr;
> + return lines;
> +}
> +
> +static int parse_extevent_file(struct event_symbol *symbols,
> + unsigned lines, char *buf)
> +{
> + char *name, *description, *ptr, *end;
> + unsigned offset = 0, count = 0;
> + int items, eaten;
> + unsigned long long discard;
> +
> +/* each line format should be "0x%llx\t%s\t%lld-%lld\t%s\n" */
> + while (lines--) {
> + items = sscanf(buf + offset + 2, "%llx",
> + &symbols[count].config);
> + if (1 != items)
> + continue;
> +
> + /* skip 0x%llx\t */
> + ptr = strchr(buf + offset, '\t') + 1;
> +
> + name = description = NULL;
> +
> + end = strchr(ptr, '\t');
> + if (end) {
> + name = strndup(ptr, end - ptr);
> + ptr = end + 1;
> + }
> +
> + ptr = end;
> + items = sscanf(ptr, "%lld-%lld\t%n", &discard, &discard,
> + &eaten);
> + if (2 != items)
> + continue;
> +
> + ptr += eaten;
> + end = strchr(ptr, '\n');
> + if (end) {
> + description = strndup(ptr, end - ptr);
> + offset = end - buf + 1;
> + } else
> + break;
> +
> + if (name && description) {
> + symbols[count].symbol = name;
> + symbols[count].alias = "";
> + ++count;
> + /* description gets lost here */
> + free(description);
> + } else {
> + free(description);
> + free(name);
> + }
Having "free(description);" in both cases seems funny :-)
- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists