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

Bug 338171

Summary: StartOrRestartSequence_7_0 is being called for older GDBs
Product: [Tools] CDT Reporter: Marc Khouzam <marc.khouzam>
Component: cdt-debug-dsf-gdbAssignee: Marc Khouzam <marc.khouzam>
Status: RESOLVED FIXED QA Contact: Marc Khouzam <marc.khouzam>
Severity: normal    
Priority: P3 CC: cdtdoug, nobody, pawel.1.piech
Version: 8.0Flags: marc.khouzam: review? (nobody)
Target Milestone: 8.0   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Duplicating StartOrRestartSequence (for reference only)
marc.khouzam: iplog-
Fix marc.khouzam: iplog-

Description Marc Khouzam CLA 2011-02-24 20:33:44 EST 
In Bug 335528 I created a sequence called StartOrRestartSequence_7_0.  This sequence was meant to be used for GDB >= 7.0.  This is what I did for the restart case.  However, when I got to the 'start' case, I mistakenly made DebugNewProcessSequence call StartOrRestartSequence_7_0.  This is wrong because DebugNewProcessSequence is used for older GDBs.

My first idea to fix this was to have a StartOrRestartSequence for older GDBs.  However, this also required a DebugNewProcessSequence_7_0 version to call StartOrRestartSequence_7_0, and things started to be really confusing because of two things:

1- versioning a sequence, even with the new ReflectionSequence, is not as clean as versioning a service.
2- StartOrRestartSequence_7_0 is being called from two distinct places: DebugNewProcessSequence for the 'start' case, and GDBProcesses_7_0 for the 'restart' case.  This is not a good idea because if we need to override the sequence, we must override it in two places.

I think a cleaner approach is to add IGDBProcesses#start to match IGDBProcesses#restart (which is what we used to do in IGDBControl), which would allow us to call StartOrRestartSequence_7_0 from a single location, which is the IGDBProcesses services.

Attached, for reference only, is the (untested) patch for the first approach which I'm not going to use.

I will attach the second approach soon.
Comment 1 Marc Khouzam CLA 2011-02-24 20:34:25 EST 
Created attachment 189766 [details]
Duplicating StartOrRestartSequence (for reference only)
Comment 2 Marc Khouzam CLA 2011-02-24 21:20:37 EST 
Created attachment 189767 [details]
Fix

Here is the fix using IGDBProcesses#start, which is a much simpler solution (patch is almost 3 times smaller).

I also noticed that IGDBProcesses#restart should return (in the requestMonitor) the newly formed IContainerDMContext.

Committed to HEAD.
Comment 3 Marc Khouzam CLA 2011-02-24 21:21:07 EST 
Mikhail, can you review?
Comment 5 Nobody - feel free to take it CLA 2011-02-25 20:40:06 EST 
Code looks good. What do I need to do to try it?
Comment 6 Marc Khouzam CLA 2011-03-17 14:44:39 EDT 
(In reply to comment #5)
> Code looks good. What do I need to do to try it?

Did I not answer you?  Sorry, I just noticed.

If you launch a debug session with GDB 6.8, it should no longer call StartOrRestartSequence_7_0 but should instead call GDBProcesses#startOrRestart()