[SERVER-20279] OperationShardVersion::get should return a pointer, not a mutable reference Created: 03/Sep/15 Updated: 06/Dec/22 Resolved: 17/Dec/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Sharding |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Andy Schwerin | Assignee: | [DO NOT USE] Backlog - Sharding EMEA |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Assigned Teams: |
Sharding EMEA
|
||||||||
| Participants: | |||||||||
| Description |
|
OperationShardVersion is a copyable type, and OperationShardVersion::get(txn) returns a reference to "txn"'s OSV. If that value is captured into a local value variable instead of a local reference variable by omitting the &, the compiler will not fail, and this could lead to hard to find bugs. OperationShardVersion::get should be changed to return a pointer, and its value should be captured into an OperationShardVersion* when it is captured at all, to avoid this risk. |
| Comments |
| Comment by Kaloian Manassiev [ 17/Dec/21 ] |
|
OperationShardingState is now non-copyable. |
| Comment by Spencer Brody (Inactive) [ 08/Sep/15 ] |
|
Note that this commit for |
| Comment by Randolph Tan [ 03/Sep/15 ] |
|
spencer will do. I also agree with this ticket that the method should be returning a pointer. |
| Comment by Spencer Brody (Inactive) [ 03/Sep/15 ] |
|
Making it noncopyable would be a trivial code change and could probably be done in conjunction with |
| Comment by Andy Schwerin [ 03/Sep/15 ] |
|
An alternative to having OSV::get return a pointer would be to make the OSV type non-copyable, so that you could not capture a copy by accident. |