Necessary changes were made in the aws-sdk-cpp to support AIX OS. #3714
Conversation
| { | ||
| time_t timegm (struct tm *tm) | ||
| { | ||
| static const int msum [2][12] = { |
There was a problem hiding this comment.
while this pattern may be common for C. lets use C++ patterns. lets use anonymous name spacing for static. additionally lets use C++ arrays not C arrays.
namespace {
const Aws::Array<Aws::Array<int, 12>, 2> msum{{
{{ 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334}}, /* normal years */
{{ 0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335}} /* leap years */
}};
const Aws::Array<Aws::Array<int, 12>, 2> msummlen{{
{{ 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}},
{{ 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}}
}};
const int tmstr_year= 1900; /* base of 'tm_year' in 'struct tm' */
const int epoch_year= 1970; /* unix timestamp epoch */
const int base_year= -9999; /* start of a 400-year period: used to be 1601,
but this allows larger range (in 64 bit)
mind you, this is proleptic Gregorian */
}There was a problem hiding this comment.
As mentioned in the previous comment, I will not use any C++ constructs.
| @@ -0,0 +1,80 @@ | |||
| /* TimeHelper.cpp | |||
There was a problem hiding this comment.
did you copy this implementation from here? i see it is under The Unlicense, which doesnt require attribution, but lets add attribution just so the original author gets credit. something along the lines of how we do it in cjson but indicating where this came from.
There was a problem hiding this comment.
I will address this in the next commit.
| { | ||
| namespace Time | ||
| { | ||
| time_t timegm (struct tm *tm) |
There was a problem hiding this comment.
im confused here are you trying to implement timegm for SDK interface or are you trying to implement the interface for linux, and then subsequently use the linux implementation?
There was a problem hiding this comment.
I will implement timegm() as a standalone C-style API, so I will not use any C++ constructs.
| int isleapyear; | ||
| long long t; | ||
|
|
||
| if (!tm) return -1; |
There was a problem hiding this comment.
from the timegm documentation
On success, these functions return the calendar time (seconds since the Epoch), expressed as a value of type time_t. On error, they return the value (time_t) -1 and set errno to indicate the error.
dont you need to set errno here? additionally dont you need to set it everywhere you are returning with err?
| tm->tm_mday<1 || tm->tm_mday>mlen[isleapyear][tm->tm_mon] || | ||
| tm->tm_hour<0 || tm->tm_hour>23 || | ||
| tm->tm_min<0 || tm->tm_min>59 || | ||
| tm->tm_min<0 || tm->tm_sec>59) return -1; |
There was a problem hiding this comment.
isnt this a typo? shouldnt this be
tm->tm_sec<0| t += msum[isleapyear][tm->tm_mon]; | ||
| t += tm->tm_mday-1; | ||
|
|
||
| tm->tm_wday = (t+4)%7;/* 0..6=Sun..Sat; adding 4 to adjust 1970.01.01.=Thursday; */ |
There was a problem hiding this comment.
Why are you modifying input? In a conversion function i think this should be const?
There was a problem hiding this comment.
I was going through man page of mktime() as mktime and timegm functionality is same except they differ only on how they interpret the input parameter.
mktime interprets the input as a local time, while timegm interprets it as Coordinated Universal Time
(UTC)
So as per the man page:
https://man7.org/linux/man-pages/man3/mktime.3p.html
Upon successful completion, the values of the tm_wday and tm_yday
components of the structure shall be set appropriately, and the
other components shall be set to represent the specified time
since the Epoch, but with their values forced to the ranges
indicated in the <time.h> entry; the final value of tm_mday shall
not be set until tm_mon and tm_year are determined.
| # build the sdk targets | ||
| project("aws-cpp-sdk-all" VERSION "${PROJECT_VERSION}" LANGUAGES CXX) | ||
|
|
||
| if(NOT CMAKE_SYSTEM_NAME STREQUAL "AIX") |
There was a problem hiding this comment.
why does this need to be moved?
There was a problem hiding this comment.
CMake response file support is not enabled on AIX, so it must be blocked.
To block it on AIX, we need to use CMAKE_SYSTEM_NAME, which is evaluated after the project() call.
| #include <climits> | ||
| #endif | ||
| #ifdef _AIX | ||
| #define NAME_MAX 255 |
There was a problem hiding this comment.
what is this define used for?
There was a problem hiding this comment.
NAME_MAX is used in the same file at line 281.
On AIX, this macro is undefined. So need to define it with respect to AIX.
2 participants
*Issue #3440 *
Description of changes:
Check all that applies:
Require to add support for AIX platform.
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.