mbox

[v3,1/5] util: refactor fwrite

Message ID 1500318318-18852-1-git-send-email-jonatan.schlag@ipfire.org
State Superseded
Headers

Message

Jonatan Schlag July 18, 2017, 5:05 a.m. UTC 
  This functions now:
	- return an error when the destination is a directory
	- creates the destination if the destination not exist

Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
 src/functions/functions.util | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

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

I find that this makes the function unnecessarily complicated.

It would be enough to keep it as it is and add the check if $file is a
directory.

If you would prefer to have a different error message when ever writing
to the file fails, you can implement it as such:

  if ! print "%s" "$@" >> ${file}; then
     error "Could not write to file: ..."
     return ${EXIT_ERROR}
  fi

That at least saves a stat call which would be quite good since the
functions writing the configuration files make quite a lot of use of
this function.

-Michael

On Mon, 2017-07-17 at 21:05 +0200, Jonatan Schlag wrote:
> This functions now:
> 	- return an error when the destination is a directory
> 	- creates the destination if the destination not exist
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
>  src/functions/functions.util | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/src/functions/functions.util
> b/src/functions/functions.util
> index 381208c..dd223f9 100644
> --- a/src/functions/functions.util
> +++ b/src/functions/functions.util
> @@ -179,12 +179,21 @@ fwrite() {
>  	assert isset file
>  	shift
>  
> -	if [ ! -w "${file}" ]; then
> -		log ERROR "${file}: No such file"
> +	if [ -d "${file}" ]; then
> +		log ERROR "${file} is a directory"
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	print "%s" "$@" >> ${file} 2>/dev/null
> +	if [ -f "${file}" ]; then
> +		if [ -w "${file}" ]; then
> +			print "%s" "$@" >> ${file} 2>/dev/null
> +		else
> +			log ERROR "${file} is not writeable"
> +			return ${EXIT_ERROR}
> +		fi
> +	else
> +		print "%s" "$@" >> ${file} 2>/dev/null
> +	fi
>  }
>  
>  make_parent_dir() {