On Thu, Aug 12, 2021 at 11:13 PM Philipp Rudo prudo@redhat.com wrote:
Hi Kairui,
On Thu, 12 Aug 2021 13:47:29 +0800 Kairui Song kasong@redhat.com wrote:
Avoid duplicated echo / cut call, this make it easier to clean up code in later commits.
Signed-off-by: Kairui Song kasong@redhat.com
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%% *}"- echo "$_str"
I see multiple problems with this function
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...
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.