src: don't crash with no arguments to print (#106)

* src: replace uses of NULL with nullptr * src: don't segfault on null commands This commit prevents the DoExecute() methods from segfaulting when the cmd argument is null. * test: allow stderr to be scripted This commit allows stderr to be scripted in the same way that stdout is. * test: validate command usage messages PR-URL: #106 Reviewed-By: Ben Noordhuis <[email protected]>
nodejs · Jun 29, 2017 · 628ad41 · 628ad41
1 parent 3de7485
commit 628ad41
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 53 deletions.
diff --git a/src/llnode.cc b/src/llnode.cc
@@ -130,7 +130,7 @@ bool BacktraceCmd::DoExecute(SBDebugger d, char** cmd,
 
 bool PrintCmd::DoExecute(SBDebugger d, char** cmd,
                          SBCommandReturnObject& result) {
-  if (*cmd == NULL) {
+  if (cmd == nullptr || *cmd == nullptr) {
     if (detailed_) {
       result.SetError("USAGE: v8 inspect [flags] expr\n");
     } else {
@@ -184,6 +184,11 @@ bool PrintCmd::DoExecute(SBDebugger d, char** cmd,
 
 bool ListCmd::DoExecute(SBDebugger d, char** cmd,
                         SBCommandReturnObject& result) {
+  if (cmd == nullptr || *cmd == nullptr) {
+    result.SetError("USAGE: v8 source list\n");
+    return false;
+  }
+
   static SBFrame last_frame;
   static uint64_t last_line = 0;
   SBTarget target = d.GetSelectedTarget();

diff --git a/src/llscan.cc b/src/llscan.cc
@@ -72,7 +72,7 @@ bool FindObjectsCmd::DoExecute(SBDebugger d, char** cmd,
 
 bool FindInstancesCmd::DoExecute(SBDebugger d, char** cmd,
                                  SBCommandReturnObject& result) {
-  if (*cmd == NULL) {
+  if (cmd == nullptr || *cmd == nullptr) {
     result.SetError("USAGE: v8 findjsinstances [-Fm] instance_name\n");
     return false;
   }
@@ -300,7 +300,7 @@ bool NodeInfoCmd::DoExecute(SBDebugger d, char** cmd,
 
 bool FindReferencesCmd::DoExecute(SBDebugger d, char** cmd,
                                   SBCommandReturnObject& result) {
-  if (*cmd == NULL) {
+  if (cmd == nullptr || *cmd == nullptr) {
     result.SetError("USAGE: v8 findrefs expr\n");
     return false;
   }

diff --git a/test/common.js b/test/common.js
@@ -23,76 +23,40 @@ else
 
 exports.llnodePath = path.join(exports.buildDir, pluginName);
 
-function Session(scenario) {
+function SessionOutput(session, stream) {
   EventEmitter.call(this);
-
-  // lldb -- node scenario.js
-  this.lldb = spawn(process.env.TEST_LLDB_BINARY || 'lldb', [
-    '--',
-    process.execPath,
-    '--abort_on_uncaught_exception',
-    path.join(exports.fixturesDir, scenario)
-  ], {
-    stdio: [ 'pipe', 'pipe', 'inherit' ],
-    env: util._extend(util._extend({}, process.env), {
-      LLNODE_RANGESFILE: exports.ranges
-    })
-  });
-
-  this.lldb.stdin.write(`plugin load "${exports.llnodePath}"\n`);
-  this.lldb.stdin.write('run\n');
-
-  this.initialized = false;
   this.waiting = false;
   this.waitQueue = [];
 
   let buf = '';
-  this.lldb.stdout.on('data', (data) => {
+
+  stream.on('data', (data) => {
     buf += data;
 
     for (;;) {
       let index = buf.indexOf('\n');
+
       if (index === -1)
         break;
 
       const line = buf.slice(0, index);
       buf = buf.slice(index + 1);
 
       if (/process \d+ exited/i.test(line))
-        this.kill();
-      else if (this.initialized)
+        session.kill();
+      else if (session.initialized)
         this.emit('line', line);
       else if (/process \d+ launched/i.test(line))
-        this.initialized = true;
+        session.initialized = true;
     }
   });
 
   // Ignore errors
-  this.lldb.stdin.on('error', () => {});
-  this.lldb.stdout.on('error', () => {});
+  stream.on('error', () => {});
 }
-util.inherits(Session, EventEmitter);
-exports.Session = Session;
-
-Session.create = function create(scenario) {
-  return new Session(scenario);
-};
+util.inherits(SessionOutput, EventEmitter);
 
-Session.prototype.kill = function kill() {
-  this.lldb.kill();
-  this.lldb = null;
-};
-
-Session.prototype.quit = function quit() {
-  this.send('kill');
-  this.send('quit');
-};
-
-Session.prototype.send = function send(line, callback) {
-  this.lldb.stdin.write(line + '\n', callback);
-};
-
-Session.prototype._queueWait = function _queueWait(retry) {
+SessionOutput.prototype._queueWait = function _queueWait(retry) {
   if (this.waiting) {
     this.waitQueue.push(retry);
     return false;
@@ -102,13 +66,13 @@ Session.prototype._queueWait = function _queueWait(retry) {
   return true;
 };
 
-Session.prototype._unqueueWait = function _unqueueWait() {
+SessionOutput.prototype._unqueueWait = function _unqueueWait() {
   this.waiting = false;
   if (this.waitQueue.length > 0)
     this.waitQueue.shift()();
 };
 
-Session.prototype.wait = function wait(regexp, callback) {
+SessionOutput.prototype.wait = function wait(regexp, callback) {
   if (!this._queueWait(() => { this.wait(regexp, callback); }))
     return;
 
@@ -124,11 +88,11 @@ Session.prototype.wait = function wait(regexp, callback) {
   });
 };
 
-Session.prototype.waitBreak = function waitBreak(callback) {
+SessionOutput.prototype.waitBreak = function waitBreak(callback) {
   this.wait(/Process \d+ stopped/i, callback);
 };
 
-Session.prototype.linesUntil = function linesUntil(regexp, callback) {
+SessionOutput.prototype.linesUntil = function linesUntil(regexp, callback) {
   if (!this._queueWait(() => { this.linesUntil(regexp, callback); }))
     return;
 
@@ -147,6 +111,57 @@ Session.prototype.linesUntil = function linesUntil(regexp, callback) {
   });
 };
 
+
+function Session(scenario) {
+  EventEmitter.call(this);
+
+  // lldb -- node scenario.js
+  this.lldb = spawn(process.env.TEST_LLDB_BINARY || 'lldb', [
+    '--',
+    process.execPath,
+    '--abort_on_uncaught_exception',
+    path.join(exports.fixturesDir, scenario)
+  ], {
+    stdio: [ 'pipe', 'pipe', 'pipe' ],
+    env: util._extend(util._extend({}, process.env), {
+      LLNODE_RANGESFILE: exports.ranges
+    })
+  });
+
+  this.lldb.stdin.write(`plugin load "${exports.llnodePath}"\n`);
+  this.lldb.stdin.write('run\n');
+
+  this.initialized = false;
+  this.stdout = new SessionOutput(this, this.lldb.stdout);
+  this.stderr = new SessionOutput(this, this.lldb.stderr);
+
+  // Map these methods to stdout for compatibility with legacy tests.
+  this.wait = SessionOutput.prototype.wait.bind(this.stdout);
+  this.waitBreak = SessionOutput.prototype.waitBreak.bind(this.stdout);
+  this.linesUntil = SessionOutput.prototype.linesUntil.bind(this.stdout);
+}
+util.inherits(Session, EventEmitter);
+exports.Session = Session;
+
+Session.create = function create(scenario) {
+  return new Session(scenario);
+};
+
+Session.prototype.kill = function kill() {
+  this.lldb.kill();
+  this.lldb = null;
+};
+
+Session.prototype.quit = function quit() {
+  this.send('kill');
+  this.send('quit');
+};
+
+Session.prototype.send = function send(line, callback) {
+  this.lldb.stdin.write(line + '\n', callback);
+};
+
+
 exports.generateRanges = function generateRanges(cb) {
   let script;
   if (process.platform === 'darwin')

diff --git a/test/usage-test.js b/test/usage-test.js
@@ -0,0 +1,44 @@
+'use strict';
+const tape = require('tape');
+const common = require('./common');
+
+function removeBlankLines(lines) {
+  return lines.filter((line) => { return line.trim() !== ''; });
+}
+
+tape('usage messages', (t) => {
+  t.timeoutAfter(15000);
+
+  const sess = common.Session.create('inspect-scenario.js');
+
+  sess.waitBreak(() => {
+    sess.send('v8 print');
+  });
+
+  sess.stderr.linesUntil(/USAGE/, (lines) => {
+    t.ok(/^error: USAGE: v8 print expr$/.test(removeBlankLines(lines)[0]),
+         'print usage message');
+    sess.send('v8 source list');
+  });
+
+  sess.stderr.linesUntil(/USAGE/, (lines) => {
+    t.ok(/^error: USAGE: v8 source list$/.test(removeBlankLines(lines)[0]),
+         'list usage message');
+    sess.send('v8 findjsinstances');
+  });
+
+  sess.stderr.linesUntil(/USAGE/, (lines) => {
+    const re = /^error: USAGE: v8 findjsinstances \[-Fm\] instance_name$/;
+
+    t.ok(re.test(removeBlankLines(lines)[0]),
+         'findjsinstances usage message');
+    sess.send('v8 findrefs');
+  });
+
+  sess.stderr.linesUntil(/USAGE/, (lines) => {
+    t.ok(/^error: USAGE: v8 findrefs expr$/.test(removeBlankLines(lines)[0]),
+         'findrefs usage message');
+    sess.quit();
+    t.end();
+  });
+});