Skip to content

Commit e4ebbdb

Browse files
committed
Fix issues with parameters descriptors (osrf#50)
This fixes two issues: 1. The default constructor for ParameterAndDescriptor initializes it's members to null. This can cause a segfault when trying to access a parameters descriptor if undeclared parameters are allowed and it has been set without a descriptor (e.g. using setParameters()). This change defines the constructor for ParameterAndDescriptor so that we don't hit any null values. 2. Parameters that are not declared and set with setParameters() do not have their descriptors set to the correct value. This change makes sure that the descriptor is initialized with the values of the parameter being set. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
1 parent a65edc5 commit e4ebbdb

File tree

1 file changed

+18
-5
lines changed

1 file changed

+18
-5
lines changed

rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ public class NodeImpl implements Node {
138138
private Object parametersMutex;
139139

140140
class ParameterAndDescriptor {
141+
public ParameterAndDescriptor() {
142+
this.parameter = new ParameterVariant();
143+
this.descriptor = new rcl_interfaces.msg.ParameterDescriptor();
144+
}
141145
public ParameterVariant parameter;
142146
public rcl_interfaces.msg.ParameterDescriptor descriptor;
143147
}
@@ -633,7 +637,7 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically(
633637
// them in the parameter list before setting them to the new value. We
634638
// do this multi-stage thing so that all of the parameters are set, or
635639
// none of them.
636-
List<String> parametersToDeclare = new ArrayList<String>();
640+
List<ParameterVariant>parametersToDeclare = new ArrayList<ParameterVariant>();
637641
List<String> parametersToUndeclare = new ArrayList<String>();
638642
for (ParameterVariant parameter : parameters) {
639643
if (this.parameters.containsKey(parameter.getName())) {
@@ -642,7 +646,7 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically(
642646
}
643647
} else {
644648
if (this.allowUndeclaredParameters) {
645-
parametersToDeclare.add(parameter.getName());
649+
parametersToDeclare.add(parameter);
646650
} else {
647651
throw new ParameterNotDeclaredException(String.format("Parameter '%s' is not declared", parameter.getName()));
648652
}
@@ -651,7 +655,8 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically(
651655

652656
// Check to make sure that a parameter isn't both trying to be declared
653657
// and undeclared simultaneously.
654-
for (String name : parametersToDeclare) {
658+
for (ParameterVariant parameter : parametersToDeclare) {
659+
String name = parameter.getName();
655660
if (parametersToUndeclare.contains(name)) {
656661
throw new IllegalArgumentException(String.format("Cannot both declare and undeclare the same parameter name '%s'", name));
657662
}
@@ -666,8 +671,16 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically(
666671
this.parameters.remove(name);
667672
}
668673

669-
for (String name : parametersToDeclare) {
670-
this.parameters.put(name, new ParameterAndDescriptor());
674+
for (ParameterVariant parameter : parametersToDeclare) {
675+
String name = parameter.getName();
676+
ParameterAndDescriptor pandd = new ParameterAndDescriptor();
677+
rcl_interfaces.msg.ParameterDescriptor descriptor =
678+
new rcl_interfaces.msg.ParameterDescriptor()
679+
.setName(name)
680+
.setType(parameter.getType().getValue());
681+
pandd.parameter = parameter;
682+
pandd.descriptor = descriptor;
683+
this.parameters.put(name, pandd);
671684
}
672685

673686
for (ParameterVariant parameter : parameters) {

0 commit comments

Comments
 (0)