Re: [PATCH] generic/{175,297,298}: fix use of uninitialized var

["Darrick J. Wong" <djwong@kernel.org>]

On Sun, Aug 13, 2023 at 09:53:30PM +0300, Amir Goldstein wrote:
> On Sun, Aug 13, 2023 at 6:35 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Sat, Aug 12, 2023 at 06:15:00PM +0300, Amir Goldstein wrote:
> > > Not sure how those tests pass in regression tests.
> > > Probably truncate silently fails and is not critical to the test.
> > >
> > > in kdevops I get errors like:
> > > /data/fstests-install/xfstests/tests/generic/298: line 45: /dev/loop12):
> > >            syntax error: operand expected (error token is "/dev/loop12)")
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >  tests/generic/175 | 2 +-
> > >  tests/generic/297 | 2 +-
> > >  tests/generic/298 | 2 +-
> > >  3 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tests/generic/175 b/tests/generic/175
> > > index 07af2199..14825a39 100755
> > > --- a/tests/generic/175
> > > +++ b/tests/generic/175
> > > @@ -33,7 +33,7 @@ _pwrite_byte 0x61 0 $blksz "$testdir/file1" >> "$seqres.full"
> > >
> > >  fnr=19
> > >  echo "Create extents"
> > > -truncate -s $(( (2 ** i) * blksz)) "$testdir/file1"
> > > +truncate -s $(( (2 ** (fnr + 1)) * blksz)) "$testdir/file1"
> > >  for i in $(seq 0 $fnr); do
> >
> > Hmm.  Frankly I don't remember why I put those truncate calls in there.
> 
> You specifically added truncate in commit
> ddf6ff2f reflink: ensure that we can handle reflinking a lot of extents
> and then it was copied to the two new tests.
> maybe this will give you a hint?

Alas, no.  Total etch-a-sketch over here. :(

AFAICT the tests work just fine without the truncate calls, so let's
remove them.

--D

> > Does the test still work if you remove them entirely?  AFAICT the test
> > writes a single block's worth of data to the file, then truncates the
> > file size to one block.  Which I think is pointless.
> >
> > >       echo " ++ Reflink size $i, $((2 ** i)) blocks" >> "$seqres.full"
> > >       n=$(( (2 ** i) * blksz))
> > > diff --git a/tests/generic/297 b/tests/generic/297
> > > index 6bdc3e1c..1fc48671 100755
> > > --- a/tests/generic/297
> > > +++ b/tests/generic/297
> > > @@ -42,7 +42,7 @@ _pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
> > >  fnr=26               # 2^26 reflink extents should be enough to find a slow op?
> > >  timeout=8    # guarantee a good long run...
> > >  echo "Find a reflink size that takes a long time"
> > > -truncate -s $(( (2 ** i) * blksz)) $testdir/file1
> > > +truncate -s $(( (2 ** (fnr + 1)) * blksz)) $testdir/file1
> > >  for i in $(seq 0 $fnr); do
> > >       echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> > >       n=$(( (2 ** i) * blksz))
> >
> > The loop control logic could be converted to:
> >
> > echo "Find a reflink size that takes a long time"
> > deadline="$(( $(date +%s) + timeout ))"
> > for ((i = 0, now = $(date +%s); i < fnr && now < deadline; i++, now = $(date +%s))); do
> >         echo " ++ Reflink size $i, $((2 ** i)) blocks" >> $seqres.full
> >         n=$(( (2 ** i) * blksz))
> >         $XFS_IO_PROG -f -c "reflink $testdir/file1 0 $n $n" $testdir/file1 >> $seqres.full 2>&1
> > done
> >
> > (also in 298)
> >
> 
> Whatever works is fine by me.
> Feel free to keep my commit message or drop it.
> 
> Thanks,
> Amir.