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
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
