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

add command line option for user dir to allow portable install #159

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

camthesaxman
Copy link
Member

Since commit 8a6e84a , it doesn't appear to be possible to make a portable installation of Neverball that can run completely from a USB drive (at least not that I know of). With 1.5.4, you could make a launcher that set the APPDATA environment variable to control that, but that method doesn't appear to work with SHGetFolderPath.

@parasti
Copy link
Member

parasti commented Nov 3, 2017

I don't know if you're aware of this, but this doesn't actually do what it claims to do. It doesn't set the user directory, it sets the parent directory of the user directory (referred to as "home" as in the HOME env var).

@camthesaxman
Copy link
Member Author

Yeah, guess I got caught up on the difference between the home dir and the user dir. It should work as expected, now.

@qwertychouskie
Copy link
Contributor

Any reason why this is not yet merged?

@parasti
Copy link
Member

parasti commented Jun 11, 2018

As I wrote in the pull review, this patch creates a bug by freeing memory that wasn't allocated. Do you not see it? That would make pull reviews pretty useless.

@qwertychouskie
Copy link
Contributor

Hmm, no, I don't see it...

home = pick_home_path();
user = concat_string(home, "/", CONFIG_USER, NULL);
if (arg_user_path)
user = arg_user_path;
Copy link
Member

Choose a reason for hiding this comment

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

user is passed to free() down the line, which will do something unexpected if user is a passed-in option string. This should either be wrapped in a strdup() or the concat_string() result should be a separate variable that can be passed to free() safely.

@parasti
Copy link
Member

parasti commented Jun 12, 2018

Whoops, I guess I had to "submit" it. I wrote it 7 months ago.

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.

3 participants