arno-iptables-firewall / aif

GNU General Public License v2.0
151 stars 24 forks source link

No loc in conf #74

Closed arnova closed 4 years ago

arnova commented 4 years ago

@abelbeck : I've updated the PR. I'm tempted to get rid of the "verbose" autodetection of the env-file and instead always load it from the relative path. What do you think?

arnova commented 4 years ago

I've added a commit showing my intention. This would really simplifies the logic a lot and prevents a lot of user errors.

abelbeck commented 4 years ago

@arnova : You could use ${0%/*} instead of $(dirname $0) as it saves a subshell and command call.

A test:

#!/bin/sh

echo "arg 0: $0"

echo "dirname: $(dirname $0)"

echo "posix str: ${0%/*}"

Note: if $0 does not contain a / the posix string method will not return ., but that case would cause other problems without a leading /.

arnova commented 4 years ago

@arnova : You could use ${0%/*} instead of $(dirname $0) as it saves a subshell and command call.

A test:

#!/bin/sh

echo "arg 0: $0"

echo "dirname: $(dirname $0)"

echo "posix str: ${0%/*}"

Note: if $0 does not contain a / the posix string method will not return ., but that case would cause other problems without a leading /.

Ah yes, well I think in this we can use an absolute path always has a / ;-)

Btw. do you also happen to have a suggestion on how to get rid of the ugly /../ this way? EDIT: Got it :)

abelbeck commented 4 years ago

@abelbeck : It just came to mind that ${0%/*} doesn't work if the script is called as ./arno-iptables-firewall. I think we should do "../../" then, right? Or do you have a better solution?

@arnova : ${0%/*} works but ${0%/*/*} does not in the ./arno-iptables-firewall case.

You could use $PWD (in most all shells) or define AIF_PWD=$(pwd) early in the script and use ${AIF_PWD%/*} instead of ${0%/*/*}. Or use AIF_PWD=$(pwd -P) if you want to unravel symlinks.

arnova commented 4 years ago

@abelbeck : It just came to mind that ${0%/*} doesn't work if the script is called as ./arno-iptables-firewall. I think we should do "../../" then, right? Or do you have a better solution?

@arnova : ${0%/*} works but ${0%/*/*} does not in the ./arno-iptables-firewall case.

You could use $PWD (in most all shells) or define AIF_PWD=$(pwd) early in the script and use ${AIF_PWD%/*} instead of ${0%/*/*}. Or use AIF_PWD=$(pwd -P) if you want to unravel symlinks.

$PWD isn't POSIX, is it? And the problem with $(pwd) is that it returns the current path, not the path of the script. So if you cd to ie. /tmp and call /usr/local/sbin/arno-iptables-firewall, pwd will return /tmp

abelbeck commented 4 years ago

And the problem with $(pwd) is that it returns the current path...

@arnova : Egg on my face, you are absolutely correct.

$PWD isn't POSIX, is it?

Good question ... I think it is, but has the same problem as $(pwd) Ref: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03 PWD This variable shall represent an absolute pathname of the current working directory. It shall not contain any components that are dot or dot-dot. The value is set by the cd utility, and by the sh utility during initialization.

Hmmm, this is an interesting problem...

abelbeck commented 4 years ago

@arnova : Looks good.