Re: [PATCH v3 1/5] util: refactor fwrite

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