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

[20067] ParticipantDiscoveryInfo use-after-move #4043

Closed
1 task done
mtheall opened this issue Nov 23, 2023 · 2 comments
Closed
1 task done

[20067] ParticipantDiscoveryInfo use-after-move #4043

mtheall opened this issue Nov 23, 2023 · 2 comments
Labels

Comments

@mtheall
Copy link
Contributor

mtheall commented Nov 23, 2023

Is there an already existing issue for this?

  • I have searched the existing issues

Expected behavior

N/A

Current behavior

N/A

Steps to reproduce

N/A

Fast DDS version/commit

Introduced in f15c7c7

Platform/Architecture

Other. Please specify in Additional context section.

Transport layer

Default configuration, UDPv4 & SHM

Additional context

DomainParticipantImpl::MyRTPSParticipantListener::onParticipantDiscovery()
info is std::move'd, and then conditionally used after that move.

I understand that moving this object is actually a no-op, since its only members are an enum and a const reference. Perhaps for future-proofing or at least adhering to move semantics, this should probably be changed to only move once.

XML configuration file

No response

Relevant log output

No response

Network traffic capture

No response

@mtheall mtheall added the triage Issue pending classification label Nov 23, 2023
@mtheall mtheall changed the title DomainParticipantImpl use-after-move ParticipantDiscoveryInfo use-after-move Nov 23, 2023
@MiguelCompany
Copy link
Member

On the one hand, the std::move in this case will be transformed into a static_cast so yes, they are a no-op.
On the other hand, the overload of on_participant_discovery not receiving should_be_ignored will be removed in the next major version, so this will not be an issue in the future.

I just created #4078 so we don't forget.

@MiguelCompany MiguelCompany added wontfix and removed triage Issue pending classification labels Dec 1, 2023
@MiguelCompany MiguelCompany changed the title ParticipantDiscoveryInfo use-after-move [20067] ParticipantDiscoveryInfo use-after-move Dec 1, 2023
@MiguelCompany
Copy link
Member

Closing since #4078 has been merged

@MiguelCompany MiguelCompany closed this as not planned Won't fix, can't repro, duplicate, stale Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants