We now generate the new id inside the hook_new function. This is required because the hook_parse_cmdline function take now the id of the config as te first argument.

Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org --- src/functions/functions.zone | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/functions/functions.zone b/src/functions/functions.zone index 941314d..34cc983 100644 --- a/src/functions/functions.zone +++ b/src/functions/functions.zone @@ -1410,16 +1410,13 @@ zone_config_settings_read() { }

zone_config_settings_write() { - assert [ $# -ge 2 ] + assert [ $# -eq 3 ]

local zone="${1}" local hook="${2}" local id=${3}

- if ! isset id; then - id=$(zone_config_get_new_id ${zone}) - log DEBUG "ID for the config is: ${id}" - fi + assert isinteger id

local args if function_exists "hook_check_config_settings"; then

All hook_parse_cmdline take now the id as the first argument. This is required because zone_config_check_same_setting needs the id to ignore the config we edit in the moment. All hook_parse_cmdline functions are changed to keep the generic hook_edit function working. Another option was to write an own hook_edit function for the ipv4-static and ipv6-static hook. But this solution is not easier then to keep things more generic and equal. Makes also maintaining easier.

Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org --- src/hooks/configs/dhcp | 4 ++++ src/hooks/configs/ipv4-static | 7 ++++++- src/hooks/configs/ipv6-auto | 5 +++++ src/hooks/configs/ipv6-static | 7 ++++++- src/hooks/configs/pppoe-server | 5 +++++ 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/hooks/configs/dhcp b/src/hooks/configs/dhcp index 7f6780b..f1c43f3 100644 --- a/src/hooks/configs/dhcp +++ b/src/hooks/configs/dhcp @@ -35,6 +35,10 @@ hook_check_config_settings() { }

hook_parse_cmdline() { + local id=${1} + shift + + assert isinteger id while [ $# -gt 0 ]; do case "${1}" in --enable-ipv6) diff --git a/src/hooks/configs/ipv4-static b/src/hooks/configs/ipv4-static index ef74991..a6b010f 100644 --- a/src/hooks/configs/ipv4-static +++ b/src/hooks/configs/ipv4-static @@ -36,6 +36,11 @@ hook_check_config_settings() { }

hook_parse_cmdline() { + local id=${1} + shift + + assert isinteger id + local arg

while read -r arg; do @@ -99,7 +104,7 @@ hook_parse_cmdline() { exit ${EXIT_CONF_ERROR} fi

- if zone_config_check_same_setting "${zone}" "ipv4-static" "ADDRESS" "${ADDRESS}"; then + if zone_config_check_same_setting "${zone}" "ipv4-static" ${id} "ADDRESS" "${ADDRESS}"; then error "An ipv4-static config with the same IPv4 address is already configured" exit ${EXIT_CONF_ERROR} fi diff --git a/src/hooks/configs/ipv6-auto b/src/hooks/configs/ipv6-auto index 375e585..e89af28 100644 --- a/src/hooks/configs/ipv6-auto +++ b/src/hooks/configs/ipv6-auto @@ -31,6 +31,11 @@ hook_check_config_settings() { }

hook_parse_cmdline() { + local id=${1} + shift + + assert isinteger id + local arg

while read arg; do diff --git a/src/hooks/configs/ipv6-static b/src/hooks/configs/ipv6-static index c41401c..bb75240 100644 --- a/src/hooks/configs/ipv6-static +++ b/src/hooks/configs/ipv6-static @@ -34,6 +34,11 @@ hook_check_config_settings() { }

hook_parse_cmdline() { + local id=${1} + shift + + assert isinteger id + while [ $# -gt 0 ]; do case "${1}" in --address=*) @@ -49,7 +54,7 @@ hook_parse_cmdline() { shift done

- if zone_config_check_same_setting "${zone}" "ipv6-static" "ADDRESS" "${ADDRESS}"; then + if zone_config_check_same_setting "${zone}" "ipv6-static" ${id} "ADDRESS" "${ADDRESS}"; then error "An ipv6-static config with the same IPv6 address is already configured" exit ${EXIT_CONF_ERROR} fi diff --git a/src/hooks/configs/pppoe-server b/src/hooks/configs/pppoe-server index b4d2538..5861e70 100644 --- a/src/hooks/configs/pppoe-server +++ b/src/hooks/configs/pppoe-server @@ -49,6 +49,11 @@ hook_check_config_settings() { }

hook_parse_cmdline() { + local id=${1} + shift + + assert isinteger id + while [ $# -gt 0 ]; do case "${1}" in --dns-server=*)

Hi,

this patchset looks fine, but I think the solution is a little bit too complicated.

Could we not let hook_parse_cmdline() remain untouched and just let it parse all inputs? We need to parse all arguments anyways before we can make a decision. We could then check for any conflicts later in hook_edit().

That looks easier to me since:

a) the format of hook_parse_cmdline() would be the same throughout all zones, ports and configs

b) we do not need to carry around the ID variable. It will be empty any ways when we are creating a new configuration and then become difficult to handle.

Your thoughts?

Best, -Michael

On Sat, 2017-08-12 at 12:22 +0200, Jonatan Schlag wrote:

We now check if we get the config, which we edit in this moment and when we continue.

Fixes: #11451

Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org

src/functions/functions.zone | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/functions/functions.zone b/src/functions/functions.zone index 1eb492f..941314d 100644 --- a/src/functions/functions.zone +++ b/src/functions/functions.zone @@ -1155,12 +1155,13 @@ zone_config_check_same_setting() { # with the same setting is already configured for this zone. # Returns True when yes and False when no.

• assert [ $# -eq 4 ]

• assert [ $# -eq 5 ]

  local zone=${1} local hook=${2}

• local key=${3}
• local value=${4}

• local id=${3}

• local key=${4}

• local value=${5}

  # The key should be local for this function local ${key}

@@ -1171,6 +1172,12 @@ zone_config_check_same_setting() { if [[ $(zone_config_get_hook "${zone}" "${config}") != ${hook} ]]; then continue fi

• # Check if we get the config we want to create or edit (we
  

must ignore this config on edit)

• if [[ "${hook}.${id}" = "${config}" ]]; then
  

• 	continue
  

• fi
  

• # Get the value of the key for a given function zone_config_settings_read "${zone}" "${config}" \ --ignore-superfluous-settings "${key}"