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

fix timestamp_no_ms() for 32bit #3292

Merged
merged 5 commits into from Jun 22, 2022
Merged

Conversation

dhoke4tdb
Copy link
Contributor

change for 32/64 bit consistency (corrects return of incorrect 32bit values)
(affects rtools40 32bit build)


TYPE: BUG
DESC: fix timestamp_no_ms() for 32bit

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #18759: timestamp_now_ms() incorrect in 32bit environment.

@dhoke4tdb dhoke4tdb marked this pull request as ready for review June 19, 2022 22:54
@@ -44,9 +44,9 @@ namespace tiledb::sm::utils::time {

uint64_t timestamp_now_ms() {
#ifdef _WIN32
struct _timeb tb;
struct __timeb64 tb;
memset(&tb, 0, sizeof(struct _timeb));
Copy link
Member

Choose a reason for hiding this comment

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

If you switch from struct _timeb to struct __timeb64 the memset() may need adjusting too.

Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

Noting for the record, from discussion: the original _ftime_s usage was OK in MSVC because (AFAICT) _32BIT_TIME_T is not defined by default. However, it did not work correctly with mingw.

@ihnorton ihnorton merged commit f82353c into dev Jun 22, 2022
@ihnorton ihnorton deleted the dlh/sc-18759-fix-timestamp-now-ms branch June 22, 2022 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants