On Mon, Jul 1, 2024 at 12:07 AM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > On 30/06/2024 22:16, Rosen Penev wrote: > > On Sun, Jun 30, 2024 at 1:50 AM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > >> > >> Hi Rosen, > >> > >> Apologies, I hadn't seen this patch, it was filed in my spam folder for some reason. > >> > >> On 10/06/2024 23:23, Rosen Penev wrote: > >>> musl since version 1.2.0 uses 64-bit time_t even on 32-bit. Cast to > >>> 64-bit for compatibility. > >>> > >>> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx> > >>> --- > >>> utils/cec-compliance/cec-compliance.cpp | 3 ++- > >>> utils/cec-compliance/cec-test-adapter.cpp | 5 +++-- > >>> utils/cec-ctl/cec-ctl.cpp | 19 ++++++++++--------- > >>> utils/cec-follower/cec-follower.cpp | 3 ++- > >>> utils/cec-follower/cec-processing.cpp | 3 ++- > >>> utils/keytable/keytable.c | 1 + > >>> utils/libv4l2util/v4l2_driver.c | 7 ++++--- > >>> utils/v4l2-tracer/retrace.cpp | 2 +- > >>> 8 files changed, 25 insertions(+), 18 deletions(-) > >>> > >>> diff --git a/utils/cec-compliance/cec-compliance.cpp b/utils/cec-compliance/cec-compliance.cpp > >>> index 8075e1d6..df633a33 100644 > >>> --- a/utils/cec-compliance/cec-compliance.cpp > >>> +++ b/utils/cec-compliance/cec-compliance.cpp > >>> @@ -3,6 +3,7 @@ > >>> * Copyright 2016 Cisco Systems, Inc. and/or its affiliates. All rights reserved. > >>> */ > >>> > >>> +#include <cinttypes> > >>> #include <sstream> > >>> > >>> #include <fcntl.h> > >>> @@ -279,7 +280,7 @@ static std::string ts2s(__u64 ts) > >>> t = res.tv_sec; > >>> s = ctime(&t); > >>> s = s.substr(0, s.length() - 6); > >>> - sprintf(buf, "%03lu", res.tv_usec / 1000); > >>> + sprintf(buf, "%03" PRIu64, (uint64_t)res.tv_usec / 1000); > >> > >> This doesn't make sense: why is res.tv_usec cast to a uint64_t? > >> Is that field really a 64 bit value under musl? But in any case, tv_usec is > >> limited to 999999, so if it is a 64 bit value under musl, then just cast it > >> to unsigned long and leave the %03lu as-is. > > That's suseconds_t which is 64-bit. I felt it would be safer to cast > > to 64-bit, but OK. > >> > >> The commit log is certainly insufficient since a lot of the changes in this > >> patch have nothing to do with time_t but with tv_n/usec. > > include/alltypes.h.in:TYPEDEF _Int64 suseconds_t; : <-- from musl source. > >> > >> Why use PRIu64? That's not explained in the commit log either. I had to look > >> that up, as I've never seen it used before. > >> > >> As I understand it, musl doesn't support %llu in some cases? Looking at: > >> > >> https://github.com/cloudius-systems/musl/blob/master/include/inttypes.h > >> > >> it appears that PRIu64 can either be lu or llu. lu is used if UINTPTR_MAX == UINT64_MAX. > >> > >> But if that is needed, then I expect to see a single patch converting every > >> %lld and %llu in v4l-utils to use PRId/u64. Here it is only done for some > >> time-related code. > > not exactly. kernel types use long long always. musl switches between both. > > Sorry, I don't follow. Do you mean that a __u64 will always be 64 bit and can > be formatted as %llu in musl? But types like suseconds_t can be either long or > long long? __u64 is always long long, yes. > > Or do you mean something else? > > I would really prefer to avoid the use of PRIu64, and if casts to (__u64) would > do that trick, than that sounds like an acceptable solution. Cleaner. Will rework. > > Regards, > > Hans > > >> > >> Please don't mix changing to PRId/u64 with these time changes. If you believe > >> that is needed, then do that as a final patch for all of v4l-utils. But the > >> commit log should be very clear explaining why it is needed. > >> > >> Regards, > >> > >> Hans > >> > >>> return s + "." + buf; > >>> } > >>> > >>> diff --git a/utils/cec-compliance/cec-test-adapter.cpp b/utils/cec-compliance/cec-test-adapter.cpp > >>> index 08c856af..7a80d17b 100644 > >>> --- a/utils/cec-compliance/cec-test-adapter.cpp > >>> +++ b/utils/cec-compliance/cec-test-adapter.cpp > >>> @@ -4,6 +4,7 @@ > >>> */ > >>> > >>> #include <cerrno> > >>> +#include <cinttypes> > >>> #include <ctime> > >>> #include <string> > >>> > >>> @@ -1276,9 +1277,9 @@ static int testLostMsgs(struct node *node) > >>> printf("\t\tReceived messages: %d of which %d were CEC_MSG_CEC_VERSION\n", > >>> pending_rx_msgs, pending_rx_cec_version_msgs); > >>> if (pending_quick_msgs < pending_msgs) > >>> - printf("\t\tReceived %d messages immediately, and %d over %ld seconds\n", > >>> + printf("\t\tReceived %d messages immediately, and %d over %" PRIu64 " seconds\n", > >>> pending_quick_msgs, pending_msgs - pending_quick_msgs, > >>> - time(nullptr) - start); > >>> + (uint64_t)time(nullptr) - start); > >>> } > >>> print_sfts(sft[1][1], "SFTs for repeating messages (>= 7)"); > >>> print_sfts(sft[1][0], "SFTs for repeating remote messages (>= 7)"); > >>> diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp > >>> index 69aeb8cd..a2ffcb2b 100644 > >>> --- a/utils/cec-ctl/cec-ctl.cpp > >>> +++ b/utils/cec-ctl/cec-ctl.cpp > >>> @@ -6,6 +6,7 @@ > >>> #include <algorithm> > >>> #include <cctype> > >>> #include <cerrno> > >>> +#include <cinttypes> > >>> #include <cstring> > >>> #include <ctime> > >>> #include <map> > >>> @@ -416,7 +417,7 @@ std::string ts2s(__u64 ts) > >>> strftime(buf, sizeof(buf), "%a %b %e %T.000000", &tm); > >>> } > >>> secs = last_secs + t - last_t; > >>> - sprintf(buf + 14, "%02u:%02u.%06lu", secs / 60, secs % 60, res.tv_usec); > >>> + sprintf(buf + 14, "%02u:%02u.%06d", secs / 60, secs % 60, (int)res.tv_usec); > >>> return buf; > >>> } > >>> > >>> @@ -944,10 +945,10 @@ static void monitor(const struct node &node, __u32 monitor_time, const char *sto > >>> } > >>> fprintf(fstore, "# cec-ctl --store-pin\n"); > >>> fprintf(fstore, "# version %d\n", CEC_CTL_VERSION); > >>> - fprintf(fstore, "# start_monotonic %lu.%09lu\n", > >>> - start_monotonic.tv_sec, start_monotonic.tv_nsec); > >>> - fprintf(fstore, "# start_timeofday %lu.%06lu\n", > >>> - start_timeofday.tv_sec, start_timeofday.tv_usec); > >>> + fprintf(fstore, "# start_monotonic %" PRIu64 ".%09" PRIu64 "\n", > >>> + (uint64_t)start_monotonic.tv_sec, (uint64_t)start_monotonic.tv_nsec); > >>> + fprintf(fstore, "# start_timeofday %" PRIu64 ".%06" PRIu64 "\n", > >>> + (uint64_t)start_timeofday.tv_sec, (uint64_t)start_timeofday.tv_usec); > >>> fprintf(fstore, "# log_addr_mask 0x%04x\n", node.log_addr_mask); > >>> fprintf(fstore, "# phys_addr %x.%x.%x.%x\n", > >>> cec_phys_addr_exp(node.phys_addr)); > >>> @@ -986,10 +987,10 @@ static void monitor(const struct node &node, __u32 monitor_time, const char *sto > >>> */ > >>> clock_gettime(CLOCK_MONOTONIC, &start_monotonic); > >>> gettimeofday(&start_timeofday, nullptr); > >>> - fprintf(fstore, "# start_monotonic %lu.%09lu\n", > >>> - start_monotonic.tv_sec, start_monotonic.tv_nsec); > >>> - fprintf(fstore, "# start_timeofday %lu.%06lu\n", > >>> - start_timeofday.tv_sec, start_timeofday.tv_usec); > >>> + fprintf(fstore, "# start_monotonic %" PRIu64 ".%09" PRIu64 "\n", > >>> + (uint64_t)start_monotonic.tv_sec, (uint64_t)start_monotonic.tv_nsec); > >>> + fprintf(fstore, "# start_timeofday %" PRIu64 ".%06" PRIu64 "\n", > >>> + (uint64_t)start_timeofday.tv_sec, (uint64_t)start_timeofday.tv_usec); > >>> fflush(fstore); > >>> start_minute = now; > >>> } > >>> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp > >>> index a7481aea..9b29e3c6 100644 > >>> --- a/utils/cec-follower/cec-follower.cpp > >>> +++ b/utils/cec-follower/cec-follower.cpp > >>> @@ -3,6 +3,7 @@ > >>> * Copyright 2016 Cisco Systems, Inc. and/or its affiliates. All rights reserved. > >>> */ > >>> > >>> +#include <cinttypes> > >>> #include <cstring> > >>> #include <ctime> > >>> #include <sstream> > >>> @@ -354,7 +355,7 @@ void print_timers(struct node *node) > >>> printf("source: %s, ", source.c_str()); > >>> if (t.recording_seq) > >>> printf("rec-seq: 0x%x, ", t.recording_seq); > >>> - printf("needs: %ld %s\n", t.duration, "MB."); /* 1MB per second. */ > >>> + printf("needs: %" PRIu64 " %s\n", (uint64_t)t.duration, "MB."); /* 1MB per second. */ > >>> } > >>> printf("Total media space available for recording: "); > >>> if (node->state.media_space_available >= 0) > >>> diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp > >>> index 3b5c3ce5..cc38f143 100644 > >>> --- a/utils/cec-follower/cec-processing.cpp > >>> +++ b/utils/cec-follower/cec-processing.cpp > >>> @@ -6,6 +6,7 @@ > >>> #include "linux/compiler.h" > >>> > >>> #include <cerrno> > >>> +#include <cinttypes> > >>> #include <ctime> > >>> #include <string> > >>> > >>> @@ -74,7 +75,7 @@ static std::string ts2s(__u64 ts, bool wallclock) > >>> t = res.tv_sec; > >>> s = ctime(&t); > >>> s = s.substr(0, s.length() - 6); > >>> - sprintf(buf, "%03lu", res.tv_usec / 1000); > >>> + sprintf(buf, "%03" PRIu64, (uint64_t)res.tv_usec / 1000); > >>> return s + "." + buf; > >>> } > >>> > >>> diff --git a/utils/keytable/keytable.c b/utils/keytable/keytable.c > >>> index a726921a..ba7c7c4d 100644 > >>> --- a/utils/keytable/keytable.c > >>> +++ b/utils/keytable/keytable.c > >>> @@ -214,6 +214,7 @@ static enum sysfs_protocols parse_sysfs_protocol(const char *name, bool all_allo > >>> return SYSFS_INVALID; > >>> } > >>> > >>> +__attribute__((format(printf, 3, 0))) > >>> static void write_sysfs_protocols(enum sysfs_protocols protocols, FILE *fp, const char *fmt) > >>> { > >>> const struct protocol_map_entry *pme; > >>> diff --git a/utils/libv4l2util/v4l2_driver.c b/utils/libv4l2util/v4l2_driver.c > >>> index 6b6366fa..5cd63fac 100644 > >>> --- a/utils/libv4l2util/v4l2_driver.c > >>> +++ b/utils/libv4l2util/v4l2_driver.c > >>> @@ -15,6 +15,7 @@ > >>> #include <assert.h> > >>> #include <errno.h> > >>> #include <fcntl.h> > >>> +#include <inttypes.h> > >>> #include <stdio.h> > >>> #include <stdlib.h> > >>> #include <string.h> > >>> @@ -174,13 +175,13 @@ static void prt_buf_info(char *name,struct v4l2_buffer *p) > >>> { > >>> struct v4l2_timecode *tc=&p->timecode; > >>> > >>> - printf ("%s: %02ld:%02d:%02d.%08ld index=%d, type=%s, " > >>> + printf ("%s: %02" PRIu64 ":%02d:%02d.%08" PRIu64 " index=%d, type=%s, " > >>> "bytesused=%d, flags=0x%08x, " > >>> "field=%s, sequence=%d, memory=%s, offset=0x%08x, length=%d\n", > >>> - name, (p->timestamp.tv_sec/3600), > >>> + name, (uint64_t)(p->timestamp.tv_sec/3600), > >>> (int)(p->timestamp.tv_sec/60)%60, > >>> (int)(p->timestamp.tv_sec%60), > >>> - p->timestamp.tv_usec, > >>> + (uint64_t)p->timestamp.tv_usec, > >>> p->index, > >>> prt_names(p->type,v4l2_type_names), > >>> p->bytesused,p->flags, > >>> diff --git a/utils/v4l2-tracer/retrace.cpp b/utils/v4l2-tracer/retrace.cpp > >>> index 60d64d8b..010936c0 100644 > >>> --- a/utils/v4l2-tracer/retrace.cpp > >>> +++ b/utils/v4l2-tracer/retrace.cpp > >>> @@ -72,7 +72,7 @@ void retrace_mmap(json_object *mmap_obj, bool is_mmap64) > >>> (long) buf_address_retrace_pointer); > >>> > >>> if (is_verbose() || (errno != 0)) { > >>> - fprintf(stderr, "fd: %d, offset: %ld, ", fd_retrace, off); > >>> + fprintf(stderr, "fd: %d, offset: %lld, ", fd_retrace, (long long)off); > >>> if (is_mmap64) > >>> perror("mmap64"); > >>> else > >> >
