[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20131210132949.GN8098@ghostprotocols.net>
Date: Tue, 10 Dec 2013 10:29:49 -0300
From: Arnaldo Carvalho de Melo <acme@...stprotocols.net>
To: Ingo Molnar <mingo@...nel.org>
Cc: Adrian Hunter <adrian.hunter@...el.com>,
linux-kernel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
Ben Cheng <bccheng@...gle.com>,
David Ahern <dsahern@...il.com>,
Dongsheng Yang <yangds.fnst@...fujitsu.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Jiri Olsa <jolsa@...hat.com>, Mike Galbraith <efault@....de>,
Namhyung Kim <namhyung.kim@....com>,
Paul Mackerras <paulus@...ba.org>,
Peter Zijlstra <peterz@...radead.org>,
Stephane Eranian <eranian@...gle.com>,
Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 01:46:58PM +0100, Ingo Molnar escreveu:
> * Ingo Molnar <mingo@...nel.org> wrote:
> > Every time one runs into a crash like this it's a canary signal that
> > cleanliness principles need hardening.
>
> More observations about util/dso.c:
>
> - dso__binary_type_file() should probably pass in 'const struct dso'
>
> - dso__binary_type_file()'s filename string parameter should be named
> 'filename', not 'file' ...
>
> - build_id__sprintf() looks fragile: every single use of it appears
> to follow this pattern:
>
> build_id__sprintf(x, sizeof(x), ...)
>
> this could be simplified (and eliminating the possibility to typo a
> bug) by changing the function to __build_id__snprintf() and adding
> a build_id__sprintf() wrapper macro around it:
>
> build_id__sprintf(x, ...)
>
> that generates the size itself.
Right, like:
int __perf_evlist__add_default_attrs(struct perf_evlist *evlist,
struct perf_event_attr *attrs, size_t nr_attrs);
#define perf_evlist__add_default_attrs(evlist, array) \
__perf_evlist__add_default_attrs(evlist, array, ARRAY_SIZE(array))
This is all a matter of being more dilligent and judicious at employing
these and other good practices.
But don't be shy to point anything (like you did here), as time permits
we can go on doing patchkits to address things people notice.
- 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