Re: [LTP] [PATCH v2 2/3] set_dev_loop_path: Refactor set_dev_path and check return value

[Petr Vorel <pvorel@suse.cz>]

> On Thu, Mar 23, 2023 at 1:52 AM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Edward,

> > > tst_find_free_loopdev does not check the return value of set_dev_path
> > > and will return the last attempted path even if it does not pass a stat
> > > check.  set_dev_path also has a return value that is not consistent with
> > > the other functions in this file.

> > This change and change of return is a bit burden in loop rename changes.
> > I'm ok it's in single patch, but it'd be more readable if it were separate.

> Not a problem, I will split it.

+1

Also, please rebase (I pushed some changes) which touch files you also modify.

> > set_dev_path() is going to be called only if non-NULL path
> > (see include/tst_device.h). I haven't found a test which uses it this way,
> > but shouldn't it be checking path, instead of !path?

> >         if (path && set_dev_loop_path(rc, path, path_len) != 0)

> > Kind regards,
> > Petr

> Oops, I missed that in the comment and thought a NULL path should be
> checked as an error.  You are right, I will change it.  Also, I wasn't
> sure if I should be explicit with the "!= 0".

I guess we are quite ok with just "!". We try to be precise at syscalls testing
(to check if the return value on error is exactly -1, not just < 0, but with
normal non-testing code like this in tst_kernel.c it's not needed.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp