From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: network@lists.ipfire.org
Subject: Re: [PATCH v3 1/5] util: refactor fwrite
Date: Mon, 17 Jul 2017 16:30:34 -0400
Message-ID: <1500323434.2548.14.camel@ipfire.org>
In-Reply-To: <1500318318-18852-1-git-send-email-jonatan.schlag@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1347534415804430950=="
List-Id: <network.lists.ipfire.org>

--===============1347534415804430950==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

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(a)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() {

--===============1347534415804430950==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KVmVyc2lvbjogR251UEcgdjIKCmlRSWNCQUFC
Q2dBR0JRSlpiUjVxQUFvSkVJQjU4UDl2a0FrSFdVd1ArUUVDY2FESmVGWHZXY2FZeldueWxPOCsK
cGRPdHZNZGRGN2VOTzE2eUs5SjM1b2U4UUtOZitJdENsdFQ3Y3c4RlNZMWUvdGRHNHNrNWx3YVdw
d0lUVVBLNQpzeUVOVVlpbVlRd0xWK0RXNURUbW9jNHpUL1RoNTVETEdsK3lmdlRxd1czbk1EUGRZ
cVBoR0hWVnZORzFrYWtNCnpsVkRIa3QzRFU0dlR0a3lPM1c5bDdKT1BIaG1jMHVHTlRkcGNCV043
bTZaS2FjOFNaTEhLTmpHQ3FjQysrdy8KeTFYU1RpbU5KeEJKZ1FRT29pdTBGVXZkdGY1YWdDci8x
eFIrN2FMcUp0TFVXNFZDWk1sR2pidnhXVFJUQWRQdQowck9sdnpVbFpzdWdhV2Y4cDJjbTkxYkxi
QU82YlB6N29JbDl4ZTNSWG5DSWlBUmhDNmI5UWx6NXBnazVXb2xaCnYrYUJaaDhaWEU4ZUd2RGFN
UHd0ZFFQR2I1cXBuS2tUenE3ZHRBUkpZMzBRTlg4WDcrTkdIVm5yd2p6c2RaOHUKMzV6K0N0dVNo
eGxDYkJIakdKSkJsakxQNnU5VlhqS2lMSkRwZFc0L3VmUEpNeGNRRXZGN2ViNmRib1NyVUdjcgp3
RXZKOUt0WTcwR1crSnBaWmMxcmlqWlhwb29jRE5HeEZSL29rd25PcHEraTlZVFVScFgzc1lYdUVW
RUplRlhlClZOcW9pQ0pVUS9QYjY5YUt5QnpPU1ZjazRpU3NBTzh4amNTaWxMVG1LM2ptcnlsUzk4
N2pzUU80VHpyRW9McjMKSlZQNlBGWElXMDFEWWJtYkpZd2FmaU93dGdMSlF6eTR5V05ZRWl0U1JD
czFET0dOeVhZU3pvU0ZtcFc0V0ZxUgpPSm52cTZqQ1ZZaWJmV1RlUzFjSwo9ZzJhbgotLS0tLUVO
RCBQR1AgU0lHTkFUVVJFLS0tLS0K

--===============1347534415804430950==--