From: Kris Van Hees <kris.van.hees@oracle.com>
To: Eugene Loh <eugene.loh@oracle.com>
Cc: Kris Van Hees <kris.van.hees@oracle.com>,
dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values
Date: Thu, 30 Jan 2025 18:03:46 -0500 [thread overview]
Message-ID: <Z5wFUtaCwaxo1LpL@oracle.com> (raw)
In-Reply-To: <4b5002a1-1c42-9ab1-92f8-3cbb29368de3@oracle.com>
On Thu, Jan 30, 2025 at 05:33:45PM -0500, Eugene Loh wrote:
> My main question is how it is better to provide incorrect results than it is
> not to have implemented this functionality? How about just waiting until we
> implement this functionality? Isn't it better that a user is told that we
> haven't implemented this stuff rather than give them results that are
> incorrect?
That is always a balance. Ideally, we should have tests for each independent
member of each translator, so that we can do more fine grained testing. But
it is not sensible that we do not end up testing any translator members because
some are not implemented, whereas providing a default value for them (which we
e.g. also do for members in the io translators). That is the precedent I am
depending on here.
> It is odd that XFAIL is being lifted when we are producing incorrect output,
> but I suppose that is the burden of some other test/s. So, okay.
Yes, we need more fine grained tests.
> Meanwhile, XFAIL is being lifted for
> test/unittest/builtinvar/tst.psinfo-bug21974606.d
> even though that change should probably actually appear in an earlier patch,
> "procfs: implement d_execargs() for pr_psargs translator support", where it
> starts to XPASS since it depends only on ps_args and not on any of the
> fields handled in the current patch.
Hm, yes, forgot to move that one. Will do.
>
> On 1/28/25 01:31, Kris Van Hees wrote:
> > The pr_argc, pr_argv, and pr_envp fields in psinfo are not implemented
> > yet, so it makes sense to set them to 0 rather than not providing any
> > translator for them.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> > dlibs/aarch64/5.11/procfs.d | 8 +++-----
> > dlibs/aarch64/5.12/procfs.d | 8 +++-----
> > dlibs/aarch64/5.14/procfs.d | 8 +++-----
> > dlibs/aarch64/5.16/procfs.d | 8 +++-----
> > dlibs/aarch64/5.2/procfs.d | 8 +++-----
> > dlibs/aarch64/5.6/procfs.d | 8 +++-----
> > dlibs/aarch64/6.1/procfs.d | 8 +++-----
> > dlibs/aarch64/6.10/procfs.d | 8 +++-----
> > dlibs/x86_64/5.11/procfs.d | 8 +++-----
> > dlibs/x86_64/5.12/procfs.d | 8 +++-----
> > dlibs/x86_64/5.14/procfs.d | 8 +++-----
> > dlibs/x86_64/5.16/procfs.d | 8 +++-----
> > dlibs/x86_64/5.2/procfs.d | 8 +++-----
> > dlibs/x86_64/5.6/procfs.d | 8 +++-----
> > dlibs/x86_64/6.1/procfs.d | 8 +++-----
> > dlibs/x86_64/6.10/procfs.d | 8 +++-----
> > libdtrace/procfs.d.in | 8 +++-----
> > test/unittest/builtinvar/tst.psinfo-bug21974606.d | 1 -
> > test/unittest/builtinvar/tst.psinfo-bug22561297.d | 4 +---
> > test/unittest/builtinvar/tst.psinfo1.d | 1 -
> > 20 files changed, 52 insertions(+), 90 deletions(-)
> >
> > diff --git a/dlibs/aarch64/5.11/procfs.d b/dlibs/aarch64/5.11/procfs.d
> > index 9c06fe1f..52b2bbe2 100644
> > --- a/dlibs/aarch64/5.11/procfs.d
> > +++ b/dlibs/aarch64/5.11/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.12/procfs.d b/dlibs/aarch64/5.12/procfs.d
> > index 9c06fe1f..52b2bbe2 100644
> > --- a/dlibs/aarch64/5.12/procfs.d
> > +++ b/dlibs/aarch64/5.12/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.14/procfs.d b/dlibs/aarch64/5.14/procfs.d
> > index 2824d137..8c05e299 100644
> > --- a/dlibs/aarch64/5.14/procfs.d
> > +++ b/dlibs/aarch64/5.14/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.16/procfs.d b/dlibs/aarch64/5.16/procfs.d
> > index daf30745..e52ab29a 100644
> > --- a/dlibs/aarch64/5.16/procfs.d
> > +++ b/dlibs/aarch64/5.16/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.2/procfs.d b/dlibs/aarch64/5.2/procfs.d
> > index 3594e5e9..4a95dfd1 100644
> > --- a/dlibs/aarch64/5.2/procfs.d
> > +++ b/dlibs/aarch64/5.2/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.6/procfs.d b/dlibs/aarch64/5.6/procfs.d
> > index 9c06fe1f..52b2bbe2 100644
> > --- a/dlibs/aarch64/5.6/procfs.d
> > +++ b/dlibs/aarch64/5.6/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/6.1/procfs.d b/dlibs/aarch64/6.1/procfs.d
> > index 2d52f079..4881aa5b 100644
> > --- a/dlibs/aarch64/6.1/procfs.d
> > +++ b/dlibs/aarch64/6.1/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/6.10/procfs.d b/dlibs/aarch64/6.10/procfs.d
> > index 2d52f079..4881aa5b 100644
> > --- a/dlibs/aarch64/6.10/procfs.d
> > +++ b/dlibs/aarch64/6.10/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.11/procfs.d b/dlibs/x86_64/5.11/procfs.d
> > index 7679db2e..96e55dc1 100644
> > --- a/dlibs/x86_64/5.11/procfs.d
> > +++ b/dlibs/x86_64/5.11/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.12/procfs.d b/dlibs/x86_64/5.12/procfs.d
> > index 7679db2e..96e55dc1 100644
> > --- a/dlibs/x86_64/5.12/procfs.d
> > +++ b/dlibs/x86_64/5.12/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.14/procfs.d b/dlibs/x86_64/5.14/procfs.d
> > index 3a348ebc..8dbf3c01 100644
> > --- a/dlibs/x86_64/5.14/procfs.d
> > +++ b/dlibs/x86_64/5.14/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.16/procfs.d b/dlibs/x86_64/5.16/procfs.d
> > index daf30745..e52ab29a 100644
> > --- a/dlibs/x86_64/5.16/procfs.d
> > +++ b/dlibs/x86_64/5.16/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.2/procfs.d b/dlibs/x86_64/5.2/procfs.d
> > index 6ad926ee..23e05c2c 100644
> > --- a/dlibs/x86_64/5.2/procfs.d
> > +++ b/dlibs/x86_64/5.2/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.6/procfs.d b/dlibs/x86_64/5.6/procfs.d
> > index 7679db2e..96e55dc1 100644
> > --- a/dlibs/x86_64/5.6/procfs.d
> > +++ b/dlibs/x86_64/5.6/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/6.1/procfs.d b/dlibs/x86_64/6.1/procfs.d
> > index 2d52f079..4881aa5b 100644
> > --- a/dlibs/x86_64/6.1/procfs.d
> > +++ b/dlibs/x86_64/6.1/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/6.10/procfs.d b/dlibs/x86_64/6.10/procfs.d
> > index 2d52f079..4881aa5b 100644
> > --- a/dlibs/x86_64/6.10/procfs.d
> > +++ b/dlibs/x86_64/6.10/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* Not implemented yet. */
> > + pr_argv = 0; /* Not implemented yet. */
> > + pr_envp = 0; /* Not implemented yet. */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/libdtrace/procfs.d.in b/libdtrace/procfs.d.in
> > index 827d6b81..d4433be4 100644
> > --- a/libdtrace/procfs.d.in
> > +++ b/libdtrace/procfs.d.in
> > @@ -181,11 +181,9 @@ translator psinfo_t < struct task_struct *T > {
> > pr_fname = T->comm;
> > pr_psargs = d_execargs(T);
> > pr_wstat = 0;
> > -/*
> > - pr_argc = get_psinfo(T)->__psinfo(argc);
> > - pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > - pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > + pr_argc = 0; /* not implemented yet */
> > + pr_argv = 0; /* not implemented yet */
> > + pr_envp = 0; /* not implemented yet */
> > pr_dmodel = PR_MODEL_LP64;
> > diff --git a/test/unittest/builtinvar/tst.psinfo-bug21974606.d b/test/unittest/builtinvar/tst.psinfo-bug21974606.d
> > index 68d9503d..03857e83 100644
> > --- a/test/unittest/builtinvar/tst.psinfo-bug21974606.d
> > +++ b/test/unittest/builtinvar/tst.psinfo-bug21974606.d
> > @@ -4,7 +4,6 @@
> > * Licensed under the Universal Permissive License v 1.0 as shown at
> > * http://oss.oracle.com/licenses/upl.
> > */
> > -/* @@xfail: dtv2 */
> > /*
> > * ASSERTION:
> > diff --git a/test/unittest/builtinvar/tst.psinfo-bug22561297.d b/test/unittest/builtinvar/tst.psinfo-bug22561297.d
> > index b9efd0ec..ffccf469 100644
> > --- a/test/unittest/builtinvar/tst.psinfo-bug22561297.d
> > +++ b/test/unittest/builtinvar/tst.psinfo-bug22561297.d
> > @@ -4,11 +4,9 @@
> > * Licensed under the Universal Permissive License v 1.0 as shown at
> > * http://oss.oracle.com/licenses/upl.
> > */
> > -/* @@xfail: dtv2 */
> > /*
> > - * ASSERTION:
> > - * To print psinfo structure values from profile.
> > + * ASSERTION: To print psinfo structure values from profile.
> > *
> > * SECTION: Variables/Built-in Variables
> > */
> > diff --git a/test/unittest/builtinvar/tst.psinfo1.d b/test/unittest/builtinvar/tst.psinfo1.d
> > index 9e6d5053..7c451598 100644
> > --- a/test/unittest/builtinvar/tst.psinfo1.d
> > +++ b/test/unittest/builtinvar/tst.psinfo1.d
> > @@ -4,7 +4,6 @@
> > * Licensed under the Universal Permissive License v 1.0 as shown at
> > * http://oss.oracle.com/licenses/upl.
> > */
> > -/* @@xfail: dtv2 */
> > /*
> > * ASSERTION:
next prev parent reply other threads:[~2025-01-30 23:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-28 6:31 [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values Kris Van Hees
2025-01-30 22:33 ` Eugene Loh
2025-01-30 23:03 ` Kris Van Hees [this message]
2025-01-30 23:43 ` Eugene Loh
2025-01-31 0:13 ` Kris Van Hees
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z5wFUtaCwaxo1LpL@oracle.com \
--to=kris.van.hees@oracle.com \
--cc=dtrace-devel@oss.oracle.com \
--cc=dtrace@lists.linux.dev \
--cc=eugene.loh@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.