From 3fd4ddeddca3aae12d31d8353807b722bbc7a642 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Tue, 14 Oct 2008 20:26:01 +0000 Subject: [PATCH] [240092] some cleanup to GDB parts, of which the most important ones are to fix the JUnit tests and remove get getGDBCommandLine() from IGDBBackend, which was not needed. --- .../dd/examples/pda/service/PDABackend.java | 2 +- .../launching/FinalLaunchSequence.java | 41 ++++---- .../provisional/service/GDBBackend.java | 99 ++++++++----------- .../provisional/service/IGDBBackend.java | 17 +--- 4 files changed, 68 insertions(+), 91 deletions(-) diff --git a/plugins/org.eclipse.dd.examples.pda/src/org/eclipse/dd/examples/pda/service/PDABackend.java b/plugins/org.eclipse.dd.examples.pda/src/org/eclipse/dd/examples/pda/service/PDABackend.java index 2c3801ab9bf..f3ece3aa972 100644 --- a/plugins/org.eclipse.dd.examples.pda/src/org/eclipse/dd/examples/pda/service/PDABackend.java +++ b/plugins/org.eclipse.dd.examples.pda/src/org/eclipse/dd/examples/pda/service/PDABackend.java @@ -6,7 +6,7 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Ling Wang (Nokia) - initial version. Sep 28, 2008 + * Nokia - initial version. Sep 28, 2008 *******************************************************************************/ package org.eclipse.dd.examples.pda.service; diff --git a/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/launching/FinalLaunchSequence.java b/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/launching/FinalLaunchSequence.java index 26947f39481..c75d40ce4fc 100644 --- a/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/launching/FinalLaunchSequence.java +++ b/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/launching/FinalLaunchSequence.java @@ -81,9 +81,20 @@ public class FinalLaunchSequence extends Sequence { new Step() { @Override public void execute(RequestMonitor requestMonitor) { fCommandControl = fTracker.getService(IGDBControl.class); + if (fCommandControl == null) { + requestMonitor.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, "Cannot obtain control service", null)); //$NON-NLS-1$ + } + + requestMonitor.done(); + }}, + /* + * Fetch the process service for later use + */ + new Step() { @Override + public void execute(RequestMonitor requestMonitor) { fProcService = fTracker.getService(IMIProcesses.class); - if (fCommandControl == null || fProcService == null) { - requestMonitor.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, "Cannot obtain service", null)); //$NON-NLS-1$ + if (fProcService == null) { + requestMonitor.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, "Cannot obtain process service", null)); //$NON-NLS-1$ } requestMonitor.done(); @@ -168,23 +179,17 @@ public class FinalLaunchSequence extends Sequence { /* * Specify GDB's working directory */ - new Step() { - - private IPath getWorkingDirectory(RequestMonitor requestMonitor) { - IPath path = null; - try { - path = fGDBBackend.getGDBWorkingDirectory(); - } catch (CoreException e) { - requestMonitor.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, "Cannot get working directory", e)); //$NON-NLS-1$ - requestMonitor.done(); - } - - return path; - } - - @Override + new Step() { @Override public void execute(final RequestMonitor requestMonitor) { - IPath dir = getWorkingDirectory(requestMonitor); + IPath dir = null; + try { + dir = fGDBBackend.getGDBWorkingDirectory(); + } catch (CoreException e) { + requestMonitor.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, "Cannot get working directory", e)); //$NON-NLS-1$ + requestMonitor.done(); + return; + } + if (dir != null) { fCommandControl.queueCommand( new MIEnvironmentCD(fCommandControl.getContext(), dir.toPortableString()), diff --git a/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/GDBBackend.java b/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/GDBBackend.java index 14b570f2c83..8c4ec2cd257 100644 --- a/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/GDBBackend.java +++ b/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/GDBBackend.java @@ -6,7 +6,7 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Ling Wang (Nokia) - initial API and implementation with some code moved from GDBControl. + * Nokia - initial API and implementation with some code moved from GDBControl. * Wind River System * Ericsson *******************************************************************************/ @@ -68,11 +68,6 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { private ILaunchConfiguration fLaunchConfiguration; - /** - * Command line to start GDB. - */ - private List fGDBCommandLine; - /* * Parameters for launching GDB. */ @@ -102,7 +97,8 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { fLaunchConfiguration = lc; try { - ICProject cproject = LaunchUtils.verifyCProject(lc); + // Don't call verifyCProject, because the JUnit tests are not setting a project + ICProject cproject = LaunchUtils.getCProject(lc); fProgramPath = LaunchUtils.verifyProgramPath(lc, cproject); } catch (CoreException e) { fProgramPath = new Path(""); //$NON-NLS-1$ @@ -148,37 +144,34 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { private IPath getGDBPath() { - IPath retVal = new Path(IGDBLaunchConfigurationConstants.DEBUGGER_DEBUG_NAME_DEFAULT); - try { - retVal = new Path(fLaunchConfiguration.getAttribute(IGDBLaunchConfigurationConstants.ATTR_DEBUG_NAME, - IGDBLaunchConfigurationConstants.DEBUGGER_DEBUG_NAME_DEFAULT)); - } catch (CoreException e) { - } - return retVal; + return LaunchUtils.getGDBPath(fLaunchConfiguration); } - public List getGDBCommandLine() { - if (fGDBCommandLine == null) - { - fGDBCommandLine = new ArrayList(); - - // The goal here is to keep options to an absolute minimum. - // All configuration should be done in the launch sequence - // to allow for easy overriding. - fGDBCommandLine.add(getGDBPath().toOSString()); - fGDBCommandLine.add("--interpreter"); //$NON-NLS-1$ - fGDBCommandLine.add("mi"); //$NON-NLS-1$ - // Don't read the gdbinit file here. It is read explicitly in - // the LaunchSequence to make it easier to customize. - fGDBCommandLine.add("--nx"); //$NON-NLS-1$ - } + /* + * Options for GDB process. + * Allow subclass to override. + */ + protected String getGDBCommandLine() { + StringBuffer gdbCommandLine = new StringBuffer(getGDBPath().toOSString()); + + // The goal here is to keep options to an absolute minimum. + // All configuration should be done in the launch sequence + // to allow for more flexibility. + gdbCommandLine.append(" --interpreter"); //$NON-NLS-1$ + // We currently work with MI version 2. Don't use just 'mi' because it + // points to the latest MI version, while we want mi2 specifically. + gdbCommandLine.append(" mi2"); //$NON-NLS-1$ + // Don't read the gdbinit file here. It is read explicitly in + // the LaunchSequence to make it easier to customize. + gdbCommandLine.append(" --nx"); //$NON-NLS-1$ - return fGDBCommandLine; + return gdbCommandLine.toString(); } public String getGDBInitFile() throws CoreException { if (fGDBInitFile == null) { - fGDBInitFile = fLaunchConfiguration.getAttribute(IGDBLaunchConfigurationConstants.ATTR_GDB_INIT, ""); //$NON-NLS-1$ + fGDBInitFile = fLaunchConfiguration.getAttribute(IGDBLaunchConfigurationConstants.ATTR_GDB_INIT, + IGDBLaunchConfigurationConstants.DEBUGGER_GDB_INIT_DEFAULT); } return fGDBInitFile; @@ -187,7 +180,7 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { public IPath getGDBWorkingDirectory() throws CoreException { if (fGDBWorkingDirectory == null) { - // First try to use the user-sepcified working directory for the debugged program. + // First try to use the user-specified working directory for the debugged program. // This is fine only with local debug. // For remote debug, the working dir of the debugged program will be on remote device // and hence not applicable. In such case we may just use debugged program path on host @@ -247,8 +240,9 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { ICDTLaunchConfigurationConstants.ATTR_PROGRAM_ARGUMENTS, (String)null); - if (fProgramArguments != null) + if (fProgramArguments != null) { fProgramArguments = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(fProgramArguments); + } } return fProgramArguments; @@ -262,7 +256,7 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { public List getSharedLibraryPaths() throws CoreException { if (fSharedLibPaths == null) { fSharedLibPaths = fLaunchConfiguration.getAttribute(IGDBLaunchConfigurationConstants.ATTR_DEBUGGER_SOLIB_PATH, - new ArrayList(0)); + new ArrayList(0)); } return fSharedLibPaths; @@ -272,15 +266,12 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { * Launch GDB process. * Allow subclass to override. */ - protected Process launchGDBProcess() throws CoreException { - List gdbCmdLine = getGDBCommandLine(); - String[] commandLine = gdbCmdLine.toArray(new String[gdbCmdLine.size()]); - + protected Process launchGDBProcess(String commandLine) throws CoreException { Process proc = null; try { proc = ProcessFactory.getFactory().exec(commandLine); } catch (IOException e) { - String message = "Error while launching command " + gdbCmdLine.toString(); //$NON-NLS-1$ + String message = "Error while launching command " + commandLine; //$NON-NLS-1$ throw new CoreException(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, message, e)); } @@ -311,17 +302,10 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { } public void destroy() { - if (getState() != State.STARTED) - return; - - // destroy() should be supported even if it's not spawner. - fProcess.destroy(); - /* - if (fProcess instanceof Spawner) { - Spawner gdbSpawner = (Spawner) fProcess; - gdbSpawner.destroy(); - } - */ + // destroy() should be supported even if it's not spawner. + if (getState() == State.STARTED) { + fProcess.destroy(); + } } public State getState() { @@ -370,25 +354,22 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { final Job startGdbJob = new Job("Start GDB Process Job") { //$NON-NLS-1$ @Override protected IStatus run(IProgressMonitor monitor) { - List commandList = getGDBCommandLine(); + String commandLine = getGDBCommandLine(); try { - fProcess = launchGDBProcess(); + fProcess = launchGDBProcess(commandLine); } catch(CoreException e) { - String message = "Error while launching command " + commandList.toString(); //$NON-NLS-1$ - gdbLaunchRequestMonitor.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, message, e)); + gdbLaunchRequestMonitor.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, e.getMessage(), e)); gdbLaunchRequestMonitor.done(); return Status.OK_STATUS; } try { - InputStream stream = fProcess.getInputStream(); - Reader r = new InputStreamReader(stream); + Reader r = new InputStreamReader(getMIInputStream()); BufferedReader reader = new BufferedReader(r); String line; while ((line = reader.readLine()) != null) { line = line.trim(); - //System.out.println("GDB " + line); if (line.endsWith("(gdb)")) { //$NON-NLS-1$ break; } @@ -431,7 +412,7 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { int attempts = 0; while (attempts < 10) { try { - // Don't know if we really need the exit value... but what the hell. + // Don't know if we really need the exit value... but what the heck. fGDBExitValue = fProcess.exitValue(); // throws exception if process not exited requestMonitor.done(); @@ -490,7 +471,7 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend { * This event is not consumed by any one at present, instead it's * the GDBControlInitializedDMEvent that's used to indicate that GDB * back end is ready for MI commands. But we still fire the event as - * it does no harm and may be needed sometime.... 09/29/08 + * it does no harm and may be needed sometime.... 09/29/2008 */ getSession().dispatchEvent( new BackendStateChangedEvent(getSession().getId(), getId(), IMIBackend.State.STARTED), diff --git a/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/IGDBBackend.java b/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/IGDBBackend.java index 565f8220698..b1fa83e91fa 100644 --- a/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/IGDBBackend.java +++ b/plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/IGDBBackend.java @@ -6,7 +6,8 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Ling Wang (Nokia) - initial version. Sep, 2008 + * Nokia - initial version + * Ericsson - Minor cleanup *******************************************************************************/ package org.eclipse.dd.gdb.internal.provisional.service; @@ -37,16 +38,6 @@ import org.eclipse.dd.mi.service.IMIBackend; */ public interface IGDBBackend extends IMIBackend { - - /** - * Return the command line that is used to launch GDB.
- * Note it's not needed to put debugged binary in the command, as that will - * be set by a MI command. - * - * @return - */ - public List getGDBCommandLine(); - /** * Get path of the debugged program on host. * @@ -57,8 +48,8 @@ public interface IGDBBackend extends IMIBackend { /** * Get init file for GDB. * - * @return file name, may have relative or absolute path, or empty string - * ("") indicating an init file is not specified. + * @return file name, may have relative or absolute path, or empty string ("") + * indicating an init file is not specified. * @throws CoreException * - error in getting the option. */