From 677449d867a2614f8ef55eaeab966831b8b20657 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Fri, 11 May 2012 12:27:48 -0400 Subject: [PATCH] Bug 379277: Address FindBugs issues for DSF Change-Id: I9fb46d009b55830615881b75ae50e3640e047395 Reviewed-on: https://git.eclipse.org/r/5954 Reviewed-by: Pawel Piech IP-Clean: Pawel Piech Tested-by: Pawel Piech Reviewed-by: Marc Khouzam IP-Clean: Marc Khouzam Tested-by: Marc Khouzam --- .../internal/ui/EvaluationContextManager.java | 11 ++++++++--- .../disassembly/model/DisassemblyDocument.java | 2 +- .../internal/ui/disassembly/text/REDRun.java | 2 +- .../detail/NumberFormatDetailPane.java | 2 +- .../ui/viewmodel/actions/VMHandlerUtils.java | 3 ++- .../breakpoints/AbstractBreakpointVMNode.java | 4 ++-- .../ui/viewmodel/breakpoints/DataCache.java | 2 +- .../register/SyncRegisterDataAccess.java | 16 ++++++++++++---- .../ui/viewmodel/datamodel/AbstractDMVMNode.java | 2 +- .../org/eclipse/cdt/dsf/service/DsfSession.java | 4 ++-- 10 files changed, 31 insertions(+), 17 deletions(-) diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/EvaluationContextManager.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/EvaluationContextManager.java index b8163ea525b..98021f86ea3 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/EvaluationContextManager.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/EvaluationContextManager.java @@ -54,13 +54,18 @@ public class EvaluationContextManager implements IWindowListener, IDebugContextL @Override public void run() { if ( fgManager == null ) { - fgManager = new EvaluationContextManager(); + // FindBugs reported that it is unsafe to set s_resources + // before we finish to initialize the object, because of + // multi-threading. This is why we use a temporary variable. + EvaluationContextManager manager = new EvaluationContextManager(); IWorkbench workbench = PlatformUI.getWorkbench(); IWorkbenchWindow[] windows = workbench.getWorkbenchWindows(); for( int i = 0; i < windows.length; i++ ) { - fgManager.windowOpened( windows[i] ); + manager.windowOpened( windows[i] ); } - workbench.addWindowListener( fgManager ); + workbench.addWindowListener( manager ); + + fgManager = manager; } } }; diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java index 34a2390f0b6..1411e30af77 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java @@ -851,7 +851,7 @@ public class DisassemblyDocument extends REDDocument implements IDisassemblyDocu if (positions != null) { positions.removeAll(toRemove); } - if (category != CATEGORY_MODEL) { + if (!category.equals(CATEGORY_MODEL)) { positions = (List) getDocumentManagedPositions().get(CATEGORY_MODEL); if (positions != null) { positions.removeAll(toRemove); diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/text/REDRun.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/text/REDRun.java index 8be86044ba4..8316ccd0a7b 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/text/REDRun.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/text/REDRun.java @@ -117,7 +117,7 @@ public class REDRun implements CharSequence { try { return asString(); } catch (IOException e) { - return null; + return ""; //$NON-NLS-1$ } } diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/viewmodel/numberformat/detail/NumberFormatDetailPane.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/viewmodel/numberformat/detail/NumberFormatDetailPane.java index 5858610545c..604d19a5674 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/viewmodel/numberformat/detail/NumberFormatDetailPane.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/viewmodel/numberformat/detail/NumberFormatDetailPane.java @@ -788,7 +788,7 @@ public class NumberFormatDetailPane implements IDetailPane2, IAdaptable, IProper * elements being displayed in this view */ protected void setDebugModel(String id) { - if (id != fDebugModelIdentifier) { + if (!id.equals(fDebugModelIdentifier)) { fDebugModelIdentifier = id; configureDetailsViewer(); } diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/actions/VMHandlerUtils.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/actions/VMHandlerUtils.java index 5d0d6f9274f..92dabb8e7f6 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/actions/VMHandlerUtils.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/actions/VMHandlerUtils.java @@ -65,8 +65,9 @@ public class VMHandlerUtils { IPartService partService = (IPartService)serviceLocator.getService(IPartService.class); if (partService != null) { part = partService.getActivePart(); + return getVMProviderForPart(part); } - return getVMProviderForPart(part); + return null; } } diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/breakpoints/AbstractBreakpointVMNode.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/breakpoints/AbstractBreakpointVMNode.java index 58b2c7426c1..1629846ed50 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/breakpoints/AbstractBreakpointVMNode.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/breakpoints/AbstractBreakpointVMNode.java @@ -152,7 +152,7 @@ public abstract class AbstractBreakpointVMNode extends AbstractVMNode { return IModelDelta.EXPAND | IModelDelta.SELECT; } } - else if (event instanceof DebugContextEvent && (((DebugContextEvent)event).getFlags() | DebugContextEvent.ACTIVATED) != 0) { + else if (event instanceof DebugContextEvent && (((DebugContextEvent)event).getFlags() & DebugContextEvent.ACTIVATED) != 0) { int flags = IModelDelta.NO_CHANGE; if ( Boolean.TRUE.equals(getVMProvider().getPresentationContext().getProperty(IBreakpointUIConstants.PROP_BREAKPOINTS_FILTER_SELECTION)) ) { flags |= IModelDelta.CONTENT; @@ -203,7 +203,7 @@ public abstract class AbstractBreakpointVMNode extends AbstractVMNode { } } } - else if (event instanceof DebugContextEvent && (((DebugContextEvent)event).getFlags() | DebugContextEvent.ACTIVATED) != 0) { + else if (event instanceof DebugContextEvent && (((DebugContextEvent)event).getFlags() & DebugContextEvent.ACTIVATED) != 0) { if ( Boolean.TRUE.equals(getVMProvider().getPresentationContext().getProperty(IBreakpointUIConstants.PROP_BREAKPOINTS_FILTER_SELECTION)) ) { parent.setFlags(parent.getFlags() | IModelDelta.CONTENT); } diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/breakpoints/DataCache.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/breakpoints/DataCache.java index eb97b3e6b22..521de3616d3 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/breakpoints/DataCache.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/breakpoints/DataCache.java @@ -114,7 +114,7 @@ abstract class DataCache { if (!isCanceled()) { fValid = true; fRm = null; - set(getData(), getStatus()); + set(super.getData(), super.getStatus()); } } }; diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/register/SyncRegisterDataAccess.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/register/SyncRegisterDataAccess.java index ead3d637f83..c98d32275c9 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/register/SyncRegisterDataAccess.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/register/SyncRegisterDataAccess.java @@ -729,9 +729,10 @@ public class SyncRegisterDataAccess { IRegisterGroupDMContext.class); } + if (dmc != null) { DsfSession session = DsfSession.getSession(dmc.getSessionId()); - if (dmc != null && session != null) { + if (session != null) { GetRegisterGroupDataQuery query = new GetRegisterGroupDataQuery(dmc); session.getExecutor().execute(query); @@ -741,6 +742,7 @@ public class SyncRegisterDataAccess { } catch (ExecutionException e) { } } + } return null; } @@ -762,9 +764,11 @@ public class SyncRegisterDataAccess { if (element instanceof IDMVMContext) { dmc = DMContexts.getAncestorOfType( ((IDMVMContext) element).getDMContext(), IRegisterDMContext.class ); } + + if (dmc != null) { DsfSession session = DsfSession.getSession(dmc.getSessionId()); - if (dmc != null && session != null) { + if (session != null) { GetRegisterDataQuery query = new GetRegisterDataQuery(dmc); session.getExecutor().execute(query); @@ -774,6 +778,7 @@ public class SyncRegisterDataAccess { } catch (ExecutionException e) { } } + } return null; } @@ -794,9 +799,11 @@ public class SyncRegisterDataAccess { if (element instanceof IDMVMContext) { dmc = DMContexts.getAncestorOfType( ((IDMVMContext) element).getDMContext(), IBitFieldDMContext.class ); } - DsfSession session = DsfSession.getSession(dmc.getSessionId()); - if (dmc != null && session != null) { + if (dmc != null) { + DsfSession session = DsfSession.getSession(dmc.getSessionId()); + + if (session != null) { GetBitFieldQuery query = new GetBitFieldQuery(dmc); session.getExecutor().execute(query); @@ -806,6 +813,7 @@ public class SyncRegisterDataAccess { } catch (ExecutionException e) { } } + } return null; } diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/ui/viewmodel/datamodel/AbstractDMVMNode.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/ui/viewmodel/datamodel/AbstractDMVMNode.java index c2fd67e66a9..76acabc022a 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/ui/viewmodel/datamodel/AbstractDMVMNode.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/ui/viewmodel/datamodel/AbstractDMVMNode.java @@ -169,7 +169,7 @@ abstract public class AbstractDMVMNode extends AbstractVMNode implements IVMNode if (element instanceof IDMVMContext) { // If update element is a DMC, check if session is still alive. IDMContext dmc = ((IDMVMContext)element).getDMContext(); - if (dmc.getSessionId() != getSession().getId() || !DsfSession.isSessionActive(dmc.getSessionId())) { + if (!dmc.getSessionId().equals(getSession().getId()) || !DsfSession.isSessionActive(dmc.getSessionId())) { handleFailedUpdate(update); return false; } diff --git a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/service/DsfSession.java b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/service/DsfSession.java index b2f15a3a89b..cbc5a8fd5ef 100644 --- a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/service/DsfSession.java +++ b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/service/DsfSession.java @@ -473,9 +473,9 @@ public class DsfSession if (o1.fListener == o2.fListener) { return 0; } if (o1.fListener instanceof IDsfService && !(o2.fListener instanceof IDsfService)) { - return Integer.MIN_VALUE; + return -1; } else if (o2.fListener instanceof IDsfService && !(o1.fListener instanceof IDsfService)) { - return Integer.MAX_VALUE; + return 1; } else if ( (o1.fListener instanceof IDsfService) && (o2.fListener instanceof IDsfService) ) { return ((IDsfService)o1.fListener).getStartupNumber() - ((IDsfService)o2.fListener).getStartupNumber(); }