cbang build error on epel

[Samuel Rakitničan]

Hi,

I am building CAMotics with cbang builds successfully for i686 and x86_64 platforms, I am trying to make it work for other architectures, in particular armv7 and ppc64 (there is no v8-314 available for others).

So while testing fixes on copr I've stumbled upon an issue of what I am not sure about, the build for ppc64le pass for f24-f27 but fails on epel7.

src/cbang/tar/TarHeader.cpp:226:43: error: format '%llo' expects argument of type 'long long unsigned int', but argument 4 has type 'uint64_t {aka long unsigned int}' [-Werror=format=]
   sprintf(buf, "%0*" PRIo64, length - 1, n);
                                           ^

copr test: https://copr.fedorainfracloud.org/coprs/srakitnican/ppc64le-tests/build/5763...
cbang: https://github.com/CauldronDevelopmentLLC/cbang/

Is this really an issue in cbang?

Best regards,
Samuel Raktiničan

[Björn 'besser82' Esser]

Am 06.07.2017 um 10:14 schrieb Samuel Rakitničan:
> Hi,
>
> I am building CAMotics with cbang builds successfully for i686 and x86_64 platforms, I am trying to make it work for other architectures, in particular armv7 and ppc64 (there is no v8-314 available for others).
>
> So while testing fixes on copr I've stumbled upon an issue of what I am not sure about, the build for ppc64le pass for f24-f27 but fails on epel7.
>
> src/cbang/tar/TarHeader.cpp:226:43: error: format '%llo' expects argument of type 'long long unsigned int', but argument 4 has type 'uint64_t {aka long unsigned int}' [-Werror=format=]
>     sprintf(buf, "%0*" PRIo64, length - 1, n);
>                                             ^
>
> copr test: https://copr.fedorainfracloud.org/coprs/srakitnican/ppc64le-tests/build/5763...
> cbang: https://github.com/CauldronDevelopmentLLC/cbang/
>
> Is this really an issue in cbang?
>
> Best regards,
> Samuel Raktiničan

Patch that line to read `sprintf(buf, "%0*" PRIo64, static_cast<long 
long unsigned int>(length - 1), n);` and you're cool. It might be 
related to how that particular gcc version in EPEL7 inherits those 
atomic data-types.

[Samuel Sieb]

On 07/06/2017 02:28 AM, Björn 'besser82' Esser wrote:
> Am 06.07.2017 um 10:14 schrieb Samuel Rakitničan:
> > src/cbang/tar/TarHeader.cpp:226:43: error: format '%llo' expects 
> > argument of type 'long long unsigned int', but argument 4 has type 
> > 'uint64_t {aka long unsigned int}' [-Werror=format=]
> >     sprintf(buf, "%0*" PRIo64, length - 1, n);
> >                                             ^
>
> Patch that line to read `sprintf(buf, "%0*" PRIo64, static_cast<long 
> long unsigned int>(length - 1), n);` and you're cool. It might be 
> related to how that particular gcc version in EPEL7 inherits those 
> atomic data-types.

Argument 4 is the "n".  The problem is with the definition of "PRIo64. 
There was a recent commit attempting to fix that, but it probably needs 
some adjustment to handle your case.  Check out the lines around line 48 
in that file and see if you can patch it to work in all cases.  The odd 
thing is that it only doesn't work on epel7.  I guess you need to 
compare how old the compiler is on there compared to the Fedora versions.

[Samuel Rakitničan]

> Argument 4 is the "n".  The problem is with the definition of "PRIo64. 
> There was a recent commit attempting to fix that, but it probably needs 
> some adjustment to handle your case.  Check out the lines around line 48 
> in that file and see if you can patch it to work in all cases.  The odd 
> thing is that it only doesn't work on epel7.  I guess you need to 
> compare how old the compiler is on there compared to the Fedora versions.

So Joseph and I managed to fix this for epel7 with following patch:

src/cbang/tar/TarHeader.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/cbang/tar/TarHeader.cpp b/src/cbang/tar/TarHeader.cpp
index 470b37c..50df710 100644
--- a/src/cbang/tar/TarHeader.cpp
+++ b/src/cbang/tar/TarHeader.cpp
@@ -45,7 +45,8 @@
 #endif

#ifndef PRIo64
-#if defined(_M_X64) || (defined(__x86_64__) && !defined(__ILP32__))
+#if defined(_M_X64) || (defined(__x86_64__) && !defined(__ILP32__)) || \
+  defined(__aarch64__) || defined(__ppc64__) || defined(__PPC64__)
 #define PRIo64 "lo"
 #else
 #define PRIo64 "llo"

It seems GCC 4.8 needs these macros defined explicitly, while GCC > 6 inherits it from something else (_M_X64 perhaps?), thoughts?

[Dan Horák]

On Wed, 12 Jul 2017 08:17:46 -0000
Samuel Rakitničan  wrote:
> > Argument 4 is the "n".  The problem is with the definition of
> > "PRIo64. There was a recent commit attempting to fix that, but it
> > probably needs some adjustment to handle your case.  Check out the
> > lines around line 48 in that file and see if you can patch it to
> > work in all cases.  The odd thing is that it only doesn't work on
> > epel7.  I guess you need to compare how old the compiler is on
> > there compared to the Fedora versions.
>
> So Joseph and I managed to fix this for epel7 with following patch:
>
> src/cbang/tar/TarHeader.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/cbang/tar/TarHeader.cpp b/src/cbang/tar/TarHeader.cpp
> index 470b37c..50df710 100644
> --- a/src/cbang/tar/TarHeader.cpp
> +++ b/src/cbang/tar/TarHeader.cpp
> @@ -45,7 +45,8 @@
>  #endif
>
> #ifndef PRIo64
> -#if defined(_M_X64) || (defined(__x86_64__) && !defined(__ILP32__))
> +#if defined(_M_X64) || (defined(__x86_64__) && !defined(__ILP32__))
> || \
> defined(__aarch64__) || defined(__ppc64__) || defined(__PPC64__)
>
> #define PRIo64 "lo"
>  #else
>  #define PRIo64 "llo"
>
> It seems GCC 4.8 needs these macros defined explicitly, while GCC > 6
> inherits it from something else (_M_X64 perhaps?), thoughts?

I guess it depends if (and how) inttypes.h header gets included, it's
provided by glibc.

Dan

[Samuel Rakitničan]

> Patch that line to read `sprintf(buf, "%0*" PRIo64, static_cast<long long unsigned int>(length - 1), n);` and you're cool. It might be related to how that particular gcc version in EPEL7 inherits those atomic data-types.

I've tried with that and this is what I get.

src/cbang/tar/TarHeader.cpp:226:80: error: field width specifier '*' expects argument of type 'int', but argument 3 has type 'long long unsigned int' [-Werror=format=]
   sprintf(buf, "%0*" PRIo64, static_cast<long long unsigned int>(length - 1), n);
                                                                                ^
src/cbang/tar/TarHeader.cpp:226:80: error: format '%llo' expects argument of type 'long long unsigned int', but argument 4 has type 'uint64_t {aka long unsigned int}' [-Werror=format=]

I've also sent a mail to ppc lists seems there is an issue somewhere in that platform.

[Samuel Rakitničan]

> Argument 4 is the "n".

Right, if I adapt the fix for 4th argument, that works.
> The problem is with the definition of "PRIo64. There was a recent commit attempting to fix that, but it probably needs some adjustment to handle your case. Check out the lines around line 48 in that file and see if you can patch it to work in all cases. The odd thing is that it only doesn't work on epel7. I guess you need to compare how old the compiler is on there compared to the Fedora versions.

Not sure how to fix this myself, I've notified program author and he thinks that the code should work [1]. So I've emailed ppc list [2].

[1] https://github.com/CauldronDevelopmentLLC/cbang/issues/21#issuecomment-31377...
[2] https://lists.fedoraproject.org/archives/list/[e-mail]/th...

[Samuel Rakitničan]

> GCC > 6

That should say GCC 6 and newer

[Samuel Rakitničan]

> I guess it depends if (and how) inttypes.h header gets included, it's
> provided by glibc.
> Dan

Find out some more information, do in both Fedora and EPEL __STDC_FORMAT_MACROS is not defined. The difference is in following patch:

https://sourceware.org/git/?p=glibc.git;a=blobdiff;f=sysdeps/generic/inttype...