On Sun, Nov 16, 2008 at 11:38 AM, Chad Mynhier <[EMAIL PROTECTED]> wrote: > On Sun, Nov 16, 2008 at 10:46 AM, Dan McDonald <[EMAIL PROTECTED]> wrote: >> On Sun, Nov 16, 2008 at 07:43:36AM -0600, Mike Gerdts wrote: >>> I've posted an updated code review that adds a test case. I've >>> verified a full clobber build, that bfu delivers the new test case, >>> and that the test case fails without the fix and succeeds with the >>> fix. >> >> I'm certainly fine with the dtri.c fix (check env ONCE, then set state), and >> you can claim me as a reviewer for that file. Let's just hope there aren't >> any grinning weirdos out there who DEPEND on being able to change >> DTRACE_DOF_INIT_DEBUG in the middle of a run. :) (You know there's probably >> at least one person out there who'll complain...) >> >> You should get a 2nd okay on the test script, however. I see how it works, >> but I'm not familiar enough with .../dtrace/test/... to know if your script >> fits in with the big picture of how tests are done. > > I'm not sure that I can count as an official code reviewer, but here > are my comments, anyway: > > - The drti.c change looks fine.
ok. Thanks! > - I don't know that I'd create a new test suite directory just for > drti. I'd say that this belongs in the usdt directory. ok, moved. > - Minor aesthetic complaint: There's a tab in front of "all" in the > Makefile embedded in the test script. It works either way, but it > doesn't match other test scripts. (Of course, I go to verify that Fixed. > this is the case and find the other instance of this in > pid/tst.provregex3.ksh: > >> hg history ./pid/tst.provregex3.ksh > changeset: 5985:a183e54573d2 > user: jhaslam > date: Thu Feb 07 06:05:33 2008 -0800 > description: > 6325485 A stddev() aggregator would be a nice adjunct to avg() > 6618705 p*d123 doesn't cause pid probes to be created > 6624541 dtrace aggregations should assume signed arguments > Contributed by Chad Mynhier <[EMAIL PROTECTED]>. > >> > > Doh!) Well, at least you caught it this time. :) I had initially missed the packaging of the test case and have fixed that as well. I'm doing a fresh build to be sure I didn't break anything and will post the updated webrev later today. -- Mike Gerdts http://mgerdts.blogspot.com/ _______________________________________________ dtrace-discuss mailing list dtrace-discuss@opensolaris.org
