#3484 PR merged
: BCF-5527: [Rear] Space in the variable device_to_be_wiped_size_bytes¶
Labels: enhancement
, fixed / solved / done
BarysBarysenka opened issue at 2025-07-01 07:00:¶
On some platforms like Debian 12
command lsblk -dbnlpo SIZE /dev/sda2
can return the string that starts with space.
For example, " 1024".
As the result command
dd if=/dev/zero of=/dev/sda2 count= 1024 iflag=count_bytes
will be failed.
Strips non-digit characters with tr to return a clean numeric value.
jsmeix commented at 2025-07-01 08:04:¶
@BarysBarysenka
thank you for your clear issue report
and your proposed fix!
I would prefer more explicit code to fix it
which better tells what it is meant to do. cf.
https://github.com/rear/rear/wiki/Coding-Style
The appended plain | xargs
does not make it clear
why it is there and we use that nowhere in ReaR
so I suggest a better fix like
device_to_be_wiped_size_bytes="$( lsblk -dbnlpo SIZE $device_to_be_wiped | tr -d '[:space:]' )"
or
device_to_be_wiped_size_bytes="$( lsblk -dbnlpo SIZE $device_to_be_wiped | tr -c -d '[:digit:]' )"
which we use already at several places in ReaR.
From my current personal point of view
I would prefer ... | tr -c -d '[:digit:]'
because only a number is syntactically correct
for count=...
in the 'dd' command.
jsmeix commented at 2025-07-01 08:37:¶
I notice right now that I made an error in
layout/recreate/default/150_wipe_disks.sh
if ! test -b $device_to_be_wiped_size_byte ; then
LogPrintError "Skip wiping $device_to_be_wiped (no output for 'lsblk -dbnlpo SIZE $device_to_be_wiped' or failed)"
which is in two ways plain wrong because
there is no variable device_to_be_wiped_size_byte
and 'test -b' for device_to_be_wiped_size_bytes makes no sense.
Fortunately(?) plain 'test -b' without argument
(or with empty or blank argument) results true, cf.
https://github.com/rear/rear/wiki/Coding-Style#beware-of-the-emptiness
From my LogPrintError message I assume
what I wanted to do is something like
if ! is_positive_integer $device_to_be_wiped_size_bytes ; then
LogPrintError "..."
I will fix that after this pull request got merged
via a separated pull request.
jsmeix commented at 2025-07-01 08:49:¶
@rear/contributors
unless there are severe objections
I would like to merge it tomorrow afternoon.
jsmeix commented at 2025-07-01 08:56:¶
@gdha
I think issues like this are no bug (also no minor bug)
because the current code did nothing wrong because
the SIZE output of 'lsblk' can be expected
to be a plain non-negative integer number
without any additional non-digit characters
so I consider such issues as enhancements
to make ReaR robust against problematic things, cf.
https://github.com/rear/rear/wiki/Coding-Style#dirty-hacks-welcome
jsmeix commented at 2025-07-02 11:51:¶
https://github.com/rear/rear/pull/3484#issuecomment-3022670414
will be fixed via
https://github.com/rear/rear/pull/3487
Therein I use the same kind of syntax that is already used in
layout/recreate/default/150_wipe_disks.sh
# Wipe at the end of the device:
if ! test $device_to_be_wiped_size_bytes -gt $bytes_to_be_wiped ; then
i.e. in my case here a direct straightforward test via
if ! test $device_to_be_wiped_size_bytes -gt 0 ; then
instead of implementing RFC1925 item 6a
via a is_positive_integer()
call indirection
which is not needed here, cf. my above
https://github.com/rear/rear/pull/3484#issuecomment-3022448767
I would prefer more explicit code to fix it
which better tells what it is meant to do
[Export of Github issue for rear/rear.]