Hi On Fri, 22 Jan 2021 at 05:55, Willy Tarreau <[email protected]> wrote: > > Hi David, > > On Thu, Jan 21, 2021 at 09:28:01PM +0000, David CARLIER wrote: > > Hi > > > > Here a little patch proposal. Since it is relatively self-contained > > change, it could be possibly backported smoothly. > > I disagree for two reasons: > - the description doesn't indicate at all the nature of the change but > just mentions a code refactoring ; > > - you also completely changed the linux code to something that is more > likely > to break in some environments (e.g. in chroots) since it tries to open > /proc while the first one was using the process' aux vector
That s fair point, did not think of chroot case. > > I'm not at all against completing the linux code if there is an identified > issue or limitation, but it if so it must be done in a separate patch and > be justified in that patch's commit message. Maybe it would even make sense > to use the current method as a fallback for yours or conversely. > > In any case, please make sure the patch mentioning FreeBSD+Mac only does > that and nothing else. As such I guess you'll need to have a preliminary > patch that only changes the function's API. In addition, please update > the comment on top of the function to reflect your API change, because > this is definitely wrong: > > /* Tries to report the executable path name on platforms supporting this. > If > * not found or not possible, returns NULL. > */ > -const char *get_exec_path() > +int get_exec_path(char *buf, size_t len) > > So if I summarize: > > - 1 patch to first update the get_exec_path() API > - 1 patch to possibly complete the way Linux retrieves its exec path > *if needed at all* (I don't think anyone mentioned an issue there) > - 1 patch to add support for freebsd > - 1 patch to add support for Mac (since freebsd and Mac do it completely > differently) > > Thanks! > Willy
From 2726e38e07b66cd47f7a8b89f627fe2f80265e8d Mon Sep 17 00:00:00 2001 From: David Carlier <[email protected]> Date: Fri, 22 Jan 2021 07:06:12 +0000 Subject: [PATCH] BUILD/MINOR: get_exe_path api update for further oses support. copying into a buffer instead as it is the way most of apis work. Signed-off-by: David Carlier <[email protected]> --- include/haproxy/tools.h | 2 +- src/log.c | 5 +++-- src/tools.c | 13 ++++++++----- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/include/haproxy/tools.h b/include/haproxy/tools.h index 76e6e730c..e55be6f22 100644 --- a/include/haproxy/tools.h +++ b/include/haproxy/tools.h @@ -991,7 +991,7 @@ void dump_addr_and_bytes(struct buffer *buf, const char *pfx, const void *addr, void dump_hex(struct buffer *out, const char *pfx, const void *buf, int len, int unsafe); int may_access(const void *ptr); const void *resolve_sym_name(struct buffer *buf, const char *pfx, const void *addr); -const char *get_exec_path(); +int get_exec_path(char *buf, size_t len); #if defined(USE_BACKTRACE) /* Note that this may result in opening libgcc() on first call, so it may need diff --git a/src/log.c b/src/log.c index c24fdbdeb..8ce0e2053 100644 --- a/src/log.c +++ b/src/log.c @@ -1098,11 +1098,12 @@ void ha_alert(const char *fmt, ...) if (!(global.mode & MODE_QUIET) || (global.mode & (MODE_VERBOSE | MODE_STARTING))) { if (!(warned & WARN_EXEC_PATH)) { - const char *path = get_exec_path(); + char path[PATH_MAX]; + int r = get_exec_path(path, sizeof(path)); warned |= WARN_EXEC_PATH; ha_notice("haproxy version is %s\n", haproxy_version); - if (path) + if (r == 0) ha_notice("path to executable is %s\n", path); } va_start(argp, fmt); diff --git a/src/tools.c b/src/tools.c index 19b612250..3169f5d6d 100644 --- a/src/tools.c +++ b/src/tools.c @@ -4593,17 +4593,20 @@ void debug_hexdump(FILE *out, const char *pfx, const char *buf, } /* Tries to report the executable path name on platforms supporting this. If - * not found or not possible, returns NULL. + * not found or not possible, returns -1. */ -const char *get_exec_path() +int get_exec_path(char *buf, size_t len) { - const char *ret = NULL; + int ret = -1; #if (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 16)) long execfn = getauxval(AT_EXECFN); - if (execfn && execfn != ENOENT) - ret = (const char *)execfn; + if (execfn && execfn != ENOENT) { + strncpy(buf, (const char *)execfn, len - 1); + buf[len - 1] = 0; + ret = 0; + } #endif return ret; } -- 2.30.0
