Fix up handling of DTraceArgument objects.

This change adds a destructor for the DTraceArgument base class,
improves handling of probe argument values, and adds tests for
more argument cases.

dtrace_argument.cc

@@ -9,12 +9,17 @@ namespace node {
 
   // Integer Argument
 
-  void * DTraceIntegerArgument::ArgumentValue(Handle<Value> value) {
 #ifdef __x86_64__
-    return (void *)(long) value->ToInteger()->Value();
+# define INTMETHOD ToInteger()
 #else
-    return (void *)(int) value->ToInt32()->Value();
+# define INTMETHOD ToInt32()
 #endif
+
+  void * DTraceIntegerArgument::ArgumentValue(Handle<Value> value) {
+    if (value->IsUndefined())
+      return 0;
+    else
+      return (void *)(long) value->INTMETHOD->Value();
   }
 
   void DTraceIntegerArgument::FreeArgument(void *arg) {
@@ -27,6 +32,9 @@ namespace node {
   // String Argument
 
   void * DTraceStringArgument::ArgumentValue(Handle<Value> value) {
+    if (value->IsUndefined())
+      return (void *) strdup("undefined");
+
     String::AsciiValue str(value->ToString());
     return (void *) strdup(*str);
   }
@@ -60,6 +68,9 @@ namespace node {
   void * DTraceJsonArgument::ArgumentValue(Handle<Value> value) {
     HandleScope scope;
 
+    if (value->IsUndefined())
+      return (void *) strdup("undefined");
+
     Handle<Value> args[1];
     args[0] = value;
     Handle<Value> j = JSON_stringify->Call(JSON, 1, args);

dtrace_probe.cc

@@ -6,15 +6,15 @@
 namespace node {
 
   using namespace v8;
-  
+
   DTraceProbe::DTraceProbe() : ObjectWrap() {
+    argc = 0;
     probedef = NULL;
   }
-  
+
   DTraceProbe::~DTraceProbe() {
-    for (size_t i = 0; i < argc; i++) {
+    for (size_t i = 0; i < argc; i++)
       delete(this->arguments[i]);
-    }
     usdt_probe_release(probedef);
   }
 
@@ -76,11 +76,6 @@ namespace node {
     Local<Array> a = Local<Array>::Cast(probe_args);
     void *argv[USDT_ARG_MAX];
 
-    // limit argc to the defined number of probe args
-    argc = a->Length();
-    if (argc > this->probedef->argc)
-      argc = this->probedef->argc;
-
     // convert each argument value
     for (size_t i = 0; i < argc; i++) {
       argv[i] = this->arguments[i]->ArgumentValue(a->Get(i));

dtrace_provider.cc

@@ -80,7 +80,6 @@ namespace node {
   Handle<Value> DTraceProvider::AddProbe(const Arguments& args) {
     HandleScope scope;
     const char *types[USDT_ARG_MAX];
-    int argc = 0;
 
     Handle<Object> obj = args.Holder();
     DTraceProvider *provider = ObjectWrap::Unwrap<DTraceProvider>(obj);
@@ -108,15 +107,15 @@ namespace node {
           probe->arguments[i] = new DTraceStringArgument();
 
         types[i] = strdup(probe->arguments[i]->Type());
-        argc++;
+        probe->argc++;
       }
     }
 
     String::AsciiValue name(args[0]->ToString());
-    probe->probedef = usdt_create_probe(*name, *name, argc, types);
+    probe->probedef = usdt_create_probe(*name, *name, probe->argc, types);
     usdt_provider_add_probe(provider->provider, probe->probedef);
 
-    for (int i = 0; i < argc; i++) {
+    for (size_t i = 0; i < probe->argc; i++) {
       free((char *)types[i]);
     }
 

dtrace_provider.h

@@ -26,10 +26,10 @@ namespace node {
 
   class DTraceArgument {
   public:
-    virtual const char *Type();
-    virtual void *ArgumentValue(Handle<Value>);
-    virtual void FreeArgument(void *);
-    virtual ~DTraceArgument();
+    virtual const char *Type() = 0;
+    virtual void *ArgumentValue(Handle<Value>) = 0;
+    virtual void FreeArgument(void *) = 0;
+    virtual ~DTraceArgument() { };
   };
 
   class DTraceIntegerArgument : public DTraceArgument {

test/basic.test.js

@@ -0,0 +1,21 @@
+var test = require('tap').test;
+var format = require('util').format;
+var dtest = require('./dtrace-test').dtraceTest;
+
+test(
+    'basic probes',
+    dtest(
+        function() {
+        },
+        [
+            'dtrace', '-Zqn',
+            'nodeapp$target:::p1{ printf("%d\\n", arg0); printf("%s\\n", copyinstr(arg1)) }',
+            '-c', format('node %s/basic_fire.js', __dirname)
+        ],
+        function(t, exit_code, traces) {
+            t.notOk(exit_code, 'dtrace exited cleanly');
+            t.equal(traces[0], '42');
+            t.equal(traces[1], 'forty-two');
+        }
+    )
+);

test/basic_fire.js

@@ -0,0 +1,8 @@
+var d = require('../dtrace-provider');
+var provider = d.createDTraceProvider("nodeapp");
+var probe = provider.addProbe("p1", "int", "char *");
+provider.enable();
+
+probe.fire(function(p) {
+    return [42, 'forty-two'];
+});

test/fewer-args-json.test.js

@@ -0,0 +1,21 @@
+var test = require('tap').test;
+var format = require('util').format;
+var dtest = require('./dtrace-test').dtraceTest;
+
+test(
+    'firing JSON probe with too few arguments',
+    dtest(
+        function() {
+        },
+        [
+            'dtrace', '-Zqn',
+            'nodeapp$target:::p1{ printf("%s\\n%s\\n", copyinstr(arg0), copyinstr(arg1)); }',
+            '-c', format('node %s/fewer-args-json_fire.js', __dirname)
+        ],
+        function(t, exit_code, traces) {
+            t.notOk(exit_code, 'dtrace exited cleanly');
+            t.equal(traces[0], '{"foo":1}');
+            t.equal(traces[1], 'undefined');
+        }
+    )
+);

test/fewer-args-json_fire.js

@@ -0,0 +1,8 @@
+var d = require('../dtrace-provider');
+var provider = d.createDTraceProvider("nodeapp");
+var probe = provider.addProbe("p1", "json", "json");
+provider.enable();
+
+probe.fire(function(p) {
+    return [{ "foo": 1 }];
+});

test/fewer-args.test.js

@@ -0,0 +1,22 @@
+var test = require('tap').test;
+var format = require('util').format;
+var dtest = require('./dtrace-test').dtraceTest;
+
+test(
+    'firing probe with too few arguments',
+    dtest(
+        function() {
+        },
+        [
+            'dtrace', '-Zqn',
+            'nodeapp$target:::p1{ printf("%d\\n%d\\n%s\\n", arg0, arg1, copyinstr(arg2)); }',
+            '-c', format('node %s/fewer-args_fire.js', __dirname)
+        ],
+        function(t, exit_code, traces) {
+            t.notOk(exit_code, 'dtrace exited cleanly');
+            t.equal(traces[0], '42');
+            t.equal(traces[1], '0');
+            t.equal(traces[2], 'undefined');
+        }
+    )
+);

test/fewer-args_fire.js

@@ -0,0 +1,8 @@
+var d = require('../dtrace-provider');
+var provider = d.createDTraceProvider("nodeapp");
+var probe = provider.addProbe("p1", "int", "int", "char *");
+provider.enable();
+
+probe.fire(function(p) {
+    return [42];
+});

test/gc.js

@@ -1,7 +1,7 @@
 // expected output:
 // 
 // $ sudo dtrace -Zn 'nodeapp*:::gcprobe{ trace(arg0); }' -c 'node --expose_gc test/gc.js'
-// dtrace: description 'nodeapp*:::gcprobe' matched 0 probes
+// Dtrace: description 'nodeapp*:::gcprobe' matched 0 probes
 // dtrace: pid 66257 has exited
 // CPU     ID                    FUNCTION:NAME
 //   1   1778                  gcprobe:gcprobe        4320227343
@@ -10,7 +10,7 @@ var d = require('../dtrace-provider');
 var dtp = d.createDTraceProvider("nodeapp");
 
 // don't assign the returned probe object anywhere
-dtp.addProbe("gcprobe");
+dtp.addProbe("gcprobe", "int");
 dtp.enable();
 
 // run GC

test/gc.test.js

@@ -0,0 +1,20 @@
+var test = require('tap').test;
+var format = require('util').format;
+var dtest = require('./dtrace-test').dtraceTest;
+
+test(
+    'check probe object is not GCd while provider exists',
+    dtest(
+        function() {
+        },
+        [
+            'dtrace', '-Zqn',
+            'nodeapp$target:::gcprobe{ printf("%d\\n", arg0); }',
+            '-c', format('node --expose_gc %s/gc_fire.js', __dirname)
+        ],
+        function(t, exit_code, traces) {
+            t.notOk(exit_code, 'dtrace exited cleanly');
+            t.equal(traces[0], '0');
+        }
+    )
+);

test/gc_fire.js

@@ -0,0 +1,14 @@
+var d = require('../dtrace-provider');
+var dtp = d.createDTraceProvider("nodeapp");
+
+// don't assign the returned probe object anywhere
+dtp.addProbe("gcprobe", "int");
+dtp.enable();
+
+// run GC
+gc();
+
+// probe object should still be around
+dtp.fire("gcprobe", function() {
+    return [];
+});

test/more-args.test.js

@@ -0,0 +1,21 @@
+var test = require('tap').test;
+var format = require('util').format;
+var dtest = require('./dtrace-test').dtraceTest;
+
+test(
+    'firing probe with too many arguments',
+    dtest(
+        function() {
+        },
+        [
+            'dtrace', '-Zqn',
+            'nodeapp$target:::p1{ printf("%d\\n%d\\n", arg0, arg1); }',
+            '-c', format('node %s/more-args_fire.js', __dirname)
+        ],
+        function(t, exit_code, traces) {
+            t.notOk(exit_code, 'dtrace exited cleanly');
+            t.equal(traces[0], '1');
+            t.equal(traces[1], '2');
+        }
+    )
+);

test/more-args_fire.js

@@ -0,0 +1,8 @@
+var d = require('../dtrace-provider');
+var provider = d.createDTraceProvider("nodeapp");
+var probe = provider.addProbe("p1", "int", "int");
+provider.enable();
+
+probe.fire(function(p) {
+    return [1, 2, 3, 4];
+});