[LTP] [PATCH v3 2/6] syscalls/readahead02: Convert to newlib and cleanup

Amir Goldstein amir73il@gmail.com
Wed Nov 28 17:50:49 CET 2018


...
> >  static int check_ret(long expected_ret)
> >  {
> > -     if (expected_ret == TEST_RETURN) {
> > -             tst_resm(TPASS, "expected ret success - "
> > -                      "returned value = %ld", TEST_RETURN);
> > +     if (expected_ret == TST_RET) {
> > +             tst_res(TPASS, "expected ret success - "
> > +                     "returned value = %ld", TST_RET);
> >               return 0;
> >       }
> > -     tst_resm(TFAIL | TTERRNO, "unexpected failure - "
> > -              "returned value = %ld, expected: %ld",
> > -              TEST_RETURN, expected_ret);
> > +     tst_res(TFAIL | TTERRNO, "unexpected failure - "
> > +             "returned value = %ld, expected: %ld",
> > +             TST_RET, expected_ret);
> >       return 1;
> >  }
>
> I would be happier if we got rid of this function.
>

Done. (In next patch)

> Maybe we should just move the loop from read_file() to a separate
> function do_readahead() then the indentation would be one level less
> there and the code could be put directly there.
>
...

> >       if (gettimeofday(&now, NULL) == -1)
> > -             tst_brkm(TBROK | TERRNO, cleanup, "gettimeofday failed");
> > +             tst_brk(TBROK | TERRNO, "gettimeofday failed");
>
> We should really switch to posix timers here, using wall clock for
> elapsed time measurements in not a good idea for several reasons.
>
> And we even do have nice API for this, see:
>
> https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#2221-measuring-elapsed-time-and-helper-functions
>

Nice! done in a separate patch.

...

> > -             sprintf(testfile, MNTPOINT"/testfile");
> > -     }
> > +     sprintf(testfile, "%s/testfile", mntpoint);
>
> This just a static string defined on the top of the test.
>

It is going to be modified per test case in patch
"test readahead() on an overlayfs file"

v4 posted.

Thanks,
Amir.


More information about the ltp mailing list