Skip to content

Fix: MdsShr->LibTimeToVMSTime: Avoid separate calls for current seconds and microseconds (#2467)#2468

Closed
merlea wants to merge 1 commit intoMDSplus:alphafrom
merlea:am-bugfix-libtimetovmstime
Closed

Fix: MdsShr->LibTimeToVMSTime: Avoid separate calls for current seconds and microseconds (#2467)#2468
merlea wants to merge 1 commit intoMDSplus:alphafrom
merlea:am-bugfix-libtimetovmstime

Conversation

@merlea
Copy link
Contributor

@merlea merlea commented Jun 28, 2022

See #2467.

In MdsShr->LibTimeToVMSTime, if time_in is not provided in input, then the current time is fetched using a single call to gettimeofday.

@merlea merlea force-pushed the am-bugfix-libtimetovmstime branch from c35d51b to d4499c9 Compare June 28, 2022 14:11
@joshStillerman
Copy link
Contributor

I am reviewing this now. Seems odd to me that the input to this routine is a time_t but I guess that is the way we wrote it, or I am confused about what a time_t is. Do you have time to discuss?

@joshStillerman
Copy link
Contributor

retest this please

@merlea
Copy link
Contributor Author

merlea commented Jun 29, 2022

I am not an expert at all about compilation on Windows and I don't know what is your build setup but my guess is that since gettimeofday is a POSIX extension, it might have to be added in an ad-hoc way on Windows systems.

Otherwise as the tv argument is a timeval and its tv_sec member is declared as a time_t, there should be no problem when passing that to get_tz_offset. So I suppose on your Windows build timeval.tv_sec is not declared as a time_t.

I then see 2 options:

  • we can introduce an intermediate variable which casts explicitely the tv.tv_sec value to time_t and pass its address to get_tz_offset (this is quite easy but does feel like a workaround only).
  • Fix the definitions of timeval and gettimeofday in the Windows build (this feels like a better option but I don't know how to do it).

@merlea
Copy link
Contributor Author

merlea commented Jun 29, 2022

I have looked further and it seems the definition of timeval and gettimeofday come from mingw directly. And according to this ticket, they will not change it. So we must accept and take into account that for Windows, timeval is [long,long] and not [time_t,suseconds_t].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants