[DTrace-devel] [PATCH 2/2] Add support for substr() subroutine

[Eugene Loh]

On 9/4/21 2:10 AM, Kris Van Hees wrote:

> On Thu, Sep 02, 2021 at 06:34:55PM -0400, Eugene Loh wrote:
>> For now, my main comment is that it would be helpful (for review but
>> someday also for our users) if the semantics we're supporting would be
>> described in the patch (commit msg or even better in the code, like in
>> bpf/substr.S).  The exotic cases of idx and cnt do not seem to be
>> described in the documentation (which is another problem).  High-level
>> comments in bpf/substr.S saying what is intended would help someone
>> understand whether the code is operating correctly.
> I don't think that the commit message or the code is the best place to
> document the subroutine.  That ought to be done in, well, the documentation.
I agree, but the case is that we are not in that ideal situation. So far 
as I can tell, the intricate semantics are not described anywhere, 
except perhaps in your personal notes as you figured out what to 
implement.  So, it would be good to have these semantics described 
somewhere, even if not in the ideal place.  Among other things, that 
would help get the semantics documented properly someday.

I'm not sure if we should expect to see another version posted, but:
     Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
for whatever the most recent version is.  BTW, there might be an 
orphaned .Lidx_neg label left in substr.S.