zilexa / autosetup-VERO4K_RPi

Automatically installs and configures a RPi device running OSMC or Debian to get a "Netflix-like" experience.
19 stars 6 forks source link

Various fixes #7

Closed Jekotia closed 1 year ago

Jekotia commented 5 years ago

I hope this doesn't sound too harsh; I'm not great with words and everything below had to be addressed before I could get this script to run properly.

There's several instances of variables not being expanded, or inviting problems caused by symbols in user input, due to the incorrect use of quotes.

If quoting a variable that is to be expanded, double-quotes MUST be used. Single quotes lead to literal text. It's also generally good practice to enclose user input fields in single quotes so that if they use characters which have a special meaning to the shell, the characters are still handled as a literal string.

You have an error in your variable naming; in one instance of trying to use the TraktUser variable you instead use TraktUsername which does not exist. There's also an instance of SyncThing being mis-spelled.

You're using an obsolete invocation for system service control that is not supported by current Debian-derived distributions. systemctl is the base command, followed by the action, followed by the service.

You also appear to be generally lacking any checks for if operations succeeded. This can be quite dangerous. You should be checking exit codes or otherwise verifying if operations were successful.

I would recommend the wonderful terminal application Shellcheck. It's basically a syntax checker for shell scripting. Not only does it check for issues, it has unique numerical ID's for each which you can then lookup on the project's wiki for more information.