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

Bugfix mamdatetime 32 bit windows #274

Merged
merged 9 commits into from
May 17, 2017
Merged

Conversation

garymolloy
Copy link
Contributor

Windows core on dateTime outside the range 1970-3000

Summary

Windows doesn't support time_t values outside the range 1970, jan 1, 00:00:00 - 3000, dec 31, 23:59:59 for 64bit gmtime and 1970, jan 1, 00:00:00 - 2038, jan 18, 23:59:59 for 32bit gmtime.

gmtime() is used by the OpenMAMA function utcTm() but due to it's date limitations (above) was the root cause of this issue.

Areas Affected

  • MAMAC
  • MAMACPP
  • MAMADOTNET
  • MAMAJNI
  • MAMDA
  • MAMDACPP
  • MAMDADOTNET
  • MAMDAJNI
  • Visual Studio
  • SCons
  • Unit Tests
  • Examples

Details

We have removed the compiler option '_USE_32BIT_TIME_T' when building 32bit and also replaced all calls to utcTm() with calls to Apache APR. This included making changes to the following dateTime functions:

  • mamaDateTime_getStructTmWithTz()
  • mamaDateTime_getAsString()
  • mamaDateTime_getTimeAsString()
  • mamaDateTime_getDateAsString()
  • mamaDateTime_getAsFormattedStringWithTz()
    This will of course create a dependency on Apache APR now for building.

Testing

10 new unittests have been added to cover dates outside the range of 1970 - 3000 (5 pre 1970 and 5 post 3000)

All unittests pass for 32bit builds

Win32 Bit was coring when dates outside the range of 1970-3000 were used.

This change will do a few things:
1) remove the compiler option _USE_32BIT_TIME_T when building 32bit
2) create a new dependency on Apache APR
3) replace calls to the utcTm() function with calls to Apache APR
4) new unittests added for testing dates outside of these ranges

Signed-off-by: Gary Molloy <gmolloy@velatt.com>
Signed-off-by: Gary Molloy <gmolloy@velatt.com>
Signed-off-by: Gary Molloy <gmolloy@velatt.com>
Signed-off-by: Gary Molloy <gmolloy@velatt.com>
@ghost
Copy link

ghost commented May 10, 2017

This fixes issue #271, right?

@ghost
Copy link

ghost commented May 10, 2017

Frank, should we expect 6.2.1 release with this fix merged in?

@fquinner
Copy link
Collaborator

@dfedorov-solace yes when this is merged, a 6.2.1 RC will be next.

…tu its in a different location

Signed-off-by: Gary Molloy <gmolloy@velatt.com>
@fquinner
Copy link
Collaborator

Yeah it's a funny one this - apache apr seem to have a pretty weird build setup. It has definitions for cmake, but these only seem to work on windows. When it works though it works in a standard way with the headers going into include/*

It then uses automake on linux if you compile it or package managers. Builds of this actually nest the include directory into a configurable location but defaults to include/apr-1. The ubuntu folks (which the travis builds are based on) actually use include/apr-1.0 just to be awkward. Luckily though they look like they provide a tool apr-1-config which allows you to resolve include paths after installation. That should be used in the build scripts on linux to resolve include paths.

The code should then use the header file names excluding the include subdirectory.

As Ubuntu installs APR into a different location I have added calls to apr-1-config
to give me the location of the include directory as well as the library name and compiler flags

Also updated .travis.yml to remove the soft link for finding the apr include directory

Signed-off-by: Gary Molloy <gmolloy@velatt.com>
Ubuntu was not parsing the apr libs correctly, RH/Centos was ok.

Signed-off-by: Gary Molloy <gmolloy@velatt.com>
On Ubuntu LINKFLAGS was causing the apr library to be statically linked and was
resulting in libmama.so not being created

Signed-off-by: Gary Molloy <gmolloy@velatt.com>
Decided to keep 'apr-1' listed in the LIBS as well as keeping the
recent changes.

Signed-off-by: Gary Molloy <gmolloy@velatt.com>
@fquinner
Copy link
Collaborator

Getting CI env ready for the new dependency here.

@fquinner
Copy link
Collaborator

OK linux and windows look ready to receive and build instructions updated - going to squash and merge this.

@fquinner fquinner merged commit 1fffe05 into next May 17, 2017
@fquinner fquinner added this to the OpenMAMA-6.2.1 milestone May 18, 2017
@fquinner fquinner added the bug label May 18, 2017
fquinner pushed a commit to fquinner/OpenMAMA that referenced this pull request Jul 21, 2017
Fixed windows core on datetime outside the range 1970-3000

Win32 Bit was coring when dates outside the range of 1970-3000 were used.

This change will do a few things:
1) remove the compiler option _USE_32BIT_TIME_T when building 32bit
2) create a new dependency on Apache APR
3) replace calls to the utcTm() function with calls to Apache APR
4) new unittests added for testing dates outside of these ranges

Signed-off-by: Gary Molloy <gmolloy@velatt.com>
fquinner pushed a commit to fquinner/OpenMAMA that referenced this pull request Jul 21, 2017
Fixed windows core on datetime outside the range 1970-3000

Win32 Bit was coring when dates outside the range of 1970-3000 were used.

This change will do a few things:
1) remove the compiler option _USE_32BIT_TIME_T when building 32bit
2) create a new dependency on Apache APR
3) replace calls to the utcTm() function with calls to Apache APR
4) new unittests added for testing dates outside of these ranges

Signed-off-by: Gary Molloy <gmolloy@velatt.com>
@fquinner fquinner deleted the bugfix-mamdatetime-32-bit-windows branch February 3, 2018 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants