Copilot commented on code in PR #18840:
URL: https://github.com/apache/nuttx/pull/18840#discussion_r3177835667
##########
drivers/rpmsg/rpmsg_ping.c:
##########
@@ -189,11 +189,7 @@ static void rpmsg_ping_logout(FAR const char *s, clock_t
value)
perf_convert(value, &ts);
-#ifdef CONFIG_SYSTEM_TIME64
syslog(LOG_EMERG, "%s: %" PRIu64 " s, %ld ns\n", s, ts.tv_sec, ts.tv_nsec);
Review Comment:
This log format is now always `%` `PRIu64`, but NuttX's `syslog`/`vsnprintf`
path only pulls `long long` arguments when `CONFIG_HAVE_LONG_LONG` is enabled.
On toolchains without that support, `ts.tv_sec` will be printed incorrectly
instead of falling back to the 32-bit format that was previously used.
##########
fs/procfs/fs_procfsuptime.c:
##########
@@ -250,13 +246,8 @@ static ssize_t uptime_read(FAR struct file *filep, FAR
char *buffer,
/* Convert the seconds + hundredths of seconds to a string */
-#ifdef CONFIG_SYSTEM_TIME64
linesize = procfs_snprintf(attr->line, UPTIME_LINELEN,
"%7" PRIu64 ".%02u\n", sec, csec);
Review Comment:
`PRIu64` expands to `%llu` here, but NuttX's `vsnprintf` only fetches `long
long` arguments when `CONFIG_HAVE_LONG_LONG` is enabled. On toolchains where
that option is unset, this will mis-format or truncate 64-bit uptime values;
the old `CONFIG_SYSTEM_TIME64` guard avoided emitting `%llu` on those builds.
##########
include/inttypes.h:
##########
@@ -338,36 +338,6 @@
#define SCNxOFF SCNx32
#endif
Review Comment:
This deletes the `PRI*TM` / `SCN*TM` aliases outright instead of redefining
them to the 64-bit forms. Any code that formats or scans `time_t` via these
public macros will now fail to compile, even though the PR description says
those macros should continue to exist and map to 64-bit specifiers.
##########
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 procfs format strings now unconditionally use `PRIu64`/`%llu`, but
NuttX's printf backend only reads `long long` arguments when
`CONFIG_HAVE_LONG_LONG` is enabled. On builds without that support, the logged
power-management times/percentages will be truncated or corrupted.
##########
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
Review Comment:
This replacement comment is now a truncated sentence that no longer
documents the `clockid_t` bit layout correctly. It looks like part of an
unrelated tickless-time comment was spliced into this block, so the public
header documentation is misleading as written.
--
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]
