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
Search for subgames using $MINETEST_SUBGAME_PATH. #1609
Conversation
👍 |
@@ -66,6 +66,19 @@ SubgameSpec findSubgame(const std::string &id) | |||
std::string share = porting::path_share; | |||
std::string user = porting::path_user; | |||
std::vector<GameFindPath> find_paths; | |||
|
|||
char *search_paths = getenv("MINETEST_SUBGAME_PATH"); | |||
char *search_path = strtok(search_paths, ":"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From Linux man pages:
As typically implemented, getenv() returns a pointer to a string within the environment list. The caller must take care not to modify this string, since that would change the environment of the process.
From FreeBSD man pages:
The getenv() function obtains the current value of the environment vari- able, name. The application should not modify the string pointed to by the getenv() function.
strtok
(which is non-reentrant btw) obviously modifies the string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I'll fix that issue.
80a1054
to
fe9e042
Compare
I updated this patch to use the reentrant string tokenizer Any other issues? I must confess that I don't often write C/C++, so it's very possible that I'm still missing something. Thanks for reviewing this. |
73b8c08
to
56195dc
Compare
The env var should be documented in the man pages. |
dest[0] = '\0'; | ||
} else { | ||
int path_length = strlen(subgame_path) + 1; | ||
dest = new char[path_length]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You never free the allocated memory. Also it's probably better to use std::string and let it manage memory for you. You can split it using class Strfnd, defined in src/strfnd.h
; it's in Finnish but we have to live with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this. I was anticipating a memory management issue.
fe9e042
to
5dd4f70
Compare
Looks good. |
Thanks @sfan5! |
Using an environment variable allows the subgame search paths to be defined by the user, rather than fixed at compile time. This patch will search the paths contained within the environment variable
MINETEST_SUBGAME_PATH
for subgames before searching the share and home directories. When this environment variable is not set, Minetest behaves as it has prior to this patch.One particular use-case for this patch is to allow Minetest to be played on the GNU system running the GNU Guix package manager, where applications are installed on a per-user basis and packaged files are immutable. In other words, there's no
/usr/share/minetest/games
directory to write to. The necessary search path is dependent upon which user is running Minetest and where their "profile" (installed packages) is located.