Re: [cdt-dev] Two patches for AbstractMIControl

[Marc Khouzam <marc.khouzam@xxxxxxxxxxxx>]

Sorry Jason, I guess I jumped the gun in my first reply.
When I saw code, I assumed it was a patch :O
Let me go through you email in more detail and I'll reply again.

> -----Original Message-----
> From: cdt-dev-bounces@xxxxxxxxxxx 
> [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of Jason Litton
> Sent: Tuesday, March 13, 2012 10:30 AM
> To: CDT General developers list.
> Subject: [cdt-dev] Two patches for AbstractMIControl
> 
> I've been working on CDT 8.0.1 extensions and I've found a 
> couple of bugs in AbstractMIControl that I'd like to submit 
> patches for. The first is around line 919 in the command:
>                         final String errorResult =  
> rr.getResultClass();
> 						
> 			if ( 
> errorResult.equals(MIResultRecord.ERROR) ) {
> 				String status = 
> getStatusString(commandHandle.getCommand(),response);
> 				String message = 
> getBackendMessage(response);
> 				Exception exception = new 
> Exception(message);
> 				rm.setStatus(new 
> Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, REQUEST_FAILED, 
> status, exception)); 
> 			}
> 
> 
>                       if (commandHandle.getRequestMonitor() != null) {
>                                 
> commandHandle.getRequestMonitor().done();
>                       }
> 
>                       processCommandDone(commandHandle, finalResult);
> 
> If a GDB command throws an error message on the GDB traces 
> console, the request monitor will have its status set to 
> error, but the .done() method ends up throwing an exception 
> and the command handle never reaches the processCommandDone. 
> It appears the intention here was to process all commands as 
> done, whether they're errors or not. This has an effect on 
> the whole queueCommand sequence since some of the commands in 
> the queue will never be registered as done. 
> 
> Second, I've found that it's possible to overrun the queue. I 
> instrumented the calls for queueCommand and 
> processNextQueuedCommand. Then I was doing some operations 
> that were flooding the queue (1024 events). In queueCommand, 
> fCommandQueue.add(handle) would add the correct handle, but 
> in processNextQueuedCommand would sometimes pull two of one 
> handle and then skip the next handle. For example, 
> queueCommand would queue handles A, B, C, D, E, etc, but 
> processNextQueuedCommand would remove handles A, B, C, C, E. 
> Therefore, the command associated with D would never be 
> marked as completed. Higher up, I was using a countdown 
> latch, and it would get stuck.
> 
> I have pretty simple patches for both of these bugs, but I've 
> never submitted patches before, so I need a little help with 
> that part. I've searched bugzilla and haven't found bugs for 
> these. I'm willing to file them, but I'd like to make sure 
> that I'm not replicating bugs that already exist. Second, I'm 
> trying to come up with reproduction for the bugs. I've found 
> them through our extensions. The first bug, I'm trying to 
> figure out something that will always cause a GDB error. For 
> the second, I was flooding with monitor commands. I can 
> probably knock up a method to run 1024 memory reads, but I 
> assume this would have to be an external class that you could 
> run. Does this need to have a GUI interface? Should it be an 
> independent plugin? I'd just like to make sure that I do this 
> right. Any tips would be greatly appreciated.
> Thanks,
> Jason Litton
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>