Copilot commented on code in PR #18842:
URL: https://github.com/apache/nuttx/pull/18842#discussion_r3177873970
##########
include/nuttx/clock.h:
##########
@@ -77,15 +77,7 @@
# define __HAVE_KERNEL_GLOBALS 1
#endif
-/* If CONFIG_SYSTEM_TIME64 is selected and the CPU supports long long types,
- * then a 64-bit system time will be used.
- */
-
-#ifndef CONFIG_HAVE_LONG_LONG
-# undef CONFIG_SYSTEM_TIME64
-#endif
-
-/* The following are the bit fields of the clockid_t
+/* If CONFIG_SCHED_TICKLESS is not defined, then the interrupt interval of
* bit 0~2: the clock type
* CLOCK_REALTIME - 0
* CLOCK_MONOTONIC - 1
Review Comment:
This header comment was accidentally mangled when the old
`CONFIG_SYSTEM_TIME64` block was removed. It now starts with `If
CONFIG_SCHED_TICKLESS is not defined, then the interrupt interval of` but the
rest of the block documents `clockid_t` bit fields, so the resulting
public-header documentation is misleading.
##########
drivers/power/pm/pm_procfs.c:
##########
@@ -50,19 +50,11 @@
#define PFHDR "CALLBACKS IDLE STANDBY SLEEP\n"
#define WAHDR "DOMAIN%-2d STATE COUNT
TIME\n"
-#ifdef CONFIG_SYSTEM_TIME64
-# define STFMT "%-18s %8" PRIu64 "s %3" PRIu64 "%% %8" PRIu64 "s %3" \
- PRIu64 "%% %8" PRIu64 "s %3" PRIu64 "%%\n"
-# define PFFMT "%-18p %8" PRIu64 "s %3" PRIu64 "%% %8" PRIu64 "s %3" \
- PRIu64 "%% %8" PRIu64 "s %3" PRIu64 "%%\n"
-# define WAFMT "%-25s %-14s %-14" PRIu32 " %" PRIu64 "s\n"
-#else
-# define STFMT "%-18s %8" PRIu32 "s %3" PRIu32 "%% %8" PRIu32 "s %3" \
- PRIu32 "%% %8" PRIu32 "s %3" PRIu32 "%%\n"
-# define PFFMT "%-18p %8" PRIu32 "s %3" PRIu32 "%% %8" PRIu32 "s %3" \
- PRIu32 "%% %8" PRIu32 "s %3" PRIu32 "%%\n"
-# define WAFMT "%-25s %-14s %-14" PRIu32 " %" PRIu32 "s\n"
-#endif
+#define STFMT "%-18s %8" PRIu64 "s %3" PRIu64 "%% %8" PRIu64 "s %3" \
+ PRIu64 "%% %8" PRIu64 "s %3" PRIu64 "%%\n"
+#define PFFMT "%-18p %8" PRIu64 "s %3" PRIu64 "%% %8" PRIu64 "s %3" \
+ PRIu64 "%% %8" PRIu64 "s %3" PRIu64 "%%\n"
+#define WAFMT "%-25s %-14s %-14" PRIu32 " %" PRIu64 "s\n"
Review Comment:
These format strings still use `PRIu64`, but the values passed here
(`time_t`/`tv_sec`) are now `int64_t`. On LP64 targets this becomes `%lu` with
a signed `long` argument, which is a `printf` type mismatch and can produce
incorrect output once negative sentinels or deltas show up. Please switch these
specifiers to the signed `time_t` format.
##########
include/inttypes.h:
##########
@@ -338,36 +338,6 @@
#define SCNxOFF SCNx32
#endif
-/* time_t */
-
-#ifdef CONFIG_SYSTEM_TIME64
-#define PRIdTM PRId64
-#define PRIiTM PRIi64
-#define PRIoTM PRIo64
-#define PRIuTM PRIu64
-#define PRIxTM PRIx64
-#define PRIXTM PRIX64
-
-#define SCNdTM SCNd64
-#define SCNiTM SCNi64
-#define SCNoTM SCNo64
-#define SCNuTM SCNu64
-#define SCNxTM SCNx64
-#else
-#define PRIdTM PRId32
-#define PRIiTM PRIi32
-#define PRIoTM PRIo32
-#define PRIuTM PRIu32
-#define PRIxTM PRIx32
-#define PRIXTM PRIX32
-
-#define SCNdTM SCNd32
-#define SCNiTM SCNi32
-#define SCNoTM SCNo32
-#define SCNuTM SCNu32
-#define SCNxTM SCNx32
-#endif
-
/****************************************************************************
Review Comment:
This hunk removes the `PRI*TM` / `SCN*TM` aliases from a public header,
which is a source-incompatible API change for any out-of-tree code formatting
or scanning `time_t` with the NuttX helpers. Since `time_t` still exists and is
now unconditionally 64-bit signed, keeping these macros mapped to the 64-bit
signed forms would preserve compatibility.
##########
arch/avr/src/avrdx/avrdx_timerisr_tickless_alarm.c:
##########
@@ -301,8 +301,7 @@ static void avrdx_check_alarm_expired(uint8_t context)
/* Note about data types - struct timespec is defined
* in include/time.h, tv_sec is of type time_t which is defined
- * in include/sys/types.h as uint32_t or uint64_t based
- * on CONFIG_SYSTEM_TIME64
+ * in include/sys/types.h as uint64_t.
Review Comment:
The updated comment says `tv_sec` is `uint64_t`, but this PR changes
`time_t` to `int64_t`. Leaving the old signedness here is misleading for anyone
touching the tickless alarm math in this file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
