Skip to content
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.

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

Protect NPE in CMRI connection preferences #2734

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
a40b2c9
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 18, 2016
1acdaab
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 19, 2016
63fd56f
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 20, 2016
345f929
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 20, 2016
bcc1efa
Protect NPE while changing CMRI connections
KenC57 Dec 20, 2016
1100222
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 21, 2016
2332728
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 25, 2016
469e09d
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 25, 2016
222d49c
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 27, 2016
a18da70
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 29, 2016
e700c98
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 30, 2016
3e5c3eb
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 30, 2016
ec5d0e1
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Dec 31, 2016
728fb68
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 1, 2017
2a299d4
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 1, 2017
a44ed8e
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 2, 2017
2dd633f
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 3, 2017
d65fbb8
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 3, 2017
62e5dca
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 3, 2017
c272052
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 4, 2017
c4a40b8
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 5, 2017
31b3005
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 6, 2017
b0da2de
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 6, 2017
6a433ff
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 6, 2017
f780a5b
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 9, 2017
a7f85ee
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 9, 2017
94100ef
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 9, 2017
f2770dc
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 11, 2017
305c08c
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 11, 2017
aa2dfab
Merge branch 'master' of https://github.com/JMRI/JMRI
KenC57 Jan 15, 2017
6f51bba
After 4.6 this became broken due to changes in the properties file.
KenC57 Jan 15, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -2,8 +2,8 @@

import javax.swing.JButton;
import javax.swing.JPanel;
import jmri.jmrix.cmri.serial.nodeconfig.NodeConfigAction;
import jmri.jmrix.cmri.CMRISystemConnectionMemo;
import jmri.jmrix.cmri.serial.nodeconfig.NodeConfigAction;

/**
* Definition of objects to handle configuring a layout connection via a
Expand Down Expand Up @@ -38,7 +38,9 @@ public String name() {

public void loadDetails(JPanel details) {

b.addActionListener(new NodeConfigAction((CMRISystemConnectionMemo)adapter.getSystemConnectionMemo()));
if (adapter != null) {
b.addActionListener(new NodeConfigAction((CMRISystemConnectionMemo)adapter.getSystemConnectionMemo()));
Copy link
Member

Choose a reason for hiding this comment

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

If this runs with adapter==null, it won't crash right away, but it will behave oddly later on (because the listener won't be present). I'm much rather see a Large Amount of information being logged here so we have some chance of tracking down the real problem, which is the null adapter value.

}
if (!additionalItems.contains(b)) {
additionalItems.add(b);
}
Expand Down
Expand Up @@ -52,33 +52,35 @@ protected void getInstance(Object object) {
// though it would be good to fix it if you're working in this area
protected void extendElement(Element e) {
SerialTrafficController tc = ((CMRISystemConnectionMemo)adapter.getSystemConnectionMemo()).getTrafficController();
SerialNode node = (SerialNode) tc.getNode(0);
int index = 1;
while (node != null) {
// add node as an element
Element n = new Element("node");
n.setAttribute("name", "" + node.getNodeAddress());
e.addContent(n);
// add parameters to the node as needed
n.addContent(makeParameter("nodetype", "" + node.getNodeType()));
n.addContent(makeParameter("bitspercard", "" + node.getNumBitsPerCard()));
n.addContent(makeParameter("transmissiondelay", "" + node.getTransmissionDelay()));
n.addContent(makeParameter("num2lsearchlights", "" + node.getNum2LSearchLights()));
n.addContent(makeParameter("pulsewidth", "" + node.getPulseWidth()));
String value = "";
for (int i = 0; i < node.getLocSearchLightBits().length; i++) {
value = value + Integer.toHexString(node.getLocSearchLightBits()[i] & 0xF);
if (tc != null) {
Copy link
Member

Choose a reason for hiding this comment

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

I think that if this code runs with tc==null, you're going to store without the node definitions, thereby losing the node definitions. Is that really what you want?

SerialNode node = (SerialNode) tc.getNode(0);
int index = 1;
while (node != null) {
// add node as an element
Element n = new Element("node");
n.setAttribute("name", "" + node.getNodeAddress());
e.addContent(n);
// add parameters to the node as needed
n.addContent(makeParameter("nodetype", "" + node.getNodeType()));
n.addContent(makeParameter("bitspercard", "" + node.getNumBitsPerCard()));
n.addContent(makeParameter("transmissiondelay", "" + node.getTransmissionDelay()));
n.addContent(makeParameter("num2lsearchlights", "" + node.getNum2LSearchLights()));
n.addContent(makeParameter("pulsewidth", "" + node.getPulseWidth()));
String value = "";
for (int i = 0; i < node.getLocSearchLightBits().length; i++) {
value = value + Integer.toHexString(node.getLocSearchLightBits()[i] & 0xF);
}
n.addContent(makeParameter("locsearchlightbits", "" + value));
value = "";
for (int i = 0; i < node.getCardTypeLocation().length; i++) {
value = value + Integer.toHexString(node.getCardTypeLocation()[i] & 0xF);
}
n.addContent(makeParameter("cardtypelocation", "" + value));

// look for the next node
node = (SerialNode) tc.getNode(index);
index++;
}
n.addContent(makeParameter("locsearchlightbits", "" + value));
value = "";
for (int i = 0; i < node.getCardTypeLocation().length; i++) {
value = value + Integer.toHexString(node.getCardTypeLocation()[i] & 0xF);
}
n.addContent(makeParameter("cardtypelocation", "" + value));

// look for the next node
node = (SerialNode) tc.getNode(index);
index++;
}
}

Expand Down
Expand Up @@ -37,33 +37,35 @@ public ConnectionConfigXml() {
// though it would be good to fix it if you're working in this area
protected void extendElement(Element e) {
SerialTrafficController tc = ((CMRISystemConnectionMemo)adapter.getSystemConnectionMemo()).getTrafficController();
SerialNode node = (SerialNode) tc.getNode(0);
int index = 1;
while (node != null) {
// add node as an element
Element n = new Element("node");
n.setAttribute("name", "" + node.getNodeAddress());
e.addContent(n);
// add parameters to the node as needed
n.addContent(makeParameter("nodetype", "" + node.getNodeType()));
n.addContent(makeParameter("bitspercard", "" + node.getNumBitsPerCard()));
n.addContent(makeParameter("transmissiondelay", "" + node.getTransmissionDelay()));
n.addContent(makeParameter("num2lsearchlights", "" + node.getNum2LSearchLights()));
n.addContent(makeParameter("pulsewidth", "" + node.getPulseWidth()));
String value = "";
for (int i = 0; i < node.getLocSearchLightBits().length; i++) {
value = value + Integer.toHexString(node.getLocSearchLightBits()[i] & 0xF);
if (tc != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Similar to the other comment on this commit, I think this will write a configuration file that doesn't contain, hence loses for all time, the node information.

SerialNode node = (SerialNode) tc.getNode(0);
int index = 1;
while (node != null) {
// add node as an element
Element n = new Element("node");
n.setAttribute("name", "" + node.getNodeAddress());
e.addContent(n);
// add parameters to the node as needed
n.addContent(makeParameter("nodetype", "" + node.getNodeType()));
n.addContent(makeParameter("bitspercard", "" + node.getNumBitsPerCard()));
n.addContent(makeParameter("transmissiondelay", "" + node.getTransmissionDelay()));
n.addContent(makeParameter("num2lsearchlights", "" + node.getNum2LSearchLights()));
n.addContent(makeParameter("pulsewidth", "" + node.getPulseWidth()));
String value = "";
for (int i = 0; i < node.getLocSearchLightBits().length; i++) {
value = value + Integer.toHexString(node.getLocSearchLightBits()[i] & 0xF);
}
n.addContent(makeParameter("locsearchlightbits", "" + value));
value = "";
for (int i = 0; i < node.getCardTypeLocation().length; i++) {
value = value + Integer.toHexString(node.getCardTypeLocation()[i] & 0xF);
}
n.addContent(makeParameter("cardtypelocation", "" + value));

// look for the next node
node = (SerialNode) tc.getNode(index);
index++;
}
n.addContent(makeParameter("locsearchlightbits", "" + value));
value = "";
for (int i = 0; i < node.getCardTypeLocation().length; i++) {
value = value + Integer.toHexString(node.getCardTypeLocation()[i] & 0xF);
}
n.addContent(makeParameter("cardtypelocation", "" + value));

// look for the next node
node = (SerialNode) tc.getNode(index);
index++;
}
}

Expand Down
4 changes: 2 additions & 2 deletions java/src/jmri/jmrix/loconet/AbstractBoardProgPanel.java
Expand Up @@ -178,8 +178,8 @@ protected JPanel provideAddressing(String type) {
pane0.setLayout(new FlowLayout());
pane0.add(new JLabel(rb.getString("LABEL_UNIT_ADDRESS") + " "));
pane0.add(addrField);
readAllButton = new JToggleButton(java.util.ResourceBundle.getBundle("jmri/jmrit/symbolicprog/SymbolicProgBundle").getString("READ FROM ") + " " + type);
writeAllButton = new JToggleButton(java.util.ResourceBundle.getBundle("jmri/jmrit/symbolicprog/SymbolicProgBundle").getString("WRITE TO ") + " " + type);
readAllButton = new JToggleButton(java.util.ResourceBundle.getBundle("jmri/jmrit/symbolicprog/SymbolicProgBundle").getString("READ FROM") + " " + type);
writeAllButton = new JToggleButton(java.util.ResourceBundle.getBundle("jmri/jmrit/symbolicprog/SymbolicProgBundle").getString("WRITE TO") + " " + type);

// make both buttons a little bit bigger, with identical (preferred) sizes
// (width increased because some computers/displays trim the button text)
Expand Down