Community
Participate
Working Groups
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
do you want to attach a patch?
Sure, I'll post a patch. There's no rush from my end. Does this make sense to defer until after Helios?
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.
Created attachment 168126 [details] Fix Committed to HEAD.
FIXED
*** 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
