On 19/07/2024 09:25, Hans Verkuil wrote:
> Hi Rosen,
>
> On 02/07/2024 21:33, Rosen Penev wrote:
>> Under musl, if a format string has an integer followed by %s, a mismatch
>> between types can cause the second half of the integer to be interpreted
>> by %s.
>>
>> Eg: printf("%d %s", 64bittype, string);
>>
>> will crash, especially on 32-bit big endian.
>>
>> The reason these are cast to __u64 is because time_t and suseconds_t
>> are 64-bit under musl, even on 32-bit platforms. __u64 helps avoid
>> any truncation issues that may or may not arise.
>>
>> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
>> ---
>> utils/cec-follower/cec-follower.cpp | 2 +-
>> utils/libv4l2util/v4l2_driver.c | 6 +++---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
>> index a7481aea..67e0d92b 100644
>> --- a/utils/cec-follower/cec-follower.cpp
>> +++ b/utils/cec-follower/cec-follower.cpp
>> @@ -354,7 +354,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: %llu %s\n", (__u64)t.duration, "MB."); /* 1MB per second. */
>> }
>> printf("Total media space available for recording: ");
>> if (node->state.media_space_available >= 0)
>
> I will drop this change, but I will keep the next change.
>
> I'll explain more in my review of 4/4.
Actually, I decided to do this change differently by changing the
duration type to int instead of using time_t. There is no reason for
using time_t for this, since the duration is limited to 100*3600 seconds
anyway. So that solves this particular issue without the need for a cast.
I decided to accept 4/4 after all. I am a bit afraid that the same problem
will reoccur if new printf statements are added showing timestamps, since
this casting is quite fragile. But I don't really see a better approach.
Regards,
Hans
>
> Regards,
>
> Hans
>
>> diff --git a/utils/libv4l2util/v4l2_driver.c b/utils/libv4l2util/v4l2_driver.c
>> index 6b6366fa..51e97b61 100644
>> --- a/utils/libv4l2util/v4l2_driver.c
>> +++ b/utils/libv4l2util/v4l2_driver.c
>> @@ -174,13 +174,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: %02llu:%02d:%02d.%08llu 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, (__u64)(p->timestamp.tv_sec/3600),
>> (int)(p->timestamp.tv_sec/60)%60,
>> (int)(p->timestamp.tv_sec%60),
>> - p->timestamp.tv_usec,
>> + (__u64)p->timestamp.tv_usec,
>> p->index,
>> prt_names(p->type,v4l2_type_names),
>> p->bytesused,p->flags,
>
>
