Skip to content

Conversation

@jacobperron
Copy link

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.

  1. 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.

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>
Copy link
Collaborator

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM, it would be great to add a regression test

@jacobperron
Copy link
Author

Though there are no explicit tests covering these cases, the tests introduced in #48 caused both of these issues to surface, so at least it's covered.

@jacobperron jacobperron merged commit b6e4a9f into galactic-devel Nov 10, 2020
@jacobperron jacobperron deleted the jacob/fix_parameter_descriptors branch November 10, 2020 17:16
ivanpauno pushed a commit that referenced this pull request May 17, 2021
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>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 13, 2022
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>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 13, 2022
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>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 13, 2022
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>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 17, 2022
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants