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: Sat, 11 Jun 2016 09:28:09 +0900 From: Masami Hiramatsu <mhiramat@...nel.org> To: Arnaldo Carvalho de Melo <acme@...nel.org> Cc: linux-kernel@...r.kernel.org, Namhyung Kim <namhyung@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, Hemant Kumar <hemant@...ux.vnet.ibm.com>, Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>, Brendan Gregg <brendan.d.gregg@...il.com> Subject: Re: [PATCH perf/core v10 09/23] perf probe: Show all cached probes On Thu, 9 Jun 2016 11:22:55 -0300 Arnaldo Carvalho de Melo <acme@...nel.org> wrote: > Em Wed, Jun 08, 2016 at 06:30:30PM +0900, Masami Hiramatsu escreveu: > > +char *build_id_cache__origname(const char *sbuild_id) > > +{ > > + char *linkname; > > + char buf[PATH_MAX]; > > + char *ret = NULL, *p; > > + size_t offs = 5; /* == strlen("../..") */ > > + > > + linkname = build_id_cache__linkname(sbuild_id, NULL, 0); > > + if (!linkname) > > + return NULL; > > + > > + if (readlink(linkname, buf, PATH_MAX) < 0) > > + goto out; > > + /* The link should be "../..<origpath>/<sbuild_id>" */ > > + p = strrchr(buf, '/'); /* Cut off the "/<sbuild_id>" */ > > + if (p && (p > buf + offs)) { > > + *p = '\0'; > > + if (buf[offs + 1] == '[') > > + offs++; /* > > + * This is a DSO name, like [kernel.kallsyms]. > > + * Skip the first '/', since this is not the > > + * cache of a regular file. > > + */ > > + ret = strdup(buf + offs); /* Skip "../..[/]" */ > > strdup can fail. Ah, right. > > > + } > > +out: > > + free(linkname); > > + return ret; > > +} > > + > > static const char *build_id_cache__basename(bool is_kallsyms, bool is_vdso) > > { > > return is_kallsyms ? "kallsyms" : (is_vdso ? "vdso" : "elf"); > > @@ -390,6 +419,59 @@ void disable_buildid_cache(void) > > no_buildid_cache = true; > > } > > > > +int build_id_cache__list_all(struct strlist **result) > > +{ > > + struct strlist *toplist, *list, *bidlist; > > + struct str_node *nd, *nd2; > > + char *topdir, *linkdir; > > + char sbuild_id[SBUILD_ID_SIZE]; > > + int ret = 0; > > + > > + /* Open the top-level directory */ > > + if (asprintf(&topdir, "%s/.build-id/", buildid_dir) < 0) > > + return -errno; > > Wouldn't be better to just return -1 and leave the caller to look at > errno? I know that there is code that does that, but better stick to the > errno convention? Hmm, IMHO, that may depends on case. This case is possible, but in below case, pr_debug() also calls libc function and it can override the errno. This can also happen when we run some cleanup code before returning. So, I think it will be good for enough small functions, but not good for complex bigger functions. >From the viewpoint of debugging, maybe it is also possible to return -N, where N is based on the place where the error happens :) > > + toplist = lsdir(topdir, lsdir_no_dot_filter); > > + if (!toplist) { > > + pr_debug("Failed to opendir %s\n", topdir); > > + ret = -errno; > > ditto > > > + goto out; > > + } > > + bidlist = strlist__new(NULL, NULL); > > + strlist__for_each(nd, toplist) { > > + if (asprintf(&linkdir, "%s/%s", topdir, nd->s) < 0) { > > + ret = -errno; > > + goto out; > > + } > > + /* Open the lower-level directory */ > > + list = lsdir(linkdir, lsdir_no_dot_filter); > > + if (!list) { > > + pr_debug("Failed to open %s: %d\n", linkdir, -errno); > > + goto next; > > + } > > + strlist__for_each(nd2, list) { > > + ret = snprintf(sbuild_id, SBUILD_ID_SIZE, "%s%s", > > + nd->s, nd2->s); > > + if (ret != SBUILD_ID_SIZE - 1) { > > + pr_debug("%s/%s is not buildid cache\n", > > + nd->s, nd2->s); > > + continue; > > + } > > + strlist__add(bidlist, sbuild_id); > > strlist__add() can fail, please check its result. Ah, I forgot to update that... > > > + } > > + strlist__delete(list); > > +next: > > + free(linkdir); > > + } > > + > > + *result = bidlist; > > +out: > > + if (toplist) > > + strlist__delete(toplist); > > No need for checking toplist. OK. > > > + free(topdir); > > + > > + return ret; > > +} > > <SNIP> > > - Arnaldo Thank you! -- Masami Hiramatsu <mhiramat@...nel.org>
Powered by blists - more mailing lists