| Message ID | 20230919140123.6277-1-ricardo@ribalda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | yavta: Format type errors for non x86 arches | expand |
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 ? " (*)" : ""); > }; > }
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
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 :) :-)
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 :) > > :-)
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
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.
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
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);
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(-)