Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid program_invocation_name which is available only on Linux #242

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

vasild
Copy link
Contributor

@vasild vasild commented Oct 9, 2024

program_invocation_name is defined by the system headers only on Linux, or at least it is not available on FreeBSD.

Thus, maintain own global variable that contains the program name.

Boss/Main.cpp Outdated
@@ -62,6 +64,7 @@ class Main::Impl {
{
assert(argv.size() >= 1);
argv0 = argv[0];
g_argv0 = argv0.c_str();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have a lifetime issue? If Main::Impl::argv0 is destroyed, then the global g_argv0 will become dangling. Maybe copy the string to a global buffer?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that is dangerous. ChatGPT recommends a fix:
https://chatgpt.com/share/6712ab8d-b780-8008-8bba-468bea99aad2

Can you fix and re-push?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, used std::string (which holds the data) rather than a bare pointer, pointing to some other place that can be destroyed too early.

@vasild vasild force-pushed the program_invocation_name branch from fbcde82 to 2442b30 Compare October 24, 2024 13:06
@ksedgwic
Copy link
Collaborator

I found a problem w/ the unit tests; they don't set the g_argv0 var on startup. Normally they don't have unhandled exceptions, but when developing they do and are very useful.

I have a patch to use the built-ins as a fallback. I'll try to push it here ...

@ksedgwic
Copy link
Collaborator

@vasild could you make sure that compiles with FreeBSD?

`program_invocation_name` is defined by the system headers only on
Linux, or at least it is not available on FreeBSD.

Introduce own global variable that contains the program name. If
it is unset then use `getprogname()` or `program_invocation_name`.

Co-authored-by: Ken Sedgwick <[email protected]>
@vasild vasild force-pushed the program_invocation_name branch from 1016acf to f545cbf Compare October 25, 2024 09:06
@vasild
Copy link
Contributor Author

vasild commented Oct 25, 2024

Yes, it compiles and I squashed the two commits and made some further changes:

  • test for the presence of getprogname() instead of assuming "if FreeBSD then getprogname() must be present" because that would exclude other platforms that have getprogname() but are not FreeBSD.
  • return std::string from the new progname() to avoid potential issues with the lifetime - returning a pointer to something which if changed invalidates the pointer. Now the .c_str() usage is limited to the call of snprintf().

@ksedgwic ksedgwic merged commit f32426d into ZmnSCPxj:master Oct 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants