[DTrace-devel] [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values

[Eugene Loh]

On 1/30/25 18:03, Kris Van Hees wrote:

> 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.

 From the user's point of view, the choice is between:

*)  the status quo, where an attempt to use one of the unimplemented 
fields returns a rather nice error message that indicates what the 
problem is

*)  the proposed patch, where we "silently" return incorrect values, 
leaving the user with incorrect results or wasting time debugging the 
problem, filing a bug, etc.

So, there is hardly a balance here at all for users:  the status quo is 
far better.

 From our test suite's point of view, there is a choice between less 
coverage (some tests XFAIL) and "more coverage", but I question that 
characterization since the "more coverage" case still doesn't check the 
correctness of the output.  Anyhow, to the extent that this is a 
test-suite issue, let's fix it in the test suite instead of polluting 
user-visible behavior.  Let's not "game the test suite" by modifying 
dtrace.  The whole set of tst.psinfo*.d tests are in need of overhaul;  
so let's improve test coverage by cleaning up the tests rather than 
feeding users incorrect results.

If you'd like me to take a stab at cleaning up tst.psinfo*.d, let me know.

>> 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.

FWIW, test/unittest/builtinvar/tst.psinfo.d would also XPASS with this 
patch.

>> 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.