Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 329724

Summary: DSF-GDB launcher fails if there are newlines between command line arguments
Product: [Tools] CDT Reporter: Sergey Prigogin <eclipse.sprigogin>
Component: cdt-debug-dsf-gdbAssignee: Sergey Prigogin <eclipse.sprigogin>
Status: RESOLVED FIXED QA Contact: Marc Khouzam <marc.khouzam>
Severity: normal    
Priority: P3 CC: cdtdoug, pawel.1.piech
Version: 8.0   
Target Milestone: 7.0.2   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Fix that has API breaking changes
marc.khouzam: iplog-
Added new method to factory
marc.khouzam: iplog-
For posterity: fix in HEAD eclipse.sprigogin: iplog-

Description Sergey Prigogin CLA 2010-11-08 23:49:44 EST 
To reproduce:
1. Create a DSF-GDB debug launcher for a simplest application with command line arguments specified as:
-a
-b

2. Try to launch a debug session.

You should get 
/bin/bash: -c: line 0: unexpected EOF while looking for matching `"'
/bin/bash: -c: line 1: syntax error: unexpected end of file
in the application console. This is caused the following command passed to gdb:
840,682 10-gdb-set args "-a
-b"

DSF-GDB launcher should replace all newlines with spaces before passing the command line arguments to gdb.

The Standard Create Process Launcher seems to already do this.
Comment 1 Marc Khouzam CLA 2010-11-09 14:51:26 EST 
Created attachment 182759 [details]
Fix that has API breaking changes

Good find.

This patch uses the same logic as CDI.  Problem is it changes some APIs.  We can try to improve the solution to avoid those breaking changes, especially for a fix to the 7_0 branch.

I wanted to at least post a solution right away.
Comment 2 Sergey Prigogin CLA 2010-11-29 19:42:28 EST 
Fixed in cdt_7_0 and HEAD > 20101129.

A simple hacky fix that doesn't break the APIs.
Comment 4 Marc Khouzam CLA 2010-11-30 09:42:48 EST 
Created attachment 184134 [details]
Added new method to factory

Added the new constructor MIGDBSetArgs(ICommandControlDMContext, String[])  to the CommandFactory.

Committed to HEAD.

(In reply to comment #2)
> Fixed in cdt_7_0 and HEAD > 20101129.
> 
> A simple hacky fix that doesn't break the APIs.

Thanks Sergey.  The fix is fine.  We shouldn't internally keep using a deprecated method, but I guess it is better than nothing for now.

Can you please attach a patch when you commit something?  At least for DSF-GDB stuff.  This is an agreement we have to help other committers review things more quickly.  The justification has been discussed in: Bug 325943 comment 8 and bug 325943 comment 9.

Thanks.
Comment 5 CDT Genie CLA 2010-11-30 10:23:06 EST 
*** cdt cvs genie on behalf of mkhouzam ***
Bug 329724: New method added to CommandFactory

[*] CommandFactory.java 1.18 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/CommandFactory.java?root=Tools_Project&r1=1.17&r2=1.18
Comment 6 Sergey Prigogin CLA 2010-11-30 13:23:07 EST 
Created attachment 184161 [details]
For posterity: fix in HEAD

(In reply to comment #4)
I thought CDT Genie links are just as good as patches, at least for simple changes.
Comment 7 Sergey Prigogin CLA 2010-11-30 13:27:06 EST 
(In reply to comment #5)
Shouldn't CommandFactory.createMIGDBSetArgs(ICommandControlDMContext dmc, String arguments) be deprecated?
Comment 8 Marc Khouzam CLA 2010-11-30 14:21:01 EST 
(In reply to comment #7)
> (In reply to comment #5)
> Shouldn't CommandFactory.createMIGDBSetArgs(ICommandControlDMContext dmc,
> String arguments) be deprecated?

It should, but that creates a warning in the plugins that use it, and the gdbjtag plugin seems to have "warnings as errors", so marking it as deprecated was breaking the compilation.  What I would like is that we not use the deprecated method at all in any of our plugins, but this is a lower priority.