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

Bug 312481

Summary:

Launchers should not use default connection info from IGDBJtagConstants

Product:

[Tools] CDT

Reporter:

Bruce Griffith <Bruce.Griffith>

Component:

cdt-debug

Assignee:

John Cortell <john.cortell>

Status:

RESOLVED FIXED

QA Contact:

Ken Ryall <ken.ryall>

Severity:

minor

Priority:

CC:

Bruce.Griffith, elaskavaia.cdt, john.cortell, marc.khouzam, pawel.1.piech, steve.goodrich

Version:

7.0

Target Milestone:

6.0.1

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

Description	Flags
Fix	john.cortell: iplog-

Description Bruce Griffith

2010-05-11 14:19:26 EDT

Build Identifier: Build id: 20100506-2000

GDBJtagDSFFinalLaunchSequence (DSF) and GDBJtagDebugger (CDI) use default port and address constants (for example, IGDBJtagConstants.DEFAULT_IP_ADDRESS) when retrieving the stored connection information from the UI.  These constants are examples and have no relevance to any real-world probe.  In the real world, if a code bug lets these constants become the connect at launch then, at best, the result will be wierd error messages and/or unexpected behavior for the user.  At worst, the user will connect to the wrong probe and mess up someone else's debug board.

I don't think these constants should be used to "fix up" the connection string when launching a connection to a probe.  Each probe definition object contains method to retrieve the default connection (IP/Port or URI string).  If a default is really desired, then the code should retrieve the default connection from the probe definition object.  A better choice would be to fail to launch if the connection string cannot be read from the UI. 

I think there are cases in the UI (GDBJtagDebuggerTab and GDBJtagDSFDebuggerTab) where the connection defaults from IGDBJtagConstants are used when a call to a JtagDevice get default method is more appropriate.


Reproducible: Always

Steps to Reproduce:
Code review:

1.  Lines 428 to 460 of GDBJtagDSFFinalLaunchSequence.java
2.  Lines 168 to 191 of GDBJtagDebugger.java

Comment 1 Elena Laskavaia

2010-05-11 22:30:12 EDT

do you want to attach a patch?

Comment 2 Bruce Griffith

2010-05-12 02:42:13 EDT

Sure, I'll post a patch.

There's no rush from my end.  Does this make sense to defer until after Helios?

Comment 3 John Cortell

2010-05-12 08:05:51 EDT

Bruce, using DEFAULT_ constants when getting an attribute from a configuration is intended to reduce coding errors. It is a widely followed practice. I'm not for changing that approach. What I can do is address the concern you have that it might end up using some other board. I think it's easy enough to choose values that will make that unlikely. Note that what you are talking about is improving the user experience where there's a *coding* error or something really unexpected has happened (say the user opens the launch configuration file and starts ripping out lines). Altering or adding code to handle those situations is questionable, IMO. E.g., one could argue that the absence of any of the expected attributes should result in an aborted launch. Providing that sort of defense in the code is onerous and excessive. 

I will change the defaults to ones less likely to succeed, but that's about all I think we should do.

Comment 4 John Cortell

2010-05-12 08:19:21 EDT

Created attachment 168126 [details]
Fix

Committed to HEAD.

Comment 5 John Cortell

2010-05-12 08:20:36 EDT

FIXED

Comment 6 CDT Genie

2010-07-28 15:26:53 EDT

*** cdt cvs genie on behalf of jcortell ***
Bug 312481: Use defaults less likely to cause an unintended connection to another board.

[*] IGDBJtagConstants.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/jtag/org.eclipse.cdt.debug.gdbjtag.core/src/org/eclipse/cdt/debug/gdbjtag/core/IGDBJtagConstants.java?root=Tools_Project&r1=1.6&r2=1.7