[DTrace-devel] [PATCH 12/12] Add support for copyinstr() subroutine

[Kris Van Hees]

On Thu, Jul 21, 2022 at 03:52:27PM -0700, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> 
> In dt_cg_subr_copyinstr(), the TRACE_REGSET messages should refer to
> copyinstr rather than copyin.

Ah yes.

> There is a lot of code duplicated between
>     dt_cg_subr_copyin()
>     dt_cg_subr_copyinstr()
> Might it be useful to consolidate that code and reuse that consolidation for
> both purposes?

I tried that.  It was a lot more messy.  Hence I kept them separate.

> In the tests
>     test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.d
>     test/unittest/funcs/copyinstr/tst.copyinstr.d
> there is a
>      * SECTION: Actions and Subroutines/copyin()
> I do not understand the utility of such comments, but in any case it would
> seem that s/copyin/copyinstr/.

A lot of other tests have them, so I have continued that practice.  And yes,
I need to change the name (unless this is the one that actually does not have
its own section - I will check).

> The tests
>     test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.d
>     test/unittest/funcs/copyinstr/tst.copyinstr.d
> should check results as I think you're now doing with tst.copyin.d in the
> earlier copyin() patch.

Yes.

> Why does err.badsize.d have no .r file -- err.badaddr.d does!  The same
> question goes for the copyin patch.

Because I forgot something - incidentally, right now, that badsize test is
giving a BPF verifier log output which is wrong.  It should report a runtime
error.  Debugging.