[PATCH v2] kdump-lib.sh: Check the output of blkid with sed instead of eval

[Tao Liu]

Previously the output of blkid is not checked. If the output
is empty, the eval will report the following error message:

    /lib/kdump/kdump-lib.sh: eval: line 925: syntax error near unexpected token `;'
    /lib/kdump/kdump-lib.sh: eval: line 925: `; echo $TYPE'

For example, we can observe such a failing when blkid is invoked
against a lvm thinpool block device:

    $ blkid -u filesystem,crypto -o export -- "/dev/block/253\:2"
    $ echo $?
    2
    $ udevadm info /dev/block/253\:2|grep S\:
    S: mapper/vg00-thinpoll_tmeta

In this patch, we will use sed instead of eval, to output the
fstype of block device if any.

Signed-off-by: Tao Liu <ltao(a)redhat.com&gt;
---
 kdump-lib.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh
index 3e912cc..4ed5035 100755
--- a/kdump-lib.sh
+++ b/kdump-lib.sh
@@ -885,7 +885,8 @@ get_luks_crypt_dev()
 
 	[[ -b /dev/block/$1 ]] || return 1
 
-	_type=$(eval "$(blkid -u filesystem,crypto -o export -- "/dev/block/$1");
echo \$TYPE")
+	_type=$(blkid -u filesystem,crypto -o export -- "/dev/block/$1" | \
+		sed -n -E "s/^TYPE=(.*)$/\1/p")
 	[[ $_type == "crypto_LUKS" ]] && echo "$1"
 
 	for _x in "/sys/dev/block/$1/slaves/"*; do
-- 
2.33.1

[Philipp Rudo]

Hi Tao,

On Wed, 16 Feb 2022 14:26:38 +0800
Tao Liu <ltao(a)redhat.com&gt; wrote:

>  Previously the output of blkid is not checked. If the output
>  is empty, the eval will report the following error message:
>  
>      /lib/kdump/kdump-lib.sh: eval: line 925: syntax error near unexpected token `;'
>      /lib/kdump/kdump-lib.sh: eval: line 925: `; echo $TYPE'
>  
>  For example, we can observe such a failing when blkid is invoked
>  against a lvm thinpool block device:
>  
>      $ blkid -u filesystem,crypto -o export -- "/dev/block/253\:2"
>      $ echo $?
>      2
>      $ udevadm info /dev/block/253\:2|grep S\:
>      S: mapper/vg00-thinpoll_tmeta
>  
>  In this patch, we will use sed instead of eval, to output the
>  fstype of block device if any.
>  
>  Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; 
Very good commit message!

Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt;

>  ---
>   kdump-lib.sh | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>  
>  diff --git a/kdump-lib.sh b/kdump-lib.sh
>  index 3e912cc..4ed5035 100755
>  --- a/kdump-lib.sh
>  +++ b/kdump-lib.sh
>  @@ -885,7 +885,8 @@ get_luks_crypt_dev()
>   
>   	[[ -b /dev/block/$1 ]] || return 1
>   
>  -	_type=$(eval "$(blkid -u filesystem,crypto -o export --
> "/dev/block/$1"); echo \$TYPE")
>  +	_type=$(blkid -u filesystem,crypto -o export -- "/dev/block/$1" | \
>  +		sed -n -E "s/^TYPE=(.*)$/\1/p")
>   	[[ $_type == "crypto_LUKS" ]] && echo "$1"
>   
>   	for _x in "/sys/dev/block/$1/slaves/"*; do

[Coiby Xu]

Patch merged, thanks to Tao and Philipp.

On Wed, Feb 16, 2022 at 02:46:51PM +0100, Philipp Rudo wrote:
> Hi Tao,
>
> On Wed, 16 Feb 2022 14:26:38 +0800
> Tao Liu <ltao(a)redhat.com&gt; wrote:
>
> > Previously the output of blkid is not checked. If the output
> > is empty, the eval will report the following error message:
> >
> >     /lib/kdump/kdump-lib.sh: eval: line 925: syntax error near unexpected token
> `;'
> >     /lib/kdump/kdump-lib.sh: eval: line 925: `; echo $TYPE'
> >
> > For example, we can observe such a failing when blkid is invoked
> > against a lvm thinpool block device:
> >
> >     $ blkid -u filesystem,crypto -o export -- "/dev/block/253\:2"
> >     $ echo $?
> >     2
> >     $ udevadm info /dev/block/253\:2|grep S\:
> >     S: mapper/vg00-thinpoll_tmeta
> >
> > In this patch, we will use sed instead of eval, to output the
> > fstype of block device if any.
> >
> > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt;
>
> Very good commit message!
>
> Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt;
>
> > ---
> >  kdump-lib.sh | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/kdump-lib.sh b/kdump-lib.sh
> > index 3e912cc..4ed5035 100755
> > --- a/kdump-lib.sh
> > +++ b/kdump-lib.sh
> > @@ -885,7 +885,8 @@ get_luks_crypt_dev()
> >
> >  	[[ -b /dev/block/$1 ]] || return 1
> >
> > -	_type=$(eval "$(blkid -u filesystem,crypto -o export --
> "/dev/block/$1"); echo \$TYPE")
> > +	_type=$(blkid -u filesystem,crypto -o export -- "/dev/block/$1" | \
> > +		sed -n -E "s/^TYPE=(.*)$/\1/p")
> >  	[[ $_type == "crypto_LUKS" ]] && echo "$1"
> >
> >  	for _x in "/sys/dev/block/$1/slaves/"*; do
>  
-- 
Best regards,
Coiby

[Tao Liu]

Hi Coiby and Philipp,

Thanks for the suggestions, review and merge, I really appreciated it!

Thanks,
Tao Liu

On Thu, Feb 17, 2022 at 3:50 PM Coiby Xu <coxu(a)redhat.com&gt; wrote:
>  Patch merged, thanks to Tao and Philipp.
>
>  On Wed, Feb 16, 2022 at 02:46:51PM +0100, Philipp Rudo wrote:
>  >Hi Tao,
>  >
>  >On Wed, 16 Feb 2022 14:26:38 +0800
>  >Tao Liu <ltao(a)redhat.com&gt; wrote:
>  >
>  >> Previously the output of blkid is not checked. If the output
>  >> is empty, the eval will report the following error message:
>  >>
>  >>     /lib/kdump/kdump-lib.sh: eval: line 925: syntax error near unexpected token
> `;'
>  >>     /lib/kdump/kdump-lib.sh: eval: line 925: `; echo $TYPE'
>  >>
>  >> For example, we can observe such a failing when blkid is invoked
>  >> against a lvm thinpool block device:
>  >>
>  >>     $ blkid -u filesystem,crypto -o export -- "/dev/block/253\:2"
>  >>     $ echo $?
>  >>     2
>  >>     $ udevadm info /dev/block/253\:2|grep S\:
>  >>     S: mapper/vg00-thinpoll_tmeta
>  >>
>  >> In this patch, we will use sed instead of eval, to output the
>  >> fstype of block device if any.
>  >>
>  >> Signed-off-by: Tao Liu <ltao(a)redhat.com&gt;
>  >
>  >Very good commit message!
>  >
>  >Reviewed-by: Philipp Rudo <prudo(a)redhat.com&gt;
>  >
>  >> ---
>  >>  kdump-lib.sh | 3 ++-
>  >>  1 file changed, 2 insertions(+), 1 deletion(-)
>  >>
>  >> diff --git a/kdump-lib.sh b/kdump-lib.sh
>  >> index 3e912cc..4ed5035 100755
>  >> --- a/kdump-lib.sh
>  >> +++ b/kdump-lib.sh
>  >> @@ -885,7 +885,8 @@ get_luks_crypt_dev()
>  >>
>  >>      [[ -b /dev/block/$1 ]] || return 1
>  >>
>  >> -    _type=$(eval "$(blkid -u filesystem,crypto -o export --
> "/dev/block/$1"); echo \$TYPE")
>  >> +    _type=$(blkid -u filesystem,crypto -o export -- "/dev/block/$1" |
> \
>  >> +            sed -n -E "s/^TYPE=(.*)$/\1/p")
>  >>      [[ $_type == "crypto_LUKS" ]] && echo "$1"
>  >>
>  >>      for _x in "/sys/dev/block/$1/slaves/"*; do
>  >
>
>  --
>  Best regards,
>  Coiby
>