[DTrace-devel] [PATCH CORRECTED] Implement normalize() and denormalize()

Eugene Loh eugene.loh at oracle.com
Thu Jan 14 16:09:58 PST 2021


Copyright on new files should not have 2006.

Copyright on touched files should have 2021.

I guess err.D_NORMALIZE_SCALAR.bad.d has become 
err.D_PROTO_ARG.norm_non_scalar_normal.d .  The test got cleaned up in 
the process;  that's fine.  What I'm curious about is that the error has 
changed from DTv1 (from NORMALIZE_SCALAR to PROTO_ARG).  I haven't 
looked closely at this issue, but given that it's a change in 
user-visible behavior should there at least be a comment on this 
change?  (And that's assuming the change is intentional.)  FWIW:

     DTv1
         # dtrace -n 'BEGIN { @a = count(); normalize(@a, "a"); exit(0) }'
         dtrace: invalid probe specifier BEGIN { @a = count(); 
normalize(@a, "a"); exit(0) }:
             normalize( ) argument #2 must be of scalar type

     DTv2
         # dtrace -n 'BEGIN { @a = count(); normalize(@a, "a"); exit(0) }'
         dtrace: invalid probe specifier BEGIN { @a = count(); 
normalize(@a, "a"); exit(0) }:
             in action list: normalize( ) argument #2 is incompatible 
with prototype:
          prototype: uint64_t
           argument: string

It's nice that we have more err.* test coverage, but the tst.* coverage 
still seems scant.  In particular, we document "renormalization" 
behavior:  if there are multiple normalize() calls, the last one wins.  
How about at least a simple test in that respect?  I have no reason to 
believe it will fail, but it's easy enough to add.

I don't know one way or the other, but in dt_consume_one() is the 
following comment still relevant?

     /*
      * FIXME: This code is temporary.
      */

In dt_normalize(), it seems pointless to define "act = rec->dtrd_arg" 
for a single use of act.

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>




More information about the DTrace-devel mailing list