Prev: [PATCH 0/2] hwmon: Update coretemp to current Intel processors
Next: linux-next: manual merge of the davinci tree with the arm tree
From: Masami Hiramatsu on 18 May 2010 21:10
Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme(a)redhat.com>
>
> That could leave filedescriptors open and leak memory. Also stop using
> xmalloc, use malloc and handle results just like other error cases in
> the same routine that used it.

Oops, thanks for fixing.
BTW, I found some other bugs... See below.

>
> Cc: Frédéric Weisbecker <fweisbec(a)gmail.com>
> Cc: Masami Hiramatsu <mhiramat(a)redhat.com>
> Cc: Mike Galbraith <efault(a)gmx.de>
> Cc: Paul Mackerras <paulus(a)samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra(a)chello.nl>
> Cc: Tom Zanussi <tzanussi(a)gmail.com>
> LKML-Reference: <new-submission>
> Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com>
> ---
> tools/perf/util/probe-finder.c | 27 +++++++++++++++++----------
> 1 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 93583eb..0bfd46a 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -743,32 +743,34 @@ static int find_lazy_match_lines(struct list_head *head,
> const char *fname, const char *pat)
> {
> char *fbuf, *p1, *p2;
> - int fd, ret, line, nlines = 0;
> + int fd, line, nlines = -1;
> struct stat st;
>
> fd = open(fname, O_RDONLY);
> if (fd < 0) {

I think "fd = -errno;" should be here.

> pr_warning("Failed to open %s: %s\n", fname, strerror(-fd));
> - return fd;
> + return -1;

Here is also better to return fd (= -errno).

> }
>
> - ret = fstat(fd, &st);
> - if (ret < 0) {
> + nlines = fstat(fd, &st);
> + if (nlines < 0) {
> pr_warning("Failed to get the size of %s: %s\n",
> fname, strerror(errno));
> - return ret;
> + goto out_close;
> }
> - fbuf = xmalloc(st.st_size + 2);
> - ret = read(fd, fbuf, st.st_size);
> - if (ret < 0) {
> + fbuf = malloc(st.st_size + 2);
> + if (fbuf == NULL)

Here, we should set nlines = -ENOMEM;

> + goto out_close;
> + nlines = read(fd, fbuf, st.st_size);
> + if (nlines < 0) {
> pr_warning("Failed to read %s: %s\n", fname, strerror(errno));
> - return ret;
> + goto out_free_fbuf;
> }
> - close(fd);
> fbuf[st.st_size] = '\n'; /* Dummy line */
> fbuf[st.st_size + 1] = '\0';
> p1 = fbuf;
> line = 1;
> + nlines = 0;
> while ((p2 = strchr(p1, '\n')) != NULL) {
> *p2 = '\0';
> if (strlazymatch(p1, pat)) {
> @@ -778,7 +780,10 @@ static int find_lazy_match_lines(struct list_head *head,
> line++;
> p1 = p2 + 1;
> }
> +out_free_fbuf:
> free(fbuf);
> +out_close:
> + close(fd);
> return nlines;
> }
>
> @@ -955,6 +960,8 @@ int find_kprobe_trace_events(int fd, struct perf_probe_event *pev,
> if (!dbg) {
> pr_warning("No dwarf info found in the vmlinux - "
> "please rebuild with CONFIG_DEBUG_INFO=y.\n");
> + free(pf.tevs);
> + *tevs = NULL;
> return -EBADF;
> }
>

Thank you,

--
Masami Hiramatsu
e-mail: mhiramat(a)redhat.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [PATCH 0/2] hwmon: Update coretemp to current Intel processors
Next: linux-next: manual merge of the davinci tree with the arm tree