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

libs/libc/time: mktime normalize struct tm #7440

Merged
merged 1 commit into from Oct 27, 2022

Conversation

pkarashchenko
Copy link
Contributor

@pkarashchenko pkarashchenko commented Oct 26, 2022

Summary

According to https://pubs.opengroup.org/onlinepubs/9699919799/functions/mktime.html the:

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>](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/time.h.html) entry; the final value
of tm_mday shall not be set until tm_mon and tm_year are determined.

Impact

Fix operation of mktime in case if CONFIG_LIBC_LOCALTIME is not selected

Testing

Tested few cases with struct tm normalization, but more testing by others are welcomed

Signed-off-by: Petro Karashchenko <petro.karashchenko@gmail.com>
@pkarashchenko pkarashchenko marked this pull request as ready for review October 27, 2022 08:17
@pkarashchenko pkarashchenko requested review from acassis, yamt and xiaoxiang781216 and removed request for yamt October 27, 2022 08:17
@pkarashchenko pkarashchenko added the Standards NuttX application interfaces must compy to standards label Oct 27, 2022
@pkarashchenko pkarashchenko linked an issue Oct 27, 2022 that may be closed by this pull request
@xiaoxiang781216 xiaoxiang781216 merged commit fdff92f into apache:master Oct 27, 2022
@fjpanag
Copy link
Contributor

fjpanag commented Oct 27, 2022

@pkarashchenko This commit made our internal tests fail.

Specifically, I am running the Lua v5.2 test suite and it gives me the following message:

/mnt/sdcard0/files.lua:572: assertion failed!

This is the offending line:

local t = os.time()
D = os.date("*t", t)

assert(os.time(D) == t)  -- This line here fails!

I can track this down, and get the actual NuttX issue hidden behind the Lua libraries, but unfortunately due to a national holiday, this will have to wait till Monday.

Unless you can troubleshoot this sooner...

@pkarashchenko
Copy link
Contributor Author

pkarashchenko commented Oct 27, 2022

Ok. Let me take a look tomorrow. If I will not be able to narrow down the issue, then we will revert the change until I find a root cause.
Also maybe you can share the time_t value used during the test?

@pkarashchenko
Copy link
Contributor Author

@fjpanag I found the root cause. It is fixed in #7455

@jerpelea jerpelea added this to To-Add in Release Notes - 12.0.0 Dec 29, 2022
@jerpelea jerpelea moved this from To-Add to Added in Release Notes - 12.0.0 Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Standards NuttX application interfaces must compy to standards
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

mktime() does not "normalize" struct tm argument
3 participants