[PATCH] yavta: Format type errors for non x86 arches

[PATCH] yavta: Format type errors for non x86 arches

[Ricardo Ribalda]

mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
for long long ints, which result in some compilation errors.

Lets add some castings to help the compiler deal with this.

We cannot use the Format macro constants ffrom inttypes because they
seem to not be compatible with kernel (__u64 et al) types.

Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
---
 yavta.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/yavta.c b/yavta.c
index d562863..bf54e4f 100644
--- a/yavta.c
+++ b/yavta.c
@@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
 			printf("  %u: %.32s%s\n", menu.index, menu.name,
 			       menu.index == value ? " (*)" : "");
 		else
-			printf("  %u: %lld%s\n", menu.index, menu.value,
+			printf("  %u: %lld%s\n", menu.index,
+			       (long long)menu.value,
 			       menu.index == value ? " (*)" : "");
 	};
 }
@@ -1360,7 +1361,7 @@ static void video_print_control_value(const struct v4l2_query_ext_ctrl *query,
 			printf("0x%08x", ctrl->value);
 			break;
 		case V4L2_CTRL_TYPE_INTEGER64:
-			printf("%lld", ctrl->value64);
+			printf("%lld", (long long)ctrl->value64);
 			break;
 		case V4L2_CTRL_TYPE_STRING:
 			printf("%s", ctrl->string);
@@ -1399,9 +1400,11 @@ static int video_get_control(struct device *dev,
 	}
 
 	if (full) {
-		printf("control 0x%08x `%s' min %lld max %lld step %lld default %lld ",
-			query->id, query->name, query->minimum, query->maximum,
-			query->step, query->default_value);
+		printf("control 0x%08x `%s' min %lld max %lld step %llu default %lld ",
+		       query->id, query->name, (long long)query->minimum,
+		       (long long)query->maximum,
+		       (unsigned long long)query->step,
+		       (long long)query->default_value);
 		if (query->nr_of_dims) {
 			for (i = 0; i < query->nr_of_dims; ++i)
 				printf("[%u]", query->dims[i]);
@@ -2190,13 +2193,16 @@ static int video_do_capture(struct device *dev, unsigned int nframes,
 
 		clock_gettime(CLOCK_MONOTONIC, &ts);
 		get_ts_flags(buf.flags, &ts_type, &ts_source);
-		printf("%u (%u) [%c] %s %u %u B %ld.%06ld %ld.%06ld %.3f fps ts %s/%s\n", i, buf.index,
-			(buf.flags & V4L2_BUF_FLAG_ERROR) ? 'E' : '-',
-			v4l2_field_name(buf.field),
-			buf.sequence, video_buffer_bytes_used(dev, &buf),
-			buf.timestamp.tv_sec, buf.timestamp.tv_usec,
-			ts.tv_sec, ts.tv_nsec/1000, fps,
-			ts_type, ts_source);
+		printf("%u (%u) [%c] %s %u %u B %lld.%06lld %lld.%06lld %.3f fps ts %s/%s\n",
+		       i, buf.index,
+		       (buf.flags & V4L2_BUF_FLAG_ERROR) ? 'E' : '-',
+		       v4l2_field_name(buf.field),
+		       buf.sequence, video_buffer_bytes_used(dev, &buf),
+		       (long long)buf.timestamp.tv_sec,
+		       (long long)buf.timestamp.tv_usec,
+		       (long long)ts.tv_sec,
+		       (long long)(ts.tv_nsec / 1000), fps,
+		       ts_type, ts_source);
 
 		last = buf.timestamp;
 
@@ -2252,8 +2258,9 @@ static int video_do_capture(struct device *dev, unsigned int nframes,
 	bps = size/(ts.tv_nsec/1000.0+1000000.0*ts.tv_sec)*1000000.0;
 	fps = i/(ts.tv_nsec/1000.0+1000000.0*ts.tv_sec)*1000000.0;
 
-	printf("Captured %u frames in %lu.%06lu seconds (%f fps, %f B/s).\n",
-		i, ts.tv_sec, ts.tv_nsec/1000, fps, bps);
+	printf("Captured %u frames in %llu.%06llu seconds (%f fps, %f B/s).\n",
+	       i, (long long)ts.tv_sec, (long long)(ts.tv_nsec / 1000), fps,
+	       bps);
 
 done:
 	video_free_buffers(dev);
-- 
2.30.2

Re: [PATCH] yavta: Format type errors for non x86 arches

[Sakari Ailus]

Hi Ricardo,

Thanks for the patch.

On Tue, Sep 19, 2023 at 04:01:23PM +0200, Ricardo Ribalda wrote:
> mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> for long long ints, which result in some compilation errors.
> 
> Lets add some castings to help the compiler deal with this.
> 
> We cannot use the Format macro constants ffrom inttypes because they
> seem to not be compatible with kernel (__u64 et al) types.
> 
> Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> ---
>  yavta.c | 35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/yavta.c b/yavta.c
> index d562863..bf54e4f 100644
> --- a/yavta.c
> +++ b/yavta.c
> @@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
>  			printf("  %u: %.32s%s\n", menu.index, menu.name,
>  			       menu.index == value ? " (*)" : "");
>  		else
> -			printf("  %u: %lld%s\n", menu.index, menu.value,
> +			printf("  %u: %lld%s\n", menu.index,

Could you instead use PRId64 for this? You can avoid casting to another
type this way. Same for the other cases.

> +			       (long long)menu.value,
>  			       menu.index == value ? " (*)" : "");
>  	};
>  }

-- 
Regards,

Sakari Ailus

Re: [PATCH] yavta: Format type errors for non x86 arches

[Ricardo Ribalda]

Hi Sakari

On Tue, 19 Sept 2023 at 22:07, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> Hi Ricardo,
>
> Thanks for the patch.
>
> On Tue, Sep 19, 2023 at 04:01:23PM +0200, Ricardo Ribalda wrote:
> > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > for long long ints, which result in some compilation errors.
> >
> > Lets add some castings to help the compiler deal with this.
> >
> > We cannot use the Format macro constants ffrom inttypes because they
> > seem to not be compatible with kernel (__u64 et al) types.
> >
> > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> > ---
> >  yavta.c | 35 +++++++++++++++++++++--------------
> >  1 file changed, 21 insertions(+), 14 deletions(-)
> >
> > diff --git a/yavta.c b/yavta.c
> > index d562863..bf54e4f 100644
> > --- a/yavta.c
> > +++ b/yavta.c
> > @@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
> >                       printf("  %u: %.32s%s\n", menu.index, menu.name,
> >                              menu.index == value ? " (*)" : "");
> >               else
> > -                     printf("  %u: %lld%s\n", menu.index, menu.value,
> > +                     printf("  %u: %lld%s\n", menu.index,
>
> Could you instead use PRId64 for this? You can avoid casting to another
> type this way. Same for the other cases.

Already tried this:

@@ -1313,7 +1313,7 @@ static void video_query_menu(struct device *dev,
                        printf("  %u: %.32s%s\n", menu.index, menu.name,
                               menu.index == value ? " (*)" : "");
                else
-                       printf("  %u: %lld%s\n", menu.index, menu.value,
+                       printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
                               menu.index == value ? " (*)" : "");
        };
 }

but gcc was not happy:

yavta.c: In function ‘video_query_menu’:
yavta.c:1316:11: warning: format ‘%ld’ expects argument of type ‘long
int’, but argument 3 has type ‘__s64’ {aka ‘long long int’}
[-Wformat=]
 1316 |    printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
      |           ^~~~~~~~~                            ~~~~~~~~~~
      |                                                    |
      |                                                    __s64 {aka
long long int}
In file included from yavta.c:26:
/usr/include/inttypes.h:57:34: note: format string is defined here
   57 | # define PRId64  __PRI64_PREFIX "d"

So I took the shotgun and fixed it with castings :)


>
> > +                            (long long)menu.value,
> >                              menu.index == value ? " (*)" : "");
> >       };
> >  }
>
> --
> Regards,
>
> Sakari Ailus



-- 
Ricardo Ribalda

Re: [PATCH] yavta: Format type errors for non x86 arches

[Sakari Ailus]

Hi Ricardo,

On Tue, Sep 19, 2023 at 10:10:41PM +0200, Ricardo Ribalda wrote:
> Hi Sakari
> 
> On Tue, 19 Sept 2023 at 22:07, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> >
> > Hi Ricardo,
> >
> > Thanks for the patch.
> >
> > On Tue, Sep 19, 2023 at 04:01:23PM +0200, Ricardo Ribalda wrote:
> > > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > > for long long ints, which result in some compilation errors.
> > >
> > > Lets add some castings to help the compiler deal with this.
> > >
> > > We cannot use the Format macro constants ffrom inttypes because they
> > > seem to not be compatible with kernel (__u64 et al) types.
> > >
> > > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> > > ---
> > >  yavta.c | 35 +++++++++++++++++++++--------------
> > >  1 file changed, 21 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/yavta.c b/yavta.c
> > > index d562863..bf54e4f 100644
> > > --- a/yavta.c
> > > +++ b/yavta.c
> > > @@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
> > >                       printf("  %u: %.32s%s\n", menu.index, menu.name,
> > >                              menu.index == value ? " (*)" : "");
> > >               else
> > > -                     printf("  %u: %lld%s\n", menu.index, menu.value,
> > > +                     printf("  %u: %lld%s\n", menu.index,
> >
> > Could you instead use PRId64 for this? You can avoid casting to another
> > type this way. Same for the other cases.
> 
> Already tried this:
> 
> @@ -1313,7 +1313,7 @@ static void video_query_menu(struct device *dev,
>                         printf("  %u: %.32s%s\n", menu.index, menu.name,
>                                menu.index == value ? " (*)" : "");
>                 else
> -                       printf("  %u: %lld%s\n", menu.index, menu.value,
> +                       printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
>                                menu.index == value ? " (*)" : "");
>         };
>  }
> 
> but gcc was not happy:
> 
> yavta.c: In function ‘video_query_menu’:
> yavta.c:1316:11: warning: format ‘%ld’ expects argument of type ‘long
> int’, but argument 3 has type ‘__s64’ {aka ‘long long int’}
> [-Wformat=]
>  1316 |    printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
>       |           ^~~~~~~~~                            ~~~~~~~~~~
>       |                                                    |
>       |                                                    __s64 {aka
> long long int}
> In file included from yavta.c:26:
> /usr/include/inttypes.h:57:34: note: format string is defined here
>    57 | # define PRId64  __PRI64_PREFIX "d"

I guess I should have expected this...

I'm not sure if it'd be prettier but another option is to use the PRI*
macros and explicitly cast to a standard type.

Using the standard types in the V4L2 header would have avoided this issue.
I wonder if there's anything to be gained by using the kernel types.

Cc Hans, too.

> 
> So I took the shotgun and fixed it with castings :)

:-)

-- 
Regards,

Sakari Ailus

Re: [PATCH] yavta: Format type errors for non x86 arches

[Laurent Pinchart]

On Tue, Sep 19, 2023 at 08:22:57PM +0000, Sakari Ailus wrote:
> Hi Ricardo,
> 
> On Tue, Sep 19, 2023 at 10:10:41PM +0200, Ricardo Ribalda wrote:
> > Hi Sakari
> > 
> > On Tue, 19 Sept 2023 at 22:07, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > >
> > > Hi Ricardo,
> > >
> > > Thanks for the patch.
> > >
> > > On Tue, Sep 19, 2023 at 04:01:23PM +0200, Ricardo Ribalda wrote:
> > > > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > > > for long long ints, which result in some compilation errors.
> > > >
> > > > Lets add some castings to help the compiler deal with this.
> > > >
> > > > We cannot use the Format macro constants ffrom inttypes because they
> > > > seem to not be compatible with kernel (__u64 et al) types.
> > > >
> > > > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> > > > ---
> > > >  yavta.c | 35 +++++++++++++++++++++--------------
> > > >  1 file changed, 21 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/yavta.c b/yavta.c
> > > > index d562863..bf54e4f 100644
> > > > --- a/yavta.c
> > > > +++ b/yavta.c
> > > > @@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
> > > >                       printf("  %u: %.32s%s\n", menu.index, menu.name,
> > > >                              menu.index == value ? " (*)" : "");
> > > >               else
> > > > -                     printf("  %u: %lld%s\n", menu.index, menu.value,
> > > > +                     printf("  %u: %lld%s\n", menu.index,
> > >
> > > Could you instead use PRId64 for this? You can avoid casting to another
> > > type this way. Same for the other cases.
> > 
> > Already tried this:
> > 
> > @@ -1313,7 +1313,7 @@ static void video_query_menu(struct device *dev,
> >                         printf("  %u: %.32s%s\n", menu.index, menu.name,
> >                                menu.index == value ? " (*)" : "");
> >                 else
> > -                       printf("  %u: %lld%s\n", menu.index, menu.value,
> > +                       printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
> >                                menu.index == value ? " (*)" : "");
> >         };
> >  }
> > 
> > but gcc was not happy:
> > 
> > yavta.c: In function ‘video_query_menu’:
> > yavta.c:1316:11: warning: format ‘%ld’ expects argument of type ‘long
> > int’, but argument 3 has type ‘__s64’ {aka ‘long long int’}
> > [-Wformat=]
> >  1316 |    printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
> >       |           ^~~~~~~~~                            ~~~~~~~~~~
> >       |                                                    |
> >       |                                                    __s64 {aka
> > long long int}
> > In file included from yavta.c:26:
> > /usr/include/inttypes.h:57:34: note: format string is defined here
> >    57 | # define PRId64  __PRI64_PREFIX "d"
> 
> I guess I should have expected this...
> 
> I'm not sure if it'd be prettier but another option is to use the PRI*
> macros and explicitly cast to a standard type.
> 
> Using the standard types in the V4L2 header would have avoided this issue.
> I wonder if there's anything to be gained by using the kernel types.

The kernel has defined __s64 as signed int int for a long time now, on
all architectures, at least since

commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf
Author: Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Thu Jan 23 15:53:43 2014 -0800

    asm/types.h: Remove include/asm-generic/int-l64.h

which was merged in v3.14.

According to https://buildd.debian.org/status/package.php?p=yavta,
however, __s64 seems to be defined as long int on some platforms.

/me is puzzled

> Cc Hans, too.
> 
> > So I took the shotgun and fixed it with castings :)
> 
> :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH] yavta: Format type errors for non x86 arches

[Ricardo Ribalda]

Hi Laurent, Hi Sakari



On Tue, 19 Sept 2023 at 23:02, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Tue, Sep 19, 2023 at 08:22:57PM +0000, Sakari Ailus wrote:
> > Hi Ricardo,
> >
> > On Tue, Sep 19, 2023 at 10:10:41PM +0200, Ricardo Ribalda wrote:
> > > Hi Sakari
> > >
> > > On Tue, 19 Sept 2023 at 22:07, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > > >
> > > > Hi Ricardo,
> > > >
> > > > Thanks for the patch.
> > > >
> > > > On Tue, Sep 19, 2023 at 04:01:23PM +0200, Ricardo Ribalda wrote:
> > > > > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > > > > for long long ints, which result in some compilation errors.
> > > > >
> > > > > Lets add some castings to help the compiler deal with this.
> > > > >
> > > > > We cannot use the Format macro constants ffrom inttypes because they
> > > > > seem to not be compatible with kernel (__u64 et al) types.
> > > > >
> > > > > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> > > > > ---
> > > > >  yavta.c | 35 +++++++++++++++++++++--------------
> > > > >  1 file changed, 21 insertions(+), 14 deletions(-)
> > > > >
> > > > > diff --git a/yavta.c b/yavta.c
> > > > > index d562863..bf54e4f 100644
> > > > > --- a/yavta.c
> > > > > +++ b/yavta.c
> > > > > @@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
> > > > >                       printf("  %u: %.32s%s\n", menu.index, menu.name,
> > > > >                              menu.index == value ? " (*)" : "");
> > > > >               else
> > > > > -                     printf("  %u: %lld%s\n", menu.index, menu.value,
> > > > > +                     printf("  %u: %lld%s\n", menu.index,
> > > >
> > > > Could you instead use PRId64 for this? You can avoid casting to another
> > > > type this way. Same for the other cases.
> > >
> > > Already tried this:
> > >
> > > @@ -1313,7 +1313,7 @@ static void video_query_menu(struct device *dev,
> > >                         printf("  %u: %.32s%s\n", menu.index, menu.name,
> > >                                menu.index == value ? " (*)" : "");
> > >                 else
> > > -                       printf("  %u: %lld%s\n", menu.index, menu.value,
> > > +                       printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
> > >                                menu.index == value ? " (*)" : "");
> > >         };
> > >  }
> > >
> > > but gcc was not happy:
> > >
> > > yavta.c: In function ‘video_query_menu’:
> > > yavta.c:1316:11: warning: format ‘%ld’ expects argument of type ‘long
> > > int’, but argument 3 has type ‘__s64’ {aka ‘long long int’}
> > > [-Wformat=]
> > >  1316 |    printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
> > >       |           ^~~~~~~~~                            ~~~~~~~~~~
> > >       |                                                    |
> > >       |                                                    __s64 {aka
> > > long long int}
> > > In file included from yavta.c:26:
> > > /usr/include/inttypes.h:57:34: note: format string is defined here
> > >    57 | # define PRId64  __PRI64_PREFIX "d"
> >
> > I guess I should have expected this...
> >
> > I'm not sure if it'd be prettier but another option is to use the PRI*
> > macros and explicitly cast to a standard type.

I would like to avoid

  printf(" Hello %" PRId64 "\n", (uint64_t) value_s64);

That looks very bad :)

I believe the current casting is the least of the two evils.


> >
> > Using the standard types in the V4L2 header would have avoided this issue.
> > I wonder if there's anything to be gained by using the kernel types.
>
> The kernel has defined __s64 as signed int int for a long time now, on
> all architectures, at least since
>
> commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf
> Author: Geert Uytterhoeven <geert@linux-m68k.org>
> Date:   Thu Jan 23 15:53:43 2014 -0800
>
>     asm/types.h: Remove include/asm-generic/int-l64.h
>
> which was merged in v3.14.
>
> According to https://buildd.debian.org/status/package.php?p=yavta,
> however, __s64 seems to be defined as long int on some platforms.
>
> /me is puzzled

It does not help that __s64 is long long int and PRId64 is "d". I
guess we can say that inttypes and kernel types do not play along?

I guess we need kerntypes.h with proper KPRId64 but that is probably
out of scope here.



Thanks!
>
> > Cc Hans, too.
> >
> > > So I took the shotgun and fixed it with castings :)
> >
> > :-)
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

Re: [PATCH] yavta: Format type errors for non x86 arches

[Sakari Ailus]

Hi Ricardo,

On Wed, Sep 20, 2023 at 02:19:54PM +0200, Ricardo Ribalda wrote:
> Hi Laurent, Hi Sakari
> 
> 
> 
> On Tue, 19 Sept 2023 at 23:02, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > On Tue, Sep 19, 2023 at 08:22:57PM +0000, Sakari Ailus wrote:
> > > Hi Ricardo,
> > >
> > > On Tue, Sep 19, 2023 at 10:10:41PM +0200, Ricardo Ribalda wrote:
> > > > Hi Sakari
> > > >
> > > > On Tue, 19 Sept 2023 at 22:07, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > > > >
> > > > > Hi Ricardo,
> > > > >
> > > > > Thanks for the patch.
> > > > >
> > > > > On Tue, Sep 19, 2023 at 04:01:23PM +0200, Ricardo Ribalda wrote:
> > > > > > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > > > > > for long long ints, which result in some compilation errors.
> > > > > >
> > > > > > Lets add some castings to help the compiler deal with this.
> > > > > >
> > > > > > We cannot use the Format macro constants ffrom inttypes because they
> > > > > > seem to not be compatible with kernel (__u64 et al) types.
> > > > > >
> > > > > > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> > > > > > ---
> > > > > >  yavta.c | 35 +++++++++++++++++++++--------------
> > > > > >  1 file changed, 21 insertions(+), 14 deletions(-)
> > > > > >
> > > > > > diff --git a/yavta.c b/yavta.c
> > > > > > index d562863..bf54e4f 100644
> > > > > > --- a/yavta.c
> > > > > > +++ b/yavta.c
> > > > > > @@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
> > > > > >                       printf("  %u: %.32s%s\n", menu.index, menu.name,
> > > > > >                              menu.index == value ? " (*)" : "");
> > > > > >               else
> > > > > > -                     printf("  %u: %lld%s\n", menu.index, menu.value,
> > > > > > +                     printf("  %u: %lld%s\n", menu.index,
> > > > >
> > > > > Could you instead use PRId64 for this? You can avoid casting to another
> > > > > type this way. Same for the other cases.
> > > >
> > > > Already tried this:
> > > >
> > > > @@ -1313,7 +1313,7 @@ static void video_query_menu(struct device *dev,
> > > >                         printf("  %u: %.32s%s\n", menu.index, menu.name,
> > > >                                menu.index == value ? " (*)" : "");
> > > >                 else
> > > > -                       printf("  %u: %lld%s\n", menu.index, menu.value,
> > > > +                       printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
> > > >                                menu.index == value ? " (*)" : "");
> > > >         };
> > > >  }
> > > >
> > > > but gcc was not happy:
> > > >
> > > > yavta.c: In function ‘video_query_menu’:
> > > > yavta.c:1316:11: warning: format ‘%ld’ expects argument of type ‘long
> > > > int’, but argument 3 has type ‘__s64’ {aka ‘long long int’}
> > > > [-Wformat=]
> > > >  1316 |    printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
> > > >       |           ^~~~~~~~~                            ~~~~~~~~~~
> > > >       |                                                    |
> > > >       |                                                    __s64 {aka
> > > > long long int}
> > > > In file included from yavta.c:26:
> > > > /usr/include/inttypes.h:57:34: note: format string is defined here
> > > >    57 | # define PRId64  __PRI64_PREFIX "d"
> > >
> > > I guess I should have expected this...
> > >
> > > I'm not sure if it'd be prettier but another option is to use the PRI*
> > > macros and explicitly cast to a standard type.
> 
> I would like to avoid
> 
>   printf(" Hello %" PRId64 "\n", (uint64_t) value_s64);
> 
> That looks very bad :)

I actually prefer this. It doesn't look bad either IMO, apart from the PRI*
macros that are always ugly, but most importantly you're explicitly using
64-bit types that work everywhere.

> 
> I believe the current casting is the least of the two evils.
> 
> 
> > >
> > > Using the standard types in the V4L2 header would have avoided this issue.
> > > I wonder if there's anything to be gained by using the kernel types.
> >
> > The kernel has defined __s64 as signed int int for a long time now, on
> > all architectures, at least since
> >
> > commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf
> > Author: Geert Uytterhoeven <geert@linux-m68k.org>
> > Date:   Thu Jan 23 15:53:43 2014 -0800
> >
> >     asm/types.h: Remove include/asm-generic/int-l64.h
> >
> > which was merged in v3.14.
> >
> > According to https://buildd.debian.org/status/package.php?p=yavta,
> > however, __s64 seems to be defined as long int on some platforms.
> >
> > /me is puzzled
> 
> It does not help that __s64 is long long int and PRId64 is "d". I
> guess we can say that inttypes and kernel types do not play along?

I haven't encountered this issue in the past but I also haven't tried
compiling for odd architectures.

> 
> I guess we need kerntypes.h with proper KPRId64 but that is probably
> out of scope here.

This could even depend on the compiler.

I wonder why we aren't using

	typedef uint64_t __u64;

in kernel UAPI headers instead. Including inttypes.h should not be an issue
in 2023 anymore.

This problem is certainly wider in scope than yavta.

-- 
Regards,

Sakari Ailus

Re: [PATCH] yavta: Format type errors for non x86 arches

[Ricardo Ribalda]

Hi Sakari

On Wed, 20 Sept 2023 at 14:40, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> Hi Ricardo,
>
> On Wed, Sep 20, 2023 at 02:19:54PM +0200, Ricardo Ribalda wrote:
> > Hi Laurent, Hi Sakari
> >
> >
> >
> > On Tue, 19 Sept 2023 at 23:02, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > >
> > > On Tue, Sep 19, 2023 at 08:22:57PM +0000, Sakari Ailus wrote:
> > > > Hi Ricardo,
> > > >
> > > > On Tue, Sep 19, 2023 at 10:10:41PM +0200, Ricardo Ribalda wrote:
> > > > > Hi Sakari
> > > > >
> > > > > On Tue, 19 Sept 2023 at 22:07, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > > > > >
> > > > > > Hi Ricardo,
> > > > > >
> > > > > > Thanks for the patch.
> > > > > >
> > > > > > On Tue, Sep 19, 2023 at 04:01:23PM +0200, Ricardo Ribalda wrote:
> > > > > > > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > > > > > > for long long ints, which result in some compilation errors.
> > > > > > >
> > > > > > > Lets add some castings to help the compiler deal with this.
> > > > > > >
> > > > > > > We cannot use the Format macro constants ffrom inttypes because they
> > > > > > > seem to not be compatible with kernel (__u64 et al) types.
> > > > > > >
> > > > > > > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> > > > > > > ---
> > > > > > >  yavta.c | 35 +++++++++++++++++++++--------------
> > > > > > >  1 file changed, 21 insertions(+), 14 deletions(-)
> > > > > > >
> > > > > > > diff --git a/yavta.c b/yavta.c
> > > > > > > index d562863..bf54e4f 100644
> > > > > > > --- a/yavta.c
> > > > > > > +++ b/yavta.c
> > > > > > > @@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
> > > > > > >                       printf("  %u: %.32s%s\n", menu.index, menu.name,
> > > > > > >                              menu.index == value ? " (*)" : "");
> > > > > > >               else
> > > > > > > -                     printf("  %u: %lld%s\n", menu.index, menu.value,
> > > > > > > +                     printf("  %u: %lld%s\n", menu.index,
> > > > > >
> > > > > > Could you instead use PRId64 for this? You can avoid casting to another
> > > > > > type this way. Same for the other cases.
> > > > >
> > > > > Already tried this:
> > > > >
> > > > > @@ -1313,7 +1313,7 @@ static void video_query_menu(struct device *dev,
> > > > >                         printf("  %u: %.32s%s\n", menu.index, menu.name,
> > > > >                                menu.index == value ? " (*)" : "");
> > > > >                 else
> > > > > -                       printf("  %u: %lld%s\n", menu.index, menu.value,
> > > > > +                       printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
> > > > >                                menu.index == value ? " (*)" : "");
> > > > >         };
> > > > >  }
> > > > >
> > > > > but gcc was not happy:
> > > > >
> > > > > yavta.c: In function ‘video_query_menu’:
> > > > > yavta.c:1316:11: warning: format ‘%ld’ expects argument of type ‘long
> > > > > int’, but argument 3 has type ‘__s64’ {aka ‘long long int’}
> > > > > [-Wformat=]
> > > > >  1316 |    printf("  %u: %" PRId64 "%s\n", menu.index, menu.value,
> > > > >       |           ^~~~~~~~~                            ~~~~~~~~~~
> > > > >       |                                                    |
> > > > >       |                                                    __s64 {aka
> > > > > long long int}
> > > > > In file included from yavta.c:26:
> > > > > /usr/include/inttypes.h:57:34: note: format string is defined here
> > > > >    57 | # define PRId64  __PRI64_PREFIX "d"
> > > >
> > > > I guess I should have expected this...
> > > >
> > > > I'm not sure if it'd be prettier but another option is to use the PRI*
> > > > macros and explicitly cast to a standard type.
> >
> > I would like to avoid
> >
> >   printf(" Hello %" PRId64 "\n", (uint64_t) value_s64);
> >
> > That looks very bad :)
>
> I actually prefer this. It doesn't look bad either IMO, apart from the PRI*
> macros that are always ugly, but most importantly you're explicitly using
> 64-bit types that work everywhere.

I am sending a v2, but it looks uglier (not that v1 was super beauty)


>
> >
> > I believe the current casting is the least of the two evils.
> >
> >
> > > >
> > > > Using the standard types in the V4L2 header would have avoided this issue.
> > > > I wonder if there's anything to be gained by using the kernel types.
> > >
> > > The kernel has defined __s64 as signed int int for a long time now, on
> > > all architectures, at least since
> > >
> > > commit 0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf
> > > Author: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Date:   Thu Jan 23 15:53:43 2014 -0800
> > >
> > >     asm/types.h: Remove include/asm-generic/int-l64.h
> > >
> > > which was merged in v3.14.
> > >
> > > According to https://buildd.debian.org/status/package.php?p=yavta,
> > > however, __s64 seems to be defined as long int on some platforms.
> > >
> > > /me is puzzled
> >
> > It does not help that __s64 is long long int and PRId64 is "d". I
> > guess we can say that inttypes and kernel types do not play along?
>
> I haven't encountered this issue in the past but I also haven't tried
> compiling for odd architectures.

Just to make it explicit

PRId64 == ld and _s64 == long long int

is a problem also for x86_64

>
> >
> > I guess we need kerntypes.h with proper KPRId64 but that is probably
> > out of scope here.
>
> This could even depend on the compiler.
>
> I wonder why we aren't using
>
>         typedef uint64_t __u64;
>
> in kernel UAPI headers instead. Including inttypes.h should not be an issue
> in 2023 anymore.
>
> This problem is certainly wider in scope than yavta.
>
> --
> Regards,
>
> Sakari Ailus



-- 
Ricardo Ribalda

[PATCH] yavta: Format type errors for non x86 arches

[Ricardo Ribalda]

mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
for long long ints, which result in some compilation errors.

Lets add some castings and inttypes macros to help the compiler deal with
this.

We have to use the castings, because kernel types (__u64 et al) does not
seem to be compatible with inttypes macros.

Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
---
 yavta.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/yavta.c b/yavta.c
index d562863..d8c9d14 100644
--- a/yavta.c
+++ b/yavta.c
@@ -1313,7 +1313,8 @@ static void video_query_menu(struct device *dev,
 			printf("  %u: %.32s%s\n", menu.index, menu.name,
 			       menu.index == value ? " (*)" : "");
 		else
-			printf("  %u: %lld%s\n", menu.index, menu.value,
+			printf("  %u: %" PRId64 "%s\n", menu.index,
+			       (int64_t)menu.value,
 			       menu.index == value ? " (*)" : "");
 	};
 }
@@ -1360,7 +1361,7 @@ static void video_print_control_value(const struct v4l2_query_ext_ctrl *query,
 			printf("0x%08x", ctrl->value);
 			break;
 		case V4L2_CTRL_TYPE_INTEGER64:
-			printf("%lld", ctrl->value64);
+			printf("%" PRId64, (int64_t)ctrl->value64);
 			break;
 		case V4L2_CTRL_TYPE_STRING:
 			printf("%s", ctrl->string);
@@ -1399,9 +1400,10 @@ static int video_get_control(struct device *dev,
 	}
 
 	if (full) {
-		printf("control 0x%08x `%s' min %lld max %lld step %lld default %lld ",
-			query->id, query->name, query->minimum, query->maximum,
-			query->step, query->default_value);
+		printf("control 0x%08x `%s' min %" PRId64 " max %" PRId64 " step %" PRIu64 " default %" PRId64 " ",
+		       query->id, query->name, (int64_t)query->minimum,
+		       (int64_t)query->maximum, (uint64_t)query->step,
+		       (int64_t)query->default_value);
 		if (query->nr_of_dims) {
 			for (i = 0; i < query->nr_of_dims; ++i)
 				printf("[%u]", query->dims[i]);
@@ -2190,13 +2192,16 @@ static int video_do_capture(struct device *dev, unsigned int nframes,
 
 		clock_gettime(CLOCK_MONOTONIC, &ts);
 		get_ts_flags(buf.flags, &ts_type, &ts_source);
-		printf("%u (%u) [%c] %s %u %u B %ld.%06ld %ld.%06ld %.3f fps ts %s/%s\n", i, buf.index,
-			(buf.flags & V4L2_BUF_FLAG_ERROR) ? 'E' : '-',
-			v4l2_field_name(buf.field),
-			buf.sequence, video_buffer_bytes_used(dev, &buf),
-			buf.timestamp.tv_sec, buf.timestamp.tv_usec,
-			ts.tv_sec, ts.tv_nsec/1000, fps,
-			ts_type, ts_source);
+		printf("%u (%u) [%c] %s %u %u B %" PRId64 ".%06" PRId64 " %" PRId64 ".%06" PRId64 " %.3f fps ts %s/%s\n",
+		       i, buf.index,
+		       (buf.flags & V4L2_BUF_FLAG_ERROR) ? 'E' : '-',
+		       v4l2_field_name(buf.field),
+		       buf.sequence, video_buffer_bytes_used(dev, &buf),
+		       (int64_t)buf.timestamp.tv_sec,
+		       (int64_t)buf.timestamp.tv_usec,
+		       (int64_t)ts.tv_sec,
+		       (int64_t)(ts.tv_nsec / 1000), fps,
+		       ts_type, ts_source);
 
 		last = buf.timestamp;
 
@@ -2252,8 +2257,9 @@ static int video_do_capture(struct device *dev, unsigned int nframes,
 	bps = size/(ts.tv_nsec/1000.0+1000000.0*ts.tv_sec)*1000000.0;
 	fps = i/(ts.tv_nsec/1000.0+1000000.0*ts.tv_sec)*1000000.0;
 
-	printf("Captured %u frames in %lu.%06lu seconds (%f fps, %f B/s).\n",
-		i, ts.tv_sec, ts.tv_nsec/1000, fps, bps);
+	printf("Captured %u frames in %" PRId64 ".%06" PRId64 " seconds (%f fps, %f B/s).\n",
+	       i, (int64_t)ts.tv_sec, (int64_t)(ts.tv_nsec / 1000), fps,
+	       bps);
 
 done:
 	video_free_buffers(dev);
-- 
2.30.2

Re: [PATCH] yavta: Format type errors for non x86 arches

[Sakari Ailus]

Hi Ricardo,

Thanks for the update.

On Wed, Sep 20, 2023 at 02:59:39PM +0200, Ricardo Ribalda wrote:
> mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> for long long ints, which result in some compilation errors.
> 
> Lets add some castings and inttypes macros to help the compiler deal with
> this.
> 
> We have to use the castings, because kernel types (__u64 et al) does not
> seem to be compatible with inttypes macros.
> 
> Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>

It'd be great to address this in the kernel. The kernel UAPI integer types
have been around for a very long time so there could be issues in doing
that, too.

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Kind regards,

Sakari Ailus

Re: [PATCH] yavta: Format type errors for non x86 arches

[Ricardo Ribalda Delgado]

@Laurent Pinchart

Friendly Ping :)

On Wed, Sep 20, 2023 at 10:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> Hi Ricardo,
>
> Thanks for the update.
>
> On Wed, Sep 20, 2023 at 02:59:39PM +0200, Ricardo Ribalda wrote:
> > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > for long long ints, which result in some compilation errors.
> >
> > Lets add some castings and inttypes macros to help the compiler deal with
> > this.
> >
> > We have to use the castings, because kernel types (__u64 et al) does not
> > seem to be compatible with inttypes macros.
> >
> > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
>
> It'd be great to address this in the kernel. The kernel UAPI integer types
> have been around for a very long time so there could be issues in doing
> that, too.
>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> --
> Kind regards,
>
> Sakari Ailus

Re: [PATCH] yavta: Format type errors for non x86 arches

[Ricardo Ribalda]

Hi Laurent

I can see that you updated yavta recently...

Any chance that you can take a look at this?

Thanks!

On Fri, Oct 20, 2023 at 9:07 AM Ricardo Ribalda Delgado
<ricardo@ribalda.com> wrote:
>
> @Laurent Pinchart
>
> Friendly Ping :)
>
> On Wed, Sep 20, 2023 at 10:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote:
> >
> > Hi Ricardo,
> >
> > Thanks for the update.
> >
> > On Wed, Sep 20, 2023 at 02:59:39PM +0200, Ricardo Ribalda wrote:
> > > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > > for long long ints, which result in some compilation errors.
> > >
> > > Lets add some castings and inttypes macros to help the compiler deal with
> > > this.
> > >
> > > We have to use the castings, because kernel types (__u64 et al) does not
> > > seem to be compatible with inttypes macros.
> > >
> > > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> >
> > It'd be great to address this in the kernel. The kernel UAPI integer types
> > have been around for a very long time so there could be issues in doing
> > that, too.
> >
> > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >
> > --
> > Kind regards,
> >
> > Sakari Ailus

Re: [PATCH] yavta: Format type errors for non x86 arches

[Laurent Pinchart]

On Tue, Apr 02, 2024 at 03:59:56PM +0200, Ricardo Ribalda wrote:
> Hi Laurent
> 
> I can see that you updated yavta recently...
> 
> Any chance that you can take a look at this?

Patch applied, sorry for the delay.

> On Fri, Oct 20, 2023 at 9:07 AM Ricardo Ribalda Delgado
> <ricardo@ribalda.com> wrote:
> >
> > @Laurent Pinchart
> >
> > Friendly Ping :)
> >
> > On Wed, Sep 20, 2023 at 10:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > >
> > > Hi Ricardo,
> > >
> > > Thanks for the update.
> > >
> > > On Wed, Sep 20, 2023 at 02:59:39PM +0200, Ricardo Ribalda wrote:
> > > > mipsel64el, ppc64el, ia64, ppc64, sparc64 and x32 have different lenghts
> > > > for long long ints, which result in some compilation errors.
> > > >
> > > > Lets add some castings and inttypes macros to help the compiler deal with
> > > > this.
> > > >
> > > > We have to use the castings, because kernel types (__u64 et al) does not
> > > > seem to be compatible with inttypes macros.
> > > >
> > > > Signed-off-by: Ricardo Ribalda <ricardo@ribalda.com>
> > >
> > > It'd be great to address this in the kernel. The kernel UAPI integer types
> > > have been around for a very long time so there could be issues in doing
> > > that, too.
> > >
> > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >
> > > --
> > > Kind regards,
> > >
> > > Sakari Ailus

-- 
Regards,

Laurent Pinchart