[LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation

[Murphy Zhou]

On Mon, Jun 5, 2023 at 5:57 PM Li Wang <liwang@redhat.com> wrote:
>
>
>
> On Mon, Jun 5, 2023 at 4:40 PM Martin Doucha <mdoucha@suse.cz> wrote:
>>
>> Hi,
>>
>> On 04. 06. 23 11:51, Murphy Zhou wrote:
>> > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
>> > ---
>> >   lib/safe_macros.c | 17 +++++++++++++++++
>> >   1 file changed, 17 insertions(+)
>> >
>> > diff --git a/lib/safe_macros.c b/lib/safe_macros.c
>> > index af6dd0716..a66285a0e 100644
>> > --- a/lib/safe_macros.c
>> > +++ b/lib/safe_macros.c
>> > @@ -898,7 +898,24 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
>> >              const void *data)
>> >   {
>> >       int rval = -1;
>> > +     char cdir[PATH_MAX], mpath[PATH_MAX];
>> >
>> > +     if (!getcwd(cdir, PATH_MAX)) {
>> > +             tst_resm(TWARN | TERRNO, "Failed to find current directory");
>> > +             return 0;
>>
>> You return success when nothing was mounted. That is clearly wrong.
>> Either call tst_brkm_(... TBROK ...) if the failure is so serious that
>> the test cannot continue, or don't return at all.
>>
>> > +     }
>> > +
>> > +     rval = snprintf(mpath, PATH_MAX, "%s/%s", cdir, target);
>>
>> The C library has a function for resolving paths: realpath(). Please use
>> that.
>
>
>
> +1, good point. realpath() is more convenient, we don't
> need to additionally copy string to the absolute path.
>
> @Murphy, ignore my previous comments plz.

All very good catches! Thanks for reviewing!

Cooking V2.

Murphy
>
>
>>
>>
>> > +     if (rval < 0 || rval >= PATH_MAX) {
>> > +             tst_resm(TWARN | TERRNO,
>> > +                      "snprintf() should have returned %d instead of %d",
>> > +                      PATH_MAX, rval);
>> > +             return 0;
>>
>> Returning here is also wrong.
>>
>> > +     }
>> > +
>> > +     tst_resm_(file, lineno, TINFO,
>> > +             "Mounting %s to %s fstyp=%s flags=%x",
>> > +             source, mpath, filesystemtype, mountflags);
>> >       /*
>> >        * Don't try using the kernel's NTFS driver when mounting NTFS, since
>> >        * the kernel's NTFS driver doesn't have proper write support.
>>
>> --
>> Martin Doucha   mdoucha@suse.cz
>> SW Quality Engineer
>> SUSE LINUX, s.r.o.
>> CORSO IIa
>> Krizikova 148/34
>> 186 00 Prague 8
>> Czech Republic
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>
>
>
> --
> Regards,
> Li Wang