* [LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation
@ 2023-06-04 9:51 Murphy Zhou
2023-06-05 6:30 ` Li Wang
2023-06-05 8:39 ` Martin Doucha
0 siblings, 2 replies; 5+ messages in thread
From: Murphy Zhou @ 2023-06-04 9:51 UTC (permalink / raw)
To: ltp
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;
+ }
+
+ rval = snprintf(mpath, PATH_MAX, "%s/%s", cdir, target);
+ if (rval < 0 || rval >= PATH_MAX) {
+ tst_resm(TWARN | TERRNO,
+ "snprintf() should have returned %d instead of %d",
+ PATH_MAX, rval);
+ return 0;
+ }
+
+ 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.
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation
2023-06-04 9:51 [LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation Murphy Zhou
@ 2023-06-05 6:30 ` Li Wang
2023-06-05 8:39 ` Martin Doucha
1 sibling, 0 replies; 5+ messages in thread
From: Li Wang @ 2023-06-05 6:30 UTC (permalink / raw)
To: Murphy Zhou; +Cc: ltp
Hi Murphy,
This is helpful for knowing the mount info.
But as SAFE_MOUNT is not only used for relative path but
mount absolute path, so we'd better determine what the '*target'
path type.
How about changing to below:
if (target[0] == '/') {
strncpy(mpath, target, PATH_MAX-1);
mpath[PATH_MAX-1] = '\0';
} else {
.... your relative path code ...
}
tst_resm_(file, lineno, TINFO,
"Mounting %s to %s fstyp=%s flags=%x",
source, mpath, filesystemtype, mountflags);
On Sun, Jun 4, 2023 at 5:51 PM Murphy Zhou <jencce.kernel@gmail.com> 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;
> + }
> +
> + rval = snprintf(mpath, PATH_MAX, "%s/%s", cdir, target);
> + if (rval < 0 || rval >= PATH_MAX) {
> + tst_resm(TWARN | TERRNO,
> + "snprintf() should have returned %d instead of
> %d",
> + PATH_MAX, rval);
> + return 0;
> + }
> +
+ 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.
> --
> 2.31.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
--
Regards,
Li Wang
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation
2023-06-04 9:51 [LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation Murphy Zhou
2023-06-05 6:30 ` Li Wang
@ 2023-06-05 8:39 ` Martin Doucha
2023-06-05 9:56 ` Li Wang
1 sibling, 1 reply; 5+ messages in thread
From: Martin Doucha @ 2023-06-05 8:39 UTC (permalink / raw)
To: Murphy Zhou, ltp
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.
> + 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation
2023-06-05 8:39 ` Martin Doucha
@ 2023-06-05 9:56 ` Li Wang
2023-06-06 7:08 ` Murphy Zhou
0 siblings, 1 reply; 5+ messages in thread
From: Li Wang @ 2023-06-05 9:56 UTC (permalink / raw)
To: Martin Doucha; +Cc: ltp
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.
>
> > + 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
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation
2023-06-05 9:56 ` Li Wang
@ 2023-06-06 7:08 ` Murphy Zhou
0 siblings, 0 replies; 5+ messages in thread
From: Murphy Zhou @ 2023-06-06 7:08 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
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
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-06 7:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-04 9:51 [LTP] [PATCH] SAFE_MOUNT: print debug info about mounting operation Murphy Zhou
2023-06-05 6:30 ` Li Wang
2023-06-05 8:39 ` Martin Doucha
2023-06-05 9:56 ` Li Wang
2023-06-06 7:08 ` Murphy Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox