I'd like to put an end to this discussion. Mike has done enough work, and these tests are more than up to the task. If there are further comments on the code, that's fine, but we can suffer through tests that are functionally correct, but stylistically less than perfect.
Adam On Dec 10, 2008, at 6:13 AM, Roland Mainz wrote: > Mike Gerdts wrote: >> >> I believe that I have incorporated all of the feedback given >> (thanks!). Changes since the 2008-11-16 version include: >> >> - ksh style & coding standards compliance in test script (Roland) >> - "dof_init_debug == B_FALSE" vs. "!dof_init_debug" (Adam) >> >> The updated webrev is at: >> >> http://cr.opensolaris.org/~mgerdts/6750659-2008-12-03/ > > Quick review (patch code is quoted with "> "): > -- snip -- > [snip] >> + >> +PATH=/usr/bin:/usr/sbin:$PATH > > Is "export" not needed in this case, e.g. shouldn't subprocesses > inherit > PATH, too ? > >> +if (( $# != 1 )); then >> + print -u2 'expected one argument: <dtrace-path>' >> + exit 2 >> +fi >> + >> +# >> +# jdtrace does not implement the -h option that is required to >> generate >> +# C header files. >> +# >> +if [[ "$1" == */jdtrace ]]; then >> + exit 0 > > Is the zero return code (= "suceess") Ok in this case ? > >> +fi >> + >> +dtrace="$1" >> +startdir="$PWD" >> +dir=$(mktemp -td drtiXXXXXX) >> +if (( $? != 0 )); then >> + print -u2 'Could not create safe temporary directory' >> + exit 2 >> +fi >> + >> +cd "$dir" >> + >> +cat > Makefile <<EOF >> +all: main >> + >> +main: main.o prov.o >> + \$(CC) -o main main.o prov.o >> + >> +main.o: main.c prov.h >> + \$(CC) -c main.c >> + >> +prov.h: prov.d >> + $dtrace -h -s prov.d >> + >> +prov.o: prov.d main.o >> + $dtrace -G -32 -s prov.d main.o >> +EOF >> + >> +cat > prov.d <<EOF >> +provider tester { >> + probe entry(); >> +}; >> +EOF >> + >> +cat > main.c <<EOF >> +#include <stdlib.h> >> +#include <sys/sdt.h> >> +#include "prov.h" >> + >> +int >> +main(int argc, char **argv, char **envp) >> +{ >> + envp[0] = (char*)0xff; >> + TESTER_ENTRY(); >> + return 0; > > ISO C defines |EXIT_SUCCESS| (it's defined as |#define EXIT_SUCCESS > (0)|) ... I don't know whether it's better or worse in this case... > >> +} >> +EOF >> + >> +make > /dev/null >> +status=$? >> +if (( $status != 0 )) ; then > > Umpf... please remove the extra '$' - see > http://www.opensolaris.org/os/project/shell/shellstyle/#avoid_unneccesary_string_number_conversions > -- snip -- > > The remaining stuff looks good... :-) > > ---- > > Bye, > Roland > > -- > __ . . __ > (o.\ \/ /.o) [EMAIL PROTECTED] > \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer > /O /==\ O\ TEL +49 641 3992797 > (;O/ \/ \O;) > _______________________________________________ > dtrace-discuss mailing list > dtrace-discuss@opensolaris.org -- Adam Leventhal, Fishworks http://blogs.sun.com/ahl _______________________________________________ dtrace-discuss mailing list dtrace-discuss@opensolaris.org
