From 163eaa77564706da6e9d2ed955ed7841e527f6f4 Mon Sep 17 00:00:00 2001
From: Jacob Perron <jacob@openrobotics.org>
Date: Mon, 9 Nov 2020 18:45:24 -0800
Subject: [PATCH] Fix issues with parameters descriptors

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>
---
 .../java/org/ros2/rcljava/node/NodeImpl.java  | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java
index 15622b81..4e649913 100644
--- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java
+++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java
@@ -139,6 +139,10 @@ public class NodeImpl implements Node {
   private Object parametersMutex;
 
   class ParameterAndDescriptor {
+    public ParameterAndDescriptor() {
+      this.parameter = new ParameterVariant();
+      this.descriptor = new rcl_interfaces.msg.ParameterDescriptor();
+    }
     public ParameterVariant parameter;
     public rcl_interfaces.msg.ParameterDescriptor descriptor;
   }
@@ -618,7 +622,7 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically(
     // them in the parameter list before setting them to the new value.  We
     // do this multi-stage thing so that all of the parameters are set, or
     // none of them.
-    List<String> parametersToDeclare = new ArrayList<String>();
+    List<ParameterVariant>parametersToDeclare = new ArrayList<ParameterVariant>();
     List<String> parametersToUndeclare = new ArrayList<String>();
     for (ParameterVariant parameter : parameters) {
       if (this.parameters.containsKey(parameter.getName())) {
@@ -627,7 +631,7 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically(
         }
       } else {
         if (this.allowUndeclaredParameters) {
-          parametersToDeclare.add(parameter.getName());
+          parametersToDeclare.add(parameter);
         } else {
           throw new ParameterNotDeclaredException(String.format("Parameter '%s' is not declared", parameter.getName()));
         }
@@ -636,7 +640,8 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically(
 
     // Check to make sure that a parameter isn't both trying to be declared
     // and undeclared simultaneously.
-    for (String name : parametersToDeclare) {
+    for (ParameterVariant parameter : parametersToDeclare) {
+      String name = parameter.getName();
       if (parametersToUndeclare.contains(name)) {
         throw new IllegalArgumentException(String.format("Cannot both declare and undeclare the same parameter name '%s'", name));
       }
@@ -651,8 +656,16 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically(
       this.parameters.remove(name);
     }
 
-    for (String name : parametersToDeclare) {
-      this.parameters.put(name, new ParameterAndDescriptor());
+    for (ParameterVariant parameter : parametersToDeclare) {
+      String name = parameter.getName();
+      ParameterAndDescriptor pandd = new ParameterAndDescriptor();
+      rcl_interfaces.msg.ParameterDescriptor descriptor =
+        new rcl_interfaces.msg.ParameterDescriptor()
+          .setName(name)
+          .setType(parameter.getType().getValue());
+      pandd.parameter = parameter;
+      pandd.descriptor = descriptor;
+      this.parameters.put(name, pandd);
     }
 
     for (ParameterVariant parameter : parameters) {