common: Get program name based on executable path if possible #307
Conversation
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
ueno
force-pushed
the
wip/dueno/progname
branch
from
f505ed7 to
1fcf2cb
Compare
| @@ -112,7 +112,28 @@ getprogname (void) | |||
| if (p != NULL) | |||
| name = p + 1; | |||
| #elif defined (HAVE_PROGRAM_INVOCATION_SHORT_NAME) | |||
There was a problem hiding this comment.
Is this workaround only needed in this particular branch, or should it cover all of them?
There was a problem hiding this comment.
The other branches are basically for other OSes, which likely don't have /proc/self/exe I suspect. For the meantime I'd skip the effect to Linux only.
| if (!buf) | ||
| buf = realpath ("/proc/self/exe", NULL); | ||
|
|
||
| if (buf && strncmp (buf, name, strlen (buf)) == 0) |
There was a problem hiding this comment.
This won't work if name isn't a realpath itself, e.g. a relative path or a symlinked path. Is there a need to cover those cases as well?
There was a problem hiding this comment.
Indeed; however, it would bring too much complexity in this exceptional path IMO. Let's skip it until we find an actual use-case that require such checks.
ueno
force-pushed
the
wip/dueno/progname
branch
7 times, most recently
from
71500a0 to
b8fb0e4
Compare
common/frob-getprogname.c
Outdated
| } | ||
| if (nread == 0) | ||
| break; | ||
| if (sizeof(buffer) - offset < nread) { |
There was a problem hiding this comment.
Since read(..., nbytes) <= nbytes, and thus nread <= sizeof(buffer) - offset, I don't think this condition is ever true. Maybe <= or ==?
There was a problem hiding this comment.
Indeed, let's remove the check then.
common/frob-getprogname.c
Outdated
| assert (WIFEXITED (status)); | ||
| assert (WEXITSTATUS (status) == 0); | ||
|
|
||
| p = strchr (buffer, '\n'); |
There was a problem hiding this comment.
I'm not sure what exactly are you anticipating here, so that a string would be missing a newline, but I suspect it has a chance of also not being null-terminated in this case.
I propose folding this check into a strncmp ("frob-getprogname\n", buffer, sizeof(buffer)) or, alternatively, null-terminating the buffer after reading it (and maybe not bothering with \n altogether).
There was a problem hiding this comment.
I replaced strchr with memchr so it also handles the non null-terminated case.
Some programs (e.g., Chromium) pack command line arguments into argv[0]. Check if it is the case by reading /proc/self/exe and extract the program name. Logic borrowed from: <https://github.com/mesa3d/mesa/commit/759b94038987bb983398cd4b1d2cb1c8f79817a9>.
ueno
force-pushed
the
wip/dueno/progname
branch
from
b8fb0e4 to
f00734e
Compare
|
Thank you for the detailed review! |
Some programs (e.g., Chromium) pack command line arguments into
argv[0]. Check if it is the case by reading/proc/self/exeand extract the program name.Logic borrowed from:
https://github.com/mesa3d/mesa/commit/759b94038987bb983398cd4b1d2cb1c8f79817a9.
Fixes #263.