Re: [PATCH 5/6] procfs: add 'execargs' global variable as inline

[Kris Van Hees <kris.van.hees@oracle.com>]

On Wed, Jan 29, 2025 at 10:59:22PM -0500, Eugene Loh wrote:
> Is d_execargs() supposed to be user callable?  Is it supposed to be a
> documented feature?  Should it be tested?  What should happen if it is used
> in a user's D script?

I assume you mean this as feedback for the patch that introduces d_execargs
rather than this withdrawn patch?

Any function that is defined in D (as d_execargs needs to be) can be called by
users.

Whether it is documented is a matter to discuss with the documentation team,
but it certainly can be documented.  As long as it is understood that this is
a function that exists to support a translator and that its implementation is
done in function of that.  So e.g. we are not guaranteeing implementation
details beyond that.

It is tested.

If it is used in a user's D script, it will work as expected since it is a
regular D function, as one would expect.  I am not sure what you mean with
your question beyond that.

> On 1/29/25 01:45, Kris Van Hees wrote:
> > Actually, I am withdrawing this patch.  I forgot that we have curpsinfo as a
> > builtin, so execargs can be obtained as curpsinfo->pr_psargs, so there is no
> > need for a shorthand.
> > 
> > On Tue, Jan 28, 2025 at 09:56:17PM -0500, Kris Van Hees wrote:
> > > On Tue, Jan 28, 2025 at 07:53:51PM -0500, Eugene Loh wrote:
> > > > I'd like a little more context here.
> > > > 
> > > > Features:  There is a new execargs global variable.  I take it this is new
> > > > to D or an extension to the D language?  Would it make sense to call this a
> > > > built-in variable?  Are we also exposing a new function d_execargs()?
> > > > Shouldn't there be tests for using execargs or d_execargs() in scripts?  I
> > > > cannot tell if we are adding new user features (which should be tested) or
> > > > not.
> > > Yes, execargs is essentially a new built-in variable.  It is defined as an
> > > inline, simply doing d_execargs(curthread), so it is a short-hand to get to
> > > the pr_psargs info for the current task.
> > > 
> > > So it depends 100% on the d_execargs() implementation, which is a function
> > > that I added in order to implement the psinfo->pr_psargs translator
> > > functionality.
> > > 
> > > > Testing:  I see that the following patch (6/6) lifts some XFAILs on some
> > > > pr_psargs tests.  Should tst.psinfo.d be added to them? And should the
> > > > removal of XFAIL for test/unittest/builtinvar/tst.psinfo-bug21974606.d be
> > > > moved from that patch to this one?  And should there be correctness checks
> > > > for this output:  not simply "does this script not blow up?" but actually
> > > > "does this script produce correct or even just sensible output?"?
> > > None of the existing tests should be moved here because they do not use the
> > > ezecargs variable (since it is new).  I'll add a few tests for this built-in
> > > specifically.  I'd be happy to even drop this patch and not add a new
> > > built-in, but it seems somehow odd not to provide it.
> > > 
> > > > I'll look more but am certainly unclear if we're talking new D features here
> > > > (and if so, why they're not tested).
> > > > On 1/28/25 01:31, Kris Van Hees wrote:
> > > > 
> > > > > The 'execargs' global variable provides the psargs for curthread.
> > > > > 
> > > > > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > > > > ---
> > > > >    dlibs/aarch64/5.11/procfs.d | 4 ++++
> > > > >    dlibs/aarch64/5.12/procfs.d | 4 ++++
> > > > >    dlibs/aarch64/5.14/procfs.d | 4 ++++
> > > > >    dlibs/aarch64/5.16/procfs.d | 4 ++++
> > > > >    dlibs/aarch64/5.2/procfs.d  | 4 ++++
> > > > >    dlibs/aarch64/5.6/procfs.d  | 4 ++++
> > > > >    dlibs/aarch64/6.1/procfs.d  | 4 ++++
> > > > >    dlibs/aarch64/6.10/procfs.d | 4 ++++
> > > > >    dlibs/x86_64/5.11/procfs.d  | 4 ++++
> > > > >    dlibs/x86_64/5.12/procfs.d  | 4 ++++
> > > > >    dlibs/x86_64/5.14/procfs.d  | 4 ++++
> > > > >    dlibs/x86_64/5.16/procfs.d  | 4 ++++
> > > > >    dlibs/x86_64/5.2/procfs.d   | 4 ++++
> > > > >    dlibs/x86_64/5.6/procfs.d   | 4 ++++
> > > > >    dlibs/x86_64/6.1/procfs.d   | 4 ++++
> > > > >    dlibs/x86_64/6.10/procfs.d  | 4 ++++
> > > > >    libdtrace/procfs.d.in       | 4 ++++
> > > > >    17 files changed, 68 insertions(+)
> > > > > 
> > > > > diff --git a/dlibs/aarch64/5.11/procfs.d b/dlibs/aarch64/5.11/procfs.d
> > > > > index 70a43ddf..9c06fe1f 100644
> > > > > --- a/dlibs/aarch64/5.11/procfs.d
> > > > > +++ b/dlibs/aarch64/5.11/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/aarch64/5.12/procfs.d b/dlibs/aarch64/5.12/procfs.d
> > > > > index 70a43ddf..9c06fe1f 100644
> > > > > --- a/dlibs/aarch64/5.12/procfs.d
> > > > > +++ b/dlibs/aarch64/5.12/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/aarch64/5.14/procfs.d b/dlibs/aarch64/5.14/procfs.d
> > > > > index ef27bb70..2824d137 100644
> > > > > --- a/dlibs/aarch64/5.14/procfs.d
> > > > > +++ b/dlibs/aarch64/5.14/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/aarch64/5.16/procfs.d b/dlibs/aarch64/5.16/procfs.d
> > > > > index cad2d2c5..daf30745 100644
> > > > > --- a/dlibs/aarch64/5.16/procfs.d
> > > > > +++ b/dlibs/aarch64/5.16/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/aarch64/5.2/procfs.d b/dlibs/aarch64/5.2/procfs.d
> > > > > index 6b1b1b9c..3594e5e9 100644
> > > > > --- a/dlibs/aarch64/5.2/procfs.d
> > > > > +++ b/dlibs/aarch64/5.2/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/aarch64/5.6/procfs.d b/dlibs/aarch64/5.6/procfs.d
> > > > > index 70a43ddf..9c06fe1f 100644
> > > > > --- a/dlibs/aarch64/5.6/procfs.d
> > > > > +++ b/dlibs/aarch64/5.6/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/aarch64/6.1/procfs.d b/dlibs/aarch64/6.1/procfs.d
> > > > > index 4cb7b77c..2d52f079 100644
> > > > > --- a/dlibs/aarch64/6.1/procfs.d
> > > > > +++ b/dlibs/aarch64/6.1/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/aarch64/6.10/procfs.d b/dlibs/aarch64/6.10/procfs.d
> > > > > index 4cb7b77c..2d52f079 100644
> > > > > --- a/dlibs/aarch64/6.10/procfs.d
> > > > > +++ b/dlibs/aarch64/6.10/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/x86_64/5.11/procfs.d b/dlibs/x86_64/5.11/procfs.d
> > > > > index c2be76d8..7679db2e 100644
> > > > > --- a/dlibs/x86_64/5.11/procfs.d
> > > > > +++ b/dlibs/x86_64/5.11/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/x86_64/5.12/procfs.d b/dlibs/x86_64/5.12/procfs.d
> > > > > index c2be76d8..7679db2e 100644
> > > > > --- a/dlibs/x86_64/5.12/procfs.d
> > > > > +++ b/dlibs/x86_64/5.12/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/x86_64/5.14/procfs.d b/dlibs/x86_64/5.14/procfs.d
> > > > > index 28fada6d..3a348ebc 100644
> > > > > --- a/dlibs/x86_64/5.14/procfs.d
> > > > > +++ b/dlibs/x86_64/5.14/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/x86_64/5.16/procfs.d b/dlibs/x86_64/5.16/procfs.d
> > > > > index cad2d2c5..daf30745 100644
> > > > > --- a/dlibs/x86_64/5.16/procfs.d
> > > > > +++ b/dlibs/x86_64/5.16/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/x86_64/5.2/procfs.d b/dlibs/x86_64/5.2/procfs.d
> > > > > index 08696cf7..6ad926ee 100644
> > > > > --- a/dlibs/x86_64/5.2/procfs.d
> > > > > +++ b/dlibs/x86_64/5.2/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/x86_64/5.6/procfs.d b/dlibs/x86_64/5.6/procfs.d
> > > > > index c2be76d8..7679db2e 100644
> > > > > --- a/dlibs/x86_64/5.6/procfs.d
> > > > > +++ b/dlibs/x86_64/5.6/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/x86_64/6.1/procfs.d b/dlibs/x86_64/6.1/procfs.d
> > > > > index 4cb7b77c..2d52f079 100644
> > > > > --- a/dlibs/x86_64/6.1/procfs.d
> > > > > +++ b/dlibs/x86_64/6.1/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/dlibs/x86_64/6.10/procfs.d b/dlibs/x86_64/6.10/procfs.d
> > > > > index 4cb7b77c..2d52f079 100644
> > > > > --- a/dlibs/x86_64/6.10/procfs.d
> > > > > +++ b/dlibs/x86_64/6.10/procfs.d
> > > > > @@ -261,6 +261,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;
> > > > > diff --git a/libdtrace/procfs.d.in b/libdtrace/procfs.d.in
> > > > > index e9d50349..827d6b81 100644
> > > > > --- a/libdtrace/procfs.d.in
> > > > > +++ b/libdtrace/procfs.d.in
> > > > > @@ -307,6 +307,10 @@ inline string root = d_path(&(curthread->fs->root));
> > > > >    #pragma D attributes Stable/Stable/Common root
> > > > >    #pragma D binding "1.0" root
> > > > > +inline string execargs = d_execargs(curthread);
> > > > > +#pragma D attributes Stable/Stable/Common root
> > > > > +#pragma D binding "2.0" execargs
> > > > > +
> > > > >    inline int CLD_EXITED = 1;
> > > > >    #pragma D binding "1.0" CLD_EXITED
> > > > >    inline int CLD_KILLED = 2;