News:

The Forum Rules and Guidelines
Our forum has Rules and Guidelines. Please, be kind and read them ;).

check_and_set_dir: don't reject every path for userdir and installdir

Started by sodiboo, May 06, 2024, 12:51:17 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

sodiboo

The previous implementation had two issues:

(1) It only accepts directories that exist. In practice, this means that no directory would be accepted, and it would always fallback to its default. The default value will create a directory before returning the default value.
(2) If `testfile` parameter was null, it would never accept any path. In practice, but also in theory, this means that no directory would be accepted for any reason, because installdir and userdir passed null to this. Only basedir would have a testfile.

Summary of changes:

- Only check testfile if testfile is non-null. If it is null, it will not cause a dir to be rejected.
- Don't create directories when resolving default installdir or userdir.
- Create the directories of the final resolved userdir and installdirs in simmain.
- There are no other usages of dr_query_homedir() or dr_query_installdir() so no other code is afffected by that change.

not quite sure what format you expect patches to be in, so here's the diff i'm putting in nixpkgs for now

https://raw.githubusercontent.com/NixOS/nixpkgs/2e7d7fb577eec29f6dd3566716b77e74752f9666/pkgs/games/simutrans/fix-dirs.patch

and here it is as output from svn diff

https://gist.githubusercontent.com/sodiboo/5e91535e70bfd5e26f34355db481993c/raw/060e090eb34baabb66269bdff3118264dfdf4274/fix-dirs.patch

prissi

install_dir could be not writable by without admin rights (like unix and windows when the data is in bin pr program files) and thus directory creation could fail there.

But I submitted it anyway as that would have also led to problems before.

sodiboo

I thought the whole point of the installdir is that it specifically shouldn't require admin rights? e.g. the unprivileged game can install arbitrary files to that directory

prissi

Yes, but that is not possible under all settings. If simutrans paks are under programFiles in windows, then they are write protected. But this is hopefully a thing of the past since 123.0.2 simutrans installer will prefer other folders.

Roboron

As discussed in discord, sodiboo's patch did not really create the directories if they didn't exist (which was its original goal). I "completed" it in r11204.