#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.]