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.

Sign up for GitHub

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

Jump to bottom

[C++] PollFlightInfo does not follow rule of 5 #39673

Closed
lidavidm opened this issue Jan 17, 2024 · 0 comments · Fixed by #39711
Closed

[C++] PollFlightInfo does not follow rule of 5 #39673

lidavidm opened this issue Jan 17, 2024 · 0 comments · Fixed by #39711
Assignees
@lidavidm
Labels
Component: C++ Type: enhancement
Milestone
15.0.1

Comments

@lidavidm
Copy link
Member

Describe the enhancement requested

We declare a copy constructor but not a move constructor, and so the class ends up without a move constructor (https://en.cppreference.com/w/cpp/language/rule_of_three)

Component(s)

C++
@lidavidm lidavidm mentioned this issue Jan 19, 2024
Merged
lidavidm added a commit that referenced this issue Jan 22, 2024
@lidavidm
GH-39690: [C++][FlightRPC] Fix nullptr dereference in PollInfo (#39711)
3b73f43 
### Rationale for this change

The current implementation is a bit painful to use due to the lack of a move constructor.

### What changes are included in this PR?

- Fix a crash in PollInfo with a nullptr FlightInfo.
- Declare all necessary constructors (https://en.cppreference.com/w/cpp/language/rule_of_three)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, this adds new copy constructors.

* Closes: #39673.
* Closes: #39690

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 16.0.0 milestone Jan 22, 2024
@lidavidm lidavidm self-assigned this Jan 22, 2024
@pitrou pitrou modified the milestones: 16.0.0, 15.0.1 Feb 14, 2024
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
@lidavidm @dgreiss
apacheGH-39690: [C++][FlightRPC] Fix nullptr dereference in PollInfo (a…
90c636c 
…pache#39711)

### Rationale for this change

The current implementation is a bit painful to use due to the lack of a move constructor.

### What changes are included in this PR?

- Fix a crash in PollInfo with a nullptr FlightInfo.
- Declare all necessary constructors (https://en.cppreference.com/w/cpp/language/rule_of_three)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, this adds new copy constructors.

* Closes: apache#39673.
* Closes: apache#39690

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
raulcd pushed a commit that referenced this issue Feb 20, 2024
@lidavidm @raulcd
GH-39690: [C++][FlightRPC] Fix nullptr dereference in PollInfo (#39711)
87eae3d 
### Rationale for this change

The current implementation is a bit painful to use due to the lack of a move constructor.

### What changes are included in this PR?

- Fix a crash in PollInfo with a nullptr FlightInfo.
- Declare all necessary constructors (https://en.cppreference.com/w/cpp/language/rule_of_three)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, this adds new copy constructors.

* Closes: #39673.
* Closes: #39690

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
zanmato1984 pushed a commit to zanmato1984/arrow that referenced this issue Feb 28, 2024
@lidavidm @zanmato1984
apacheGH-39690: [C++][FlightRPC] Fix nullptr dereference in PollInfo (a…
830b45a 
…pache#39711)

### Rationale for this change

The current implementation is a bit painful to use due to the lack of a move constructor.

### What changes are included in this PR?

- Fix a crash in PollInfo with a nullptr FlightInfo.
- Declare all necessary constructors (https://en.cppreference.com/w/cpp/language/rule_of_three)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, this adds new copy constructors.

* Closes: apache#39673.
* Closes: apache#39690

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
thisisnic pushed a commit to thisisnic/arrow that referenced this issue Mar 8, 2024
@lidavidm @thisisnic
apacheGH-39690: [C++][FlightRPC] Fix nullptr dereference in PollInfo (a…
d2933bc 
…pache#39711)

### Rationale for this change

The current implementation is a bit painful to use due to the lack of a move constructor.

### What changes are included in this PR?

- Fix a crash in PollInfo with a nullptr FlightInfo.
- Declare all necessary constructors (https://en.cppreference.com/w/cpp/language/rule_of_three)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, this adds new copy constructors.

* Closes: apache#39673.
* Closes: apache#39690

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lidavidm lidavidm

Labels
Component: C++ Type: enhancement
Projects
None yet
Milestone
15.0.1
Development

Successfully merging a pull request may close this issue.

GH-39690: [C++][FlightRPC] Fix nullptr dereference in PollInfo lidavidm/arrow
2 participants
@lidavidm @pitrou