perf: Re-Add make_absolute_path
in [Kernel]
|
From: Arnaldo Carvalho de Melo on 12 Jul 2010 10:10 Em Mon, Jul 12, 2010 at 04:04:12PM +0530, Srikar Dronamraju escreveu: > perf: Re-Add make_absolute_path > > perf probe for uprobes would use make_absolute_path. > make_absolute_path can be used to convert a file name to a dso name. > > so if user specifies the function to be traced as malloc@/lib/libc.so.6 > it needs to be converted to malloc(a)libc-2.5.so Isn't this the other way around? I.e. "if the user specifies malloc(a)libc-2.5.so it needs to be converted to malloc@/lib/libc.so.6"? Also please remove the xstrdup and die calls from this function, we're trying to get rid of all such 'panic' like functions so that we can librarize as much code as possible. > This patch reverts a part of a41794cdd7ee94a5199e14f642c26d649d383fa5 > Signed-off-by: Srikar Dronamraju <srikar(a)linux.vnet.ibm.com> > --- > > tools/perf/util/abspath.c | 81 +++++++++++++++++++++++++++++++++++++++++++++ > tools/perf/util/cache.h | 1 + > 2 files changed, 82 insertions(+), 0 deletions(-) > > > diff --git a/tools/perf/util/abspath.c b/tools/perf/util/abspath.c > index 0e76aff..0a985fd 100644 > --- a/tools/perf/util/abspath.c > +++ b/tools/perf/util/abspath.c > @@ -1,5 +1,86 @@ > #include "cache.h" > > +/* > + * Do not use this for inspecting *tracked* content. When path is a > + * symlink to a directory, we do not want to say it is a directory when > + * dealing with tracked content in the working tree. > + */ > +static int is_directory(const char *path) > +{ > + struct stat st; > + return !stat(path, &st) && S_ISDIR(st.st_mode); > +} > + > +/* We allow "recursive" symbolic links. Only within reason, though. */ > +#define MAXDEPTH 5 > + > +const char *make_absolute_path(const char *path) > +{ > + static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1]; > + char cwd[1024] = ""; > + int buf_index = 1, len; > + > + int depth = MAXDEPTH; > + char *last_elem = NULL; > + struct stat st; > + > + if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) > + die("Too long path: %.*s", 60, path); > + > + while (depth--) { > + if (!is_directory(buf)) { > + char *last_slash = strrchr(buf, '/'); > + if (last_slash) { > + *last_slash = '\0'; > + last_elem = xstrdup(last_slash + 1); > + } else { > + last_elem = xstrdup(buf); > + *buf = '\0'; > + } > + } > + > + if (*buf) { > + if (!*cwd && !getcwd(cwd, sizeof(cwd))) > + die("Could not get current working directory"); > + > + if (chdir(buf)) > + die("Could not switch to '%s'", buf); > + } > + if (!getcwd(buf, PATH_MAX)) > + die("Could not get current working directory"); > + > + if (last_elem) { > + len = strlen(buf); > + > + if (len + strlen(last_elem) + 2 > PATH_MAX) > + die("Too long path name: '%s/%s'", > + buf, last_elem); > + buf[len] = '/'; > + strcpy(buf + len + 1, last_elem); > + free(last_elem); > + last_elem = NULL; > + } > + > + if (!lstat(buf, &st) && S_ISLNK(st.st_mode)) { > + len = readlink(buf, next_buf, PATH_MAX); > + if (len < 0) > + die("Invalid symlink: %s", buf); > + if (PATH_MAX <= len) > + die("symbolic link too long: %s", buf); > + next_buf[len] = '\0'; > + buf = next_buf; > + buf_index = 1 - buf_index; > + next_buf = bufs[buf_index]; > + } else > + break; > + } > + > + if (*cwd && chdir(cwd)) > + die("Could not change back to '%s'", cwd); > + > + return buf; > +} > + > static const char *get_pwd_cwd(void) > { > static char cwd[PATH_MAX + 1]; > diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h > index 27e9ebe..0dfed40 100644 > --- a/tools/perf/util/cache.h > +++ b/tools/perf/util/cache.h > @@ -73,6 +73,7 @@ static inline int is_absolute_path(const char *path) > return path[0] == '/'; > } > > +const char *make_absolute_path(const char *path); > const char *make_nonrelative_path(const char *path); > char *strip_path_suffix(const char *path, const char *suffix); > -- 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/
From: Steven Rostedt on 12 Jul 2010 10:40 On Mon, 2010-07-12 at 11:00 -0300, Arnaldo Carvalho de Melo wrote: > Also please remove the xstrdup and die calls from this function, we're > trying to get rid of all such 'panic' like functions so that we can > librarize as much code as possible. > What I found useful with the "die" calls with trace-cmd is that I made them weak, and then they could be overwritten by apps. Thus, in kernelshark, the die and warning functions produce pop-ups and bug reports. -- Steve -- 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/
From: Srikar Dronamraju on 12 Jul 2010 11:50 * Arnaldo Carvalho de Melo <acme(a)infradead.org> [2010-07-12 11:00:23]: > Em Mon, Jul 12, 2010 at 04:04:12PM +0530, Srikar Dronamraju escreveu: > > perf: Re-Add make_absolute_path > > > > perf probe for uprobes would use make_absolute_path. > > make_absolute_path can be used to convert a file name to a dso name. > > > > so if user specifies the function to be traced as malloc@/lib/libc.so.6 > > it needs to be converted to malloc(a)libc-2.5.so > > Isn't this the other way around? I.e. "if the user specifies > malloc(a)libc-2.5.so it needs to be converted to malloc@/lib/libc.so.6"? > Actually we dont need to convert malloc(a)libc-2.5.so to a malloc@/lib/libc.so.6. Because we can match the shortname of the dso. Problem will occur when users specifies a full path of the file. Since the file can refer to a symbolic link and the dso will have just the short name or the target file name. Here I am using make_absolute_path to resolve to the target file. Now we can then either check on dso full names or short names. I have chosen to use the short name. > Also please remove the xstrdup and die calls from this function, we're > trying to get rid of all such 'panic' like functions so that we can > librarize as much code as possible. Okay, Can I do that in subsequent versions of the patchset? -- Thanks and Regards Srikar -- 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/
From: Arnaldo Carvalho de Melo on 12 Jul 2010 12:20 Em Mon, Jul 12, 2010 at 10:30:36AM -0400, Steven Rostedt escreveu: > On Mon, 2010-07-12 at 11:00 -0300, Arnaldo Carvalho de Melo wrote: > > Also please remove the xstrdup and die calls from this function, we're > > trying to get rid of all such 'panic' like functions so that we can > > librarize as much code as possible. > What I found useful with the "die" calls with trace-cmd is that I made > them weak, and then they could be overwritten by apps. Thus, in > kernelshark, the die and warning functions produce pop-ups and bug > reports. Well, I prefer to follow the kernel way of doing things, i.e. to propagate as much as possible up the callchain the error return value, so that the apps can handle it in any way they prefer, i.e. die() calls in tools/perf/builtin-foo.c are okayish, but not on tools/perf/util/. When I'm writing a tools/perf/builtin-.c file I also don't use die() calls, as some routines may end up moving to the library, so its nice to avoid them from the start. The pr_{warning,err,info,etc} calls do something along the lines of what you do, but not by marking them weak, the routine that is ultimately called checks what kind of UI is being used and calls the appropriate one (TUI/NEWT of stdio). - Arnaldo -- 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/
From: Arnaldo Carvalho de Melo on 12 Jul 2010 12:30 Em Mon, Jul 12, 2010 at 09:03:57PM +0530, Srikar Dronamraju escreveu: > * Arnaldo Carvalho de Melo <acme(a)infradead.org> [2010-07-12 11:00:23]: > > Em Mon, Jul 12, 2010 at 04:04:12PM +0530, Srikar Dronamraju escreveu: > > > perf probe for uprobes would use make_absolute_path. > > > make_absolute_path can be used to convert a file name to a dso name. > > > so if user specifies the function to be traced as malloc@/lib/libc.so.6 > > > it needs to be converted to malloc(a)libc-2.5.so > > Isn't this the other way around? I.e. "if the user specifies > > malloc(a)libc-2.5.so it needs to be converted to malloc@/lib/libc.so.6"? > Actually we dont need to convert malloc(a)libc-2.5.so to a > malloc@/lib/libc.so.6. Because we can match the shortname of the dso. Humm, I see, it is path based, so the first libc-2.5.so that appears in the LD_LIBRARY_PATH equivalent used in this code will be user, is that right? I.e. if I'm testing some new libc-2.5.so that provides, say, private futexes while the one in my distro still doesn't have this feature, I'll have to specify the absolute name or make sure it is before the system's libc-2.5.so in the LD_LIBRARY_PATH, right? > Problem will occur when users specifies a full path of the file. > Since the file can refer to a symbolic link and the dso will have just > the short name or the target file name. Here I am using > make_absolute_path to resolve to the target file. > Now we can then either check on dso full names or short names. > I have chosen to use the short name. Humm, so what you want is one of: realpath - return the canonicalized absolute pathname canonicalize_file_name - return the canonicalized filename Can you please check the man pages for both before we decide re-introducing make_absolute_path? > > Also please remove the xstrdup and die calls from this function, we're > > trying to get rid of all such 'panic' like functions so that we can > > librarize as much code as possible. > > Okay, Can I do that in subsequent versions of the patchset? yeah, its ok if you don't end up using one of the suggested libc functions above :-) - Arnaldo -- 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/
|
Next
|
Last
Pages: 1 2 Prev: NOTICE! Next: [PATCH] check name_len before down_read xattr_sem and sb_read in ext2_xattr_get |