[CXX-1553] Wrong ctor call to b_timestamp Created: 16/Apr/18  Updated: 28/Oct/23  Resolved: 17/Apr/18

Status: Closed
Project: C++ Driver
Component/s: BSON
Affects Version/s: 3.2.0
Fix Version/s: 3.3.0-rc0

Type: Bug Priority: Minor - P4
Reporter: Hugo Lefebvre [X] Assignee: Andrew Morrow (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by CXX-1700 b_timestamp fields inversion Closed
Related
related to CXX-1700 b_timestamp fields inversion Closed
is related to CXX-1555 Reimplement b_timestamp type with tim... Backlog
Backwards Compatibility: Fully Compatible

 Description   

In BSON document element class, the get_timestamp() accessor create a new b_timestamp object but with parameters timestamp and increment in the wrong order.



 Comments   
Comment by Githook User [ 17/Apr/18 ]

Author:

{'name': 'Hugo Lefebvre', 'email': 'lefebvre@systran.fr', 'username': 'AsT4re'}

Message: CXX-1553 Fix wrong call to b_timestamp ctor
Branch: master
https://github.com/mongodb/mongo-cxx-driver/commit/9d887e2150ce7f85adcc5ad2311dc71fac6f3442

Comment by Hugo Lefebvre [X] [ 16/Apr/18 ]

Indeed, just reversing the parameters on the constructor call seems to be only a quick fix.
I have also opened a PR on github for this quick fix.
Kind regards,
Hugo.

Comment by Andrew Morrow (Inactive) [ 16/Apr/18 ]

AsTare - Thanks for the bug report. Looks like you are right. The obvious fix would be to just reverse the field order in the constructor call inside element.cpp. But it is weird that the fields are declared in the order that they are in b_timestamp - it looks backwards to me. Unfortunately, swapping the field order there isn't an option, since that could silently change the meaning of working code. We should probably make the obvious fix now and backport to a 3.2.1. Long term, we should think about a better structure for b_timestamp that doesn't admit this sort of reversal, as this is the second field order bug we have had with b_timestamp: see CXX-1367.

CC ajdavis - I propose we make the obvious fix on master, backport for a 3.2.1, and file a follow-up ticket to make it hard to use b_timestamp wrong. Given that the non-ordinal field has semantics as seconds since the unix epoch, and that std::chrono::seconds has an explicit constructor, perhaps giving a future b_timestamp type a constructor where the first argument took a std::chrono::seconds would fix this?

Generated at Wed Feb 07 22:03:02 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.