mbox

[4/5] config: add new functions

Message ID 1500305066-12510-4-git-send-email-jonatan.schlag@ipfire.org
State Accepted
Commit 2b474c46d4784047487e730050e831c56530ee40
Headers

Message

Jonatan Schlag July 18, 2017, 1:24 a.m. UTC 
  This patch add two new functions:
config_get_id_from_config()
config_get_hook_from_config

Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
 src/functions/functions.config | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Michael Tremer July 18, 2017, 6:22 a.m. UTC | #1 
Hi,

On Mon, 2017-07-17 at 17:24 +0200, Jonatan Schlag wrote:
> This patch add two new functions:
> config_get_id_from_config()
> config_get_hook_from_config
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
>  src/functions/functions.config | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/functions/functions.config
> b/src/functions/functions.config
> index 854f490..e11a1c2 100644
> --- a/src/functions/functions.config
> +++ b/src/functions/functions.config
> @@ -51,3 +51,23 @@ config_domainname() {
>  	# the domain part.
>  	print "${hostname#*.}"
>  }
> +
> +config_get_id_from_config() {
> +	# This function returns the id for a given config name
> +	# Example 'dhcp.0' => 0
> +	assert [ $# -eq 1 ]
> +	local config=${1}
> +
> +	local hook=$(config_get_hook_from_config ${config})
> +	echo "${config//"${hook}."/}"
> +
> +}

There is an extra empty line.

And calling config_get_hook_from_config() is an expensive call. It
forks a subshell and loads the hook and so on.

I am sure that you can find the ID without knowing what is coming
before. That would increase performance of this function tremendously.

> +
> +config_get_hook_from_config() {
> +	# This function returns the hook for a given config name
> +	# Example 'dhcp.0' => dhcp
> +	assert [ $# -eq 1 ]
> +	local config=${1}
> +
> +	echo "${config//.*[[:digit:]]/}"
> +}
Michael Tremer July 18, 2017, 6:23 a.m. UTC | #2 
On Mon, 2017-07-17 at 16:22 -0400, Michael Tremer wrote:
> Hi,
> 
> On Mon, 2017-07-17 at 17:24 +0200, Jonatan Schlag wrote:
> > This patch add two new functions:
> > config_get_id_from_config()
> > config_get_hook_from_config
> > 
> > Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> > ---
> >  src/functions/functions.config | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/src/functions/functions.config
> > b/src/functions/functions.config
> > index 854f490..e11a1c2 100644
> > --- a/src/functions/functions.config
> > +++ b/src/functions/functions.config
> > @@ -51,3 +51,23 @@ config_domainname() {
> >  	# the domain part.
> >  	print "${hostname#*.}"
> >  }
> > +
> > +config_get_id_from_config() {
> > +	# This function returns the id for a given config name
> > +	# Example 'dhcp.0' => 0
> > +	assert [ $# -eq 1 ]
> > +	local config=${1}
> > +
> > +	local hook=$(config_get_hook_from_config ${config})
> > +	echo "${config//"${hook}."/}"
> > +
> > +}
> 
> There is an extra empty line.
> 
> And calling config_get_hook_from_config() is an expensive call. It
> forks a subshell and loads the hook and so on.
> 
> I am sure that you can find the ID without knowing what is coming
> before. That would increase performance of this function
> tremendously.

Actually I am wrong here. This is the function below :)

Still you can avoid calling it...

> 
> > +
> > +config_get_hook_from_config() {
> > +	# This function returns the hook for a given config name
> > +	# Example 'dhcp.0' => dhcp
> > +	assert [ $# -eq 1 ]
> > +	local config=${1}
> > +
> > +	echo "${config//.*[[:digit:]]/}"
> > +}