[v3,2/5] util: add new function copy
Message ID | 1500318318-18852-2-git-send-email-jonatan.schlag@ipfire.org |
---|---|
State | Superseded |
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 475276159A for <patchwork@ipfire.org>; Mon, 17 Jul 2017 21:05:26 +0200 (CEST) Received: from mail01.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id 13CE52596; Mon, 17 Jul 2017 21:05:26 +0200 (CEST) Received: from ipfire.localdomain (dslb-088-073-208-102.088.073.pools.vodafone-ip.de [88.73.208.102]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id E7D0827CA; Mon, 17 Jul 2017 21:05:23 +0200 (CEST) From: Jonatan Schlag <jonatan.schlag@ipfire.org> To: network@lists.ipfire.org Subject: [PATCH v3 2/5] util: add new function copy Date: Mon, 17 Jul 2017 21:05:15 +0200 Message-Id: <1500318318-18852-2-git-send-email-jonatan.schlag@ipfire.org> X-Mailer: git-send-email 2.6.3 In-Reply-To: <1500318318-18852-1-git-send-email-jonatan.schlag@ipfire.org> References: <1500318318-18852-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 18, 2017, 5:05 a.m. UTC
Just one short note, I could also insted of removing the destintaion if she exists,
allow fwrite to overired files. I did not do this because it would first maybe break code
when the function fwrite is used. Second it is easier to remove the destintaion and
so allow fwrite writing the content to a plain file, then changing the function so that fwrite override files.
It is easy to remove a file bevore but appending to a file with a function that overried files is not possible.
So I think it is the best to keep this feature of fwrite and when somebody wants to override a file he has to remove it before.
Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
src/functions/functions.util | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
Comments
Hi, typos, typos, typos. Please go through the comments again. It really is hard to read. On Mon, 2017-07-17 at 21:05 +0200, Jonatan Schlag wrote: > Just one short note, I could also insted of removing the destintaion > if she exists, > allow fwrite to overired files. I did not do this because it would > first maybe break code > when the function fwrite is used. Second it is easier to remove the > destintaion and > so allow fwrite writing the content to a plain file, then changing > the function so that fwrite override files. > It is easy to remove a file bevore but appending to a file with a > function that overried files is not possible. > So I think it is the best to keep this feature of fwrite and when > somebody wants to override a file he has to remove it before. > > Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org> > --- > src/functions/functions.util | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/src/functions/functions.util > b/src/functions/functions.util > index dd223f9..efcdfb3 100644 > --- a/src/functions/functions.util > +++ b/src/functions/functions.util > @@ -761,3 +761,29 @@ hex2dec() { > dec2hex() { > printf "%02x\n" "${1}" > } > + > +copy() { > + # This function just copy config files > + assert [ $# -eq 2 ] > + local src=${1} > + local dst=${2} Empty line after assert() > + > + # Wo do the decleration and the initialisation in two lines > to get the return code of fread > + local data > + data=$(fread "${src}") > + if [ ! $? -eq 0 ]; then > + log ERROR "Could not read data from ${src}" > + return ${EXIT_ERROR} > + fi I am not a fan of this, but I am sure there is not a more elegant solution. > + > + # If the file exist we will overwrite it > + # fwrite would just append the contentet to the end of the > file > + if [ -f ${dst} ]; then > + rm -f ${dst} > + fi > + > + if ! fwrite "${dst}" "${data}"; then > + log ERROR "Could not write data to ${dst}" > + return ${EXIT_ERROR} > + fi > +} Here is where I don't really follow. We never want to append something to an already existing file (maybe fwrite shouldn't have been implemented as it was). It's probably not good style to call "rm -f" from this script. I would prefer unlink. But before getting to that: Isn't it easiest just to implement it like this: fread ${src} > ${dst} This will just read the content and stream it into ${dst}. If you check if $dst is a directory and similar stuff before you should be fine. Way easier and should be able to copy even large files. -Michael