[2/8] zone: accept also hids in zone_config()
Message ID | 1500998833-10543-3-git-send-email-jonatan.schlag@ipfire.org |
---|---|
State | New |
Headers |
Return-Path: <network-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (unknown [172.28.1.200]) by web02.ipfire.org (Postfix) with ESMTP id 8DAAE61C7D for <patchwork@ipfire.org>; Tue, 25 Jul 2017 18:07:24 +0200 (CEST) Received: from mail01.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id BBED027CE; Tue, 25 Jul 2017 18:07:23 +0200 (CEST) Received: from ipfire.localdomain (dslb-088-073-218-016.088.073.pools.vodafone-ip.de [88.73.218.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 9A9852769; Tue, 25 Jul 2017 18:07:20 +0200 (CEST) From: Jonatan Schlag <jonatan.schlag@ipfire.org> To: network@lists.ipfire.org Subject: [PATCH 2/8] zone: accept also hids in zone_config() Date: Tue, 25 Jul 2017 18:07:07 +0200 Message-Id: <1500998833-10543-3-git-send-email-jonatan.schlag@ipfire.org> X-Mailer: git-send-email 2.6.3 In-Reply-To: <1500998833-10543-1-git-send-email-jonatan.schlag@ipfire.org> References: <1500998833-10543-1-git-send-email-jonatan.schlag@ipfire.org> X-BeenThere: network@lists.ipfire.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List for the network package <network.lists.ipfire.org> List-Unsubscribe: <http://lists.ipfire.org/mailman/options/network>, <mailto:network-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <http://lists.ipfire.org/pipermail/network/> List-Post: <mailto:network@lists.ipfire.org> List-Help: <mailto:network-request@lists.ipfire.org?subject=help> List-Subscribe: <http://lists.ipfire.org/mailman/listinfo/network>, <mailto:network-request@lists.ipfire.org?subject=subscribe> Errors-To: network-bounces@lists.ipfire.org Sender: "network" <network-bounces@lists.ipfire.org> |
Message
Jonatan Schlag
July 26, 2017, 2:07 a.m. UTC
Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
src/functions/functions.zone | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)
Comments
Hi, On Tue, 2017-07-25 at 18:07 +0200, Jonatan Schlag wrote: > Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org> > --- > src/functions/functions.zone | 33 ++++++++++++++++++++++++--------- > 1 file changed, 24 insertions(+), 9 deletions(-) > > diff --git a/src/functions/functions.zone b/src/functions/functions.zone > index 28a22fe..091bcd6 100644 > --- a/src/functions/functions.zone > +++ b/src/functions/functions.zone > @@ -550,28 +550,43 @@ zone_config() { > zone_config_new "${zone}" "$@" > ;; > destroy) > + # id could be also a valid hid > local id=${1} > - if zone_config_id_is_valid ${zone} ${id}; then > + # usually we will get a hid so we check first if we > get one > + if zone_config_hid_is_valid ${zone} ${id}; then > + # Convert the hid into an id > + id=$(zone_config_convert_hid_to_id ${zone} > ${id}) > zone_config_destroy "${zone}" "$@" Could we not just call "zone_config_convert_hid_to_id" (maybe the name would be better without "convert") and if it does not return an ID, we assume that the HID does not exist? That would save us running through all the hooks twice which is quite an expensive thing to do. > else > - log ERROR "${id} is not a valid id" > + # We get no valid hid so we check if we get a > valid id > + if zone_config_id_is_valid ${zone} ${id}; > then > + zone_config_destroy "${zone}" "$@" > + else > + log ERROR "${id} is not a valid id or > hid" > + fi > fi > ;; > list) > zone_config_list "${zone}" "$@" > ;; > *) > - # Check is we get a valid id > - # TODO This could be also a valid hid > + # id could be also a valid hid > local id=${cmd} > - > - if zone_config_id_is_valid ${zone} ${id} && [[ ${1} > == "edit" ]]; then > + # usually we will get a hid so we check first if we > get one > + if zone_config_hid_is_valid ${zone} ${id} && [[ ${1} > == "edit" ]]; then > + id=$(zone_config_convert_hid_to_id ${zone} > ${id}) Can we also remove this ${1} == edit thing. That has confused me quite a few times and doesn't make the code very obvious to read. But same thing here as above. Could be something like: local id = $(hid to id ${hid} if ! isset id; then ... check if HID is an ID fi ... assume that we have a valid ID from here on ... This would have a little bit of a performance penalty if the user has given an ID or an invalid HID. But I think it is rather likely that someone will give a HID than an ID. Mainly because auto-completion will give a HID instead of an ID. > shift 1 > zone_config_edit "${zone}" "${id}" "$@" > else > - error "Unrecognized argument: ${cmd}" > - cli_usage root-zone-config-subcommands > - exit ${EXIT_ERROR} > + # We get no valid hid so we check if we get a > valid id > + if zone_config_id_is_valid ${zone} ${id} && > [[ ${1} == "edit" ]]; then > + shift 1 > + zone_config_edit "${zone}" "${id}" > "$@" > + else > + error "Unrecognized argument: ${cmd}" > + cli_usage root-zone-config- > subcommands > + exit ${EXIT_ERROR} > + fi > fi > ;; > esac