:: Re: [DNG] vdev status update in dae…
Góra strony
Delete this message
Reply to this message
Autor: wirelessduck
Data:  
Dla: dng, aitor
Temat: Re: [DNG] vdev status update in daedalus


> On 10 Dec 2023, at 23:38, Olaf Meeuwissen via Dng <dng@???> wrote:
>
> Hi,
>
> This is in addition to the advice Ludovic and Ralph have already
> provided.
>
> aitor <aitor_czr@???> writes:
>
>> [...]
>> The origin of the bug was in the use of an asterisk in the regex `fd*` used in the helper `disk.sh` (line 57):
>> https://github.com/jcnelson/vdev/blob/master/vdevd/helpers/LINUX/disk.sh:
>> in order to skip inappropriate block devices.
>> The piece of code responsible for that is:
>>   # skip inappropriate block devices
>>   if [ -n "$(echo "$VDEV_PATH" | /bin/egrep "fd*|mtd*|nbd*|gnbd*|btibm*|dm-*|md*|zram*|mmcblk[0-9]*rpmb")" ]; then
>>      return 0
>>   fi
>> Indeed:
>> $ echo "sdf" | egrep "fd*"
>> sdf

>>
>> Which syntax would you suggest for this purpose?
>
> Based on the above code and assuming /bin/sh as your interpreter, it
> looks like you wanted to use shell globs. In that case you can remove
> the dependency on egrep (aka grep -E) with
>
>  case "$VDEV_PATH" in
>    fd*|mtd*|nbd*|gnbd*|btibm*|dm-*|md*|zram*|mmcblk[0-9]*rpmb) return 0;;
>  esac

>
> As a matter of style, you may want to add a default action before the
> esac like
>
>    *) : ;;
>  esac

>
> If you continue using grep, I would also remove the hard-coded path. If
> you have concerns about the PATH value in your script, set that instead.
> That is, use something like
>
> PATH=/sbin:/bin:/usr/sbin:/usr/bin
>
> to make sure you get the system's grep.
>
> BTW, according to the upstream NEWS file, /usr/share/doc/grep/NEWS.gz,
> egrep (and fgrep) have been deprecated in 2.5.3 (2007) and marked
> obsolescent in 3.8 (2022-09-02). Based on that, replace egrep with
> `grep -E` sooner rather than later.
>
> The -E option signals *extended* regular expressions. These differ from
> shell globs in ways which seem to have tripped you up in disk.sh. Check
> the grep manual page for the details.
>
> Hope this helps,
> --
> Olaf Meeuwissen
> _______________________________________________


Also I would suggest running the shell scripts through `shellcheck`. A quick test on the web version at shellcheck.net produced an assortment of linting errors in disk.sh. I imagine the other vdev shell scripts would have similar linting issues.

Tom