Hi Kairui,
On Mon, 13 Sep 2021 04:26:35 +0800 Kairui Song kasong@redhat.com wrote:
On Fri, Sep 3, 2021 at 8:15 PM Philipp Rudo prudo@redhat.com wrote:
Hi Kairui,
On Thu, 19 Aug 2021 19:39:04 +0800 Kairui Song kasong@redhat.com wrote:
Signed-off-by: Kairui Song kasong@redhat.com
kdumpctl | 47 +++++++++++++++++++++-------------------------- 1 file changed, 21 insertions(+), 26 deletions(-)
diff --git a/kdumpctl b/kdumpctl index db3a3015..50e92cb4 100755 --- a/kdumpctl +++ b/kdumpctl @@ -339,8 +339,8 @@ check_files_modified() #also rebuild when Pacemaker cluster conf is changed and fence kdump is enabled. modified_files=$(get_pcs_cluster_modified_files)
EXTRA_BINS=`grep ^kdump_post $KDUMP_CONFIG_FILE | cut -d\ -f2`CHECK_FILES=`grep ^kdump_pre $KDUMP_CONFIG_FILE | cut -d\ -f2`
EXTRA_BINS=$(kdump_get_conf_val kdump_post)CHECK_FILES=$(kdump_get_conf_val kdump_pre) HOOKS="/etc/kdump/post.d/ /etc/kdump/pre.d/" if [ -d /etc/kdump/post.d ]; then for file in /etc/kdump/post.d/*; do@@ -357,17 +357,17 @@ check_files_modified() done fi HOOKS="$HOOKS $POST_FILES $PRE_FILES"
CORE_COLLECTOR=`grep ^core_collector $KDUMP_CONFIG_FILE | cut -d\ -f2`
CORE_COLLECTOR=$(kdump_get_conf_val core_collector | cut -d ' ' -f 1)keeping this cut here still makes you vulnerable to some of the problems you solved by introducing kdump_get_conf_val, e.g. when the core_collector is followed by a tab instead of a space. As this is a bash only file you could make CORE_COLLECTOR an array and then use the first element as command.
I think simply use a awk '{print $1}' should be enough just for this case.
yes, using awk should do the trick here. But a quick search gave a few other locations with similar usage of cut. So not sure if it's better to fix it now or wait until you fix the other locations.
BTW in general this function could do with a do over, e.g. files and modified_fiels could be made an array as well. Furthermore some variables seem unnecessary to me and the others probably should be made local. Anyway as the series is already pretty long I think both can wait for the next cleanup series.
Great suggestion, actually I found a lot of extra problems during making this series and recorded some on my local to-do list. Let's fix it later, currently, let me just update this series to avoid introducing any new issue.
fully agree. I just wanted to write it down so I don't forget it myself.
Thanks Philipp