[DTrace-devel] [PATCH v3] Add support for array/struct/union to trace()

[Kris Van Hees]

On Tue, Feb 01, 2022 at 06:10:17PM -0500, Eugene Loh via DTrace-devel wrote:
> On 2/1/22 5:15 PM, Kris Van Hees wrote:
> 
> > On Tue, Feb 01, 2022 at 01:50:28PM -0500, Eugene Loh via DTrace-devel wrote:
> > 
> > > Is the output in tst.array.r stable?  Or are only the first 7 bytes
> > > ("dtrace\0") stable?  The current tst.array.r might work for a broad range
> > > of cases, but strictly speaking it probably ought to be fixed.  E.g., maybe
> > > add some .r.p file that does
> > > sed 's/ 0: 64 74 72 61 63 65 00 .*  dtrace\..*/ 0: 64 74 72 61 63 65 00 00
> > > 00 00 00 00 00 00 00 00  dtrace........../'
> > It is stable.
> 
> ?  It is checking the garbage that is beyond the NUL terminating char.  That
> means the test relies on irrelevant details to pass.

I made a mistake in that the content is not guaranteed to be stable (faulty
testing - it depends on where dtrace is installed).  But your suggested .r.p
is faulty as well.  It fails to test that we do get indeed have 16 bytes in
the array output.

I'll provide a .r.p to ensure we validate the output correctly.

> > > The additional tst.struct<n>.d tests are nice, but we often try to name
> > > tests more meaningfully.  That would be easy enough to do here.  E.g.
> > > tst.struct-int8.d and so on.
> > I don't think that adds any value.
> 
> ?  It would name things in a way that reflect what they are.  It would
> improve consistency even with other tests in this same directory.  Other
> code reviews have asked for similar changes (instead of enumerating
> variations of a test name, to use test names that suggest the differences).

I have considered that but the tests aren't about the integers that are stored
in the structs but more about whether the correct allignment is assigned (and
that the DT_NF_REF flag is set).  In that sense, maybe I should just get rid
of the tst.struct[345].d tests and rename tst.struct2.d into tst.struct-1-byte.d
and be done with it.  Since all we really are testing at this point is that
the alignment is set to 2, even for a struct of size 1.  tst.struct.d already
verifies that a struct of a larger size is output as raw bytes.