[2/2] v4l-utils: fix compilation with 64-bit time_t

  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@gmail.com>
---
 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(-)
  

ping?

On Mon, Jun 10, 2024 at 2:23 PM Rosen Penev <rosenp@gmail.com> 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@gmail.com>
> ---
>  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);
>         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, "
This 100% fixes crashing under 32-bit musl as it prevents the second
half of time_t being interpreted as %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
> --
> 2.45.2
>
  

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@gmail.com>
> ---
>  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.

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.

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.

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
  

On Sun, Jun 30, 2024 at 1:50 AM Hans Verkuil <hverkuil@xs4all.nl> 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@gmail.com>
> > ---
> >  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.
>
> 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
>
  

On 30/06/2024 22:16, Rosen Penev wrote:
> On Sun, Jun 30, 2024 at 1:50 AM Hans Verkuil <hverkuil@xs4all.nl> 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@gmail.com>
>>> ---
>>>  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?

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.

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
>>
  

On Mon, Jul 1, 2024 at 12:07 AM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 30/06/2024 22:16, Rosen Penev wrote:
> > On Sun, Jun 30, 2024 at 1:50 AM Hans Verkuil <hverkuil@xs4all.nl> 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@gmail.com>
> >>> ---
> >>>  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
> >>
>