Re: [PATCH 13/47] dracut-module-setup.sh: rework kdump_get_ip_route_field

On Thu, Aug 12, 2021 at 11:13 PM Philipp Rudo <prudo(a)redhat.com&gt; wrote: > > Hi Kairui, > > On Thu, 12 Aug 2021 13:47:29 +0800 > Kairui Song <kasong(a)redhat.com&gt; wrote: > > > Avoid duplicated echo / cut call, this make it easier to clean up code > > in later commits. > > > > Signed-off-by: Kairui Song <kasong(a)redhat.com&gt; > > --- > > dracut-module-setup.sh | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh > > index 53abdc75..768026d3 100755 > > --- a/dracut-module-setup.sh > > +++ b/dracut-module-setup.sh > > @@ -523,9 +523,14 @@ kdump_get_ip_route() > > > > kdump_get_ip_route_field() > > { > > - if `echo $1 | grep -q $2`; then > > - echo ${1##*$2} | cut -d ' ' -f1 > > + local _str=$1 > > + > > + if [[ "$_str" == *" $2 "* ]]; then > > + _str="${_str##*$2 }" > > + _str="${_str%% *}" > > fi > > + > > + echo "$_str" > > I see multiple problems with this function > > 1. both the old and new version have a problem with partial matches, > e.g. consider > > $1="something match what_i_want partial_match not_this" > ^ > this space is important, > otherwise the new version > does not work > $2="match" > > both, the old and new version would return 'not_this'. On one hand > the newer version is a little better as it requires a full match to > enter the if-block on the other it now fails if the search term is > at the beginning of $1 and thus not proceeded by a space... > > 2. the new version returns the full $1 if no match was found. I would > expect it to return an empty string instead. > > I don't think the problems above cause any bug in today's uses > nevertheless I think the function should be hardened. The easiest fix > is probably to use sed again, i.e. > echo $1 | sed -n -e "s/\<$2\>\s\+\(\S\+\)\s.*$/\1/p" > should work > > Thanks > Philipp > Thanks for the review! How about: kdump_get_ip_route_field() { local _str=$1 if [[ "$_str" == *" $2 "* ]]; then _str="${_str##* $2 }" # Note the space before $2 above _str="${_str%% *}" echo "$_str" fi } This should have the same effect as using `sed -n -e "s/.*\s\+\<$2\>\s\+\(\S\+\)\s.*$/\1/p"` and bash string substitution is much faster.