What could possibly go wrong in existing apps, I wonder.
Changing the signedness of such an important variable to remove ONE warning at the risk of breaking NUMEROUS systems is insane.
Dont tell me everyone uses 64 bits. Many systems will still use 32 bits for legacy reasons. Now if they update, you give them a year 2038 problem that was not there before.
This is forcing everyone of those to migrate to 64 bit timestamps, which use doubles the field size.
DO NOT do this. Remove 32 bits support entirely, Changing the signedness of time is a huge risk that no one on this list can evaluate.
Sebastien On 04/11/2024 14:32, Guiding Li wrote:
Hi all: We decide change 'time_t' from unsigned type to signed type in PR: https://github.com/apache/nuttx/pull/14460 Because when compile some POSIX library, there always be a warning on comparison between time_t and zero. For example: The following code will generate warnings: auto now = time(nullptr); auto last_active_time = GetEventService(self->ctx_)->getActiveTime(); if (last_active_time + 60 * 1000 / 1000 <= now) { src/ams/../controller/controller_timer.h: In lambda function: src/ams/../controller/controller_timer.h:117:57: warning: comparison of integer expressions of different signedness: 'long long int' and 'long long unsigned int' [-Wsign-compare] 117 | if (last_active_time + 60 * 1000 / 1000 <= now) { And we can find an reference on the official website: https://www.gnu.org/software/libc/manual/html_node/Time-Types.html On POSIX-conformant systems, time_t is an integer type. The comparation of the merits and shortcomings: Advantage: For the most POSIX applications they assume the time_t as signed and do compare with 0. The code will become more compatible after this modification. Disadvantage: None. If there is any question about this, please let me know. Thanks, Guiding
