Skip to content

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

Open
camthesaxman wants to merge 4 commits into
Neverball:masterfrom
camthesaxman:user_dir
Open

add command line option for user dir to allow portable install#159
camthesaxman wants to merge 4 commits into
Neverball:masterfrom
camthesaxman:user_dir

Conversation

@camthesaxman

Copy link
Copy Markdown
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

parasti commented Nov 3, 2017

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
Contributor

Any reason why this is not yet merged?

@parasti

parasti commented Jun 11, 2018

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor

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

Comment thread share/base_config.c
home = pick_home_path();
user = concat_string(home, "/", CONFIG_USER, NULL);
if (arg_user_path)
user = arg_user_path;

Copy link
Copy Markdown
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

parasti commented Jun 12, 2018

Copy link
Copy Markdown
Member

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