Skip to content

IEP-1110 NPE when trying to create new launch configuration #852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 15, 2023
Merged

IEP-1110 NPE when trying to create new launch configuration #852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b8021c
fix: fixing possible NPEs when creating launch/debug configs
sigmaaa Nov 8, 2023
9c64dea
fix: adding null check to prevent NPE
sigmaaa Nov 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions ...idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabDebugger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,11 +783,15 @@ public void modifyText(ModifyEvent e)
private String getLaunchTarget()
{
launchBarManager = Activator.getService(ILaunchBarManager.class);
String selectedTarget = ""; //$NON-NLS-1$
String selectedTarget = StringUtil.EMPTY;
try
{
selectedTarget = launchBarManager.getActiveLaunchTarget().getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET,
""); //$NON-NLS-1$
if (launchBarManager.getActiveLaunchConfiguration() != null)
{
selectedTarget = launchBarManager.getActiveLaunchTarget()
.getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET, StringUtil.EMPTY);
}

Comment on lines +786 to +794
Copy link

@coderabbitai coderabbitai bot Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The null check for launchBarManager.getActiveLaunchConfiguration() is good for preventing Null Pointer Exceptions. However, it would be better to also check if launchBarManager.getActiveLaunchTarget() is not null before calling getAttribute on it. This will further enhance the robustness of the code.

- if (launchBarManager.getActiveLaunchConfiguration() != null)
+ if (launchBarManager.getActiveLaunchConfiguration() != null && launchBarManager.getActiveLaunchTarget() != null)
Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.

Suggested change
String selectedTarget = StringUtil.EMPTY;
try
{
selectedTarget = launchBarManager.getActiveLaunchTarget().getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET,
""); //$NON-NLS-1$
if (launchBarManager.getActiveLaunchConfiguration() != null)
{
selectedTarget = launchBarManager.getActiveLaunchTarget()
.getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET, StringUtil.EMPTY);
}
String selectedTarget = StringUtil.EMPTY;
try
{
if (launchBarManager.getActiveLaunchConfiguration() != null && launchBarManager.getActiveLaunchTarget() != null)
{
selectedTarget = launchBarManager.getActiveLaunchTarget()
.getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET, StringUtil.EMPTY);
}

}
catch (CoreException e)
{
Expand Down
2 changes: 1 addition & 1 deletion ...sif.idf.debug.gdbjtag.openocd/src/com/espressif/idf/debug/gdbjtag/openocd/ui/TabMain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ protected void initializeProgramName(ICElement cElement, ILaunchConfigurationWor
IProject project = cElement.getCProject().getProject();
String name = project.getName();
ICProjectDescription projDes = CCorePlugin.getDefault().getProjectDescription(project);
if (projDes != null)
if (projDes != null && projDes.getActiveConfiguration() != null)
{
String buildConfigName = projDes.getActiveConfiguration().getName();
name = name + " " + buildConfigName; //$NON-NLS-1$
Expand Down
12 changes: 8 additions & 4 deletions ...f.idf.launch.serial.ui/src/com/espressif/idf/launch/serial/ui/internal/CMakeMainTab2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,9 @@ public void widgetSelected(SelectionEvent evt) {

Optional<String> suitableTarget = Stream.of(targetsWithDfuSupport).filter(t -> {
try {
return t.contentEquals(launchBarManager.getActiveLaunchTarget().getId());
if (launchBarManager.getActiveLaunchConfiguration() != null) {
return t.contentEquals(launchBarManager.getActiveLaunchTarget().getId());
}
Comment on lines +311 to +313
Copy link

@coderabbitai coderabbitai bot Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The null check for getActiveLaunchConfiguration() is good for preventing NullPointerExceptions. However, consider storing the result of getActiveLaunchTarget() in a local variable to avoid calling it twice, which could potentially improve performance.

- if (launchBarManager.getActiveLaunchConfiguration() != null) {
-     return t.contentEquals(launchBarManager.getActiveLaunchTarget().getId());
- }
+ LaunchTarget activeLaunchTarget = launchBarManager.getActiveLaunchTarget();
+ if (activeLaunchTarget != null) {
+     return t.contentEquals(activeLaunchTarget.getId());
+ }
Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.

Suggested change
if (launchBarManager.getActiveLaunchConfiguration() != null) {
return t.contentEquals(launchBarManager.getActiveLaunchTarget().getId());
}
LaunchTarget activeLaunchTarget = launchBarManager.getActiveLaunchTarget();
if (activeLaunchTarget != null) {
return t.contentEquals(activeLaunchTarget.getId());
}

} catch (CoreException e) {
Logger.log(e);
}
Expand Down Expand Up @@ -490,7 +492,7 @@ protected void initializeCProject(IProject project, ILaunchConfigurationWorkingC
config.setMappedResources(new IResource[] { project });

ICProjectDescription projDes = CCorePlugin.getDefault().getProjectDescription(project);
if (projDes != null) {
if (projDes != null && projDes.getActiveConfiguration() != null) {
String buildConfigID = projDes.getActiveConfiguration().getId();
config.setAttribute(ICDTLaunchConfigurationConstants.ATTR_PROJECT_BUILD_CONFIG_ID, buildConfigID);
}
Expand Down Expand Up @@ -803,8 +805,10 @@ private String newVariableExpression(OpenocdDynamicVariable dynamicVariable) {
private String getLaunchTarget() {
String selectedTarget = StringUtil.EMPTY;
try {
selectedTarget = launchBarManager.getActiveLaunchTarget().getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET,
StringUtil.EMPTY);
if (launchBarManager.getActiveLaunchConfiguration() != null) {
selectedTarget = launchBarManager.getActiveLaunchTarget()
.getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET, StringUtil.EMPTY);
}
Comment on lines +808 to +811
Copy link

@coderabbitai coderabbitai bot Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the first comment, consider storing the result of getActiveLaunchTarget() in a local variable to avoid calling it twice.

- if (launchBarManager.getActiveLaunchConfiguration() != null) {
-     selectedTarget = launchBarManager.getActiveLaunchTarget()
-             .getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET, StringUtil.EMPTY);
- }
+ LaunchTarget activeLaunchTarget = launchBarManager.getActiveLaunchTarget();
+ if (activeLaunchTarget != null) {
+     selectedTarget = activeLaunchTarget.getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET, StringUtil.EMPTY);
+ }
Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.

Suggested change
if (launchBarManager.getActiveLaunchConfiguration() != null) {
selectedTarget = launchBarManager.getActiveLaunchTarget()
.getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET, StringUtil.EMPTY);
}
LaunchTarget activeLaunchTarget = launchBarManager.getActiveLaunchTarget();
if (activeLaunchTarget != null) {
selectedTarget = activeLaunchTarget.getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET, StringUtil.EMPTY);
}

} catch (CoreException e) {
Logger.log(e);
}
Expand Down