[PATCH] Add support for explicit 32/64 bit numbers

[Dmitri Pal]

Hello

The attached patch adds 4 conversion functions capable
of handling the following types of the values:
int32_t, int64_t, uint32_t and uint64_t

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/12/2010 05:13 PM, Dmitri Pal wrote:
> Hello
>
> The attached patch adds 4 conversion functions capable
> of handling the following types of the values:
> int32_t, int64_t, uint32_t and uint64_t

Ack to the code, but there is one thing to fix in the unit test:
the %lld format specifier for int64_t is correct on 32bit architectures
only, on 64bit the correct specifier is %ld - so the code emits a
compiler warning on 64bit.

Martin suggested that the values should be always casted up to long long
- - it will have some performance penalty, but since this is tracing code
only, it doesn't matter. If we ever need to print exact-sized values in
production code, we can use the C99 set of PRId64/PRId32 macros.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvEYWYACgkQHsardTLnvCWFsQCdEdVR0mpj/9VevTVNq7YebHIr
> zP4AoJxF3TwaN/WMTiFKiVGBYQY8qniz
> =BSVN
> -----END PGP SIGNATURE-----

[Dmitri Pal]

Jakub Hrozek wrote:
> On 04/12/2010 05:13 PM, Dmitri Pal wrote:
> > Hello
> > The attached patch adds 4 conversion functions capable
> > of handling the following types of the values:
> > int32_t, int64_t, uint32_t and uint64_t
>
> Ack to the code, but there is one thing to fix in the unit test:
> the %lld format specifier for int64_t is correct on 32bit architectures
> only, on 64bit the correct specifier is %ld - so the code emits a
> compiler warning on 64bit.
>
> Martin suggested that the values should be always casted up to long long
> it will have some performance penalty, but since this is tracing code
>
> only, it doesn't matter. If we ever need to print exact-sized values in
> production code, we can use the C99 set of PRId64/PRId32 macros.

Can you post the warning?
I do not understand what line generates it.
All macros have explicit type cast.
There should not be a warning.
May be I am using signed when it should be unsigned? Then it should be a
bug...

_______________________________________________
sssd-devel mailing list
[e-mail]
https://fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/13/2010 04:39 PM, Dmitri Pal wrote:
> Jakub Hrozek wrote:
> > On 04/12/2010 05:13 PM, Dmitri Pal wrote:
> > > Hello
> > > The attached patch adds 4 conversion functions capable
> > > of handling the following types of the values:
> > > int32_t, int64_t, uint32_t and uint64_t
> >
> > Ack to the code, but there is one thing to fix in the unit test:
> > the %lld format specifier for int64_t is correct on 32bit architectures
> > only, on 64bit the correct specifier is %ld - so the code emits a
> > compiler warning on 64bit.
> >
> > Martin suggested that the values should be always casted up to long long
> > it will have some performance penalty, but since this is tracing code
> >
> > only, it doesn't matter. If we ever need to print exact-sized values in
> > production code, we can use the C99 set of PRId64/PRId32 macros.
>
> Can you post the warning?
> I do not understand what line generates it.
> All macros have explicit type cast.
> There should not be a warning.
> May be I am using signed when it should be unsigned? Then it should be a
> bug...

The warnings are as follows:

- -----
ini_config_ut.c: In function ‘get_test’:
ini_config_ut.c:1169: warning: format ‘%lld’ expects type ‘long long
int’, but argument 2 has type ‘int64_t’
ini_config_ut.c:1203: warning: format ‘%llu’ expects type ‘long long
unsigned int’, but argument 2 has type ‘uint64_t’
- -----

You will not see them on your 32bit system as the formatting string is
correct for a 32bit system, it is not for a 64bit one, where a '%ld'
would be correct.

Either casting the value "long long" or "unsigned long long" or using
the C99 PRI macros would solve the issue.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvEkFAACgkQHsardTLnvCWkWQCeJf9qj24eAS6BfVICoZ2Xkp0t
> roEAoJ2mofwX0qLSu9OkLYv91g+1bRv3
> =/npj
> -----END PGP SIGNATURE-----

[Dmitri Pal]

Jakub Hrozek wrote:
> On 04/13/2010 04:39 PM, Dmitri Pal wrote:
> > Jakub Hrozek wrote:
> > > On 04/12/2010 05:13 PM, Dmitri Pal wrote:
> > > > Hello
> > > > The attached patch adds 4 conversion functions capable
> > > > of handling the following types of the values:
> > > > int32_t, int64_t, uint32_t and uint64_t
> > >
> > > Ack to the code, but there is one thing to fix in the unit test:
> > > the %lld format specifier for int64_t is correct on 32bit architectures
> > > only, on 64bit the correct specifier is %ld - so the code emits a
> > > compiler warning on 64bit.
> > >
> > > Martin suggested that the values should be always casted up to long
>
> long
> > > it will have some performance penalty, but since this is tracing code
> > >
> > > only, it doesn't matter. If we ever need to print exact-sized values in
> > > production code, we can use the C99 set of PRId64/PRId32 macros.
> >
> > Can you post the warning?
> > I do not understand what line generates it.
> > All macros have explicit type cast.
> > There should not be a warning.
> > May be I am using signed when it should be unsigned? Then it should be a
> > bug...
>
> The warnings are as follows:
>
> ini_config_ut.c: In function get_test:
> ini_config_ut.c:1169: warning: format %lld expects type long long
> int, but argument 2 has type int64_t
> ini_config_ut.c:1203: warning: format %llu expects type long long
> unsigned int, but argument 2 has type uint64_t
>
> You will not see them on your 32bit system as the formatting string is
> correct for a 32bit system, it is not for a 64bit one, where a '%ld'
> would be correct.
>
> Either casting the value "long long" or "unsigned long long" or using
> the C99 PRI macros would solve the issue.

Ah thanks!

Now I see it:

COLOUT(printf("Value: %lld\n", val_int64));

Should be:

COLOUT(printf("Value: %lld\n", (long long)val_int64));

I will send an updated patch shortly

__

_____________________________________________
sssd-devel mailing list
[e-mail]
https://fedorahosted.org/mailman/listinfo/sssd-devel

[Dmitri Pal]

Dmitri Pal wrote:
> Jakub Hrozek wrote:
> > On 04/13/2010 04:39 PM, Dmitri Pal wrote:
> > > Jakub Hrozek wrote:
> > > > On 04/12/2010 05:13 PM, Dmitri Pal wrote:
> > > > > Hello
> > > > > The attached patch adds 4 conversion functions capable
> > > > > of handling the following types of the values:
> > > > > int32_t, int64_t, uint32_t and uint64_t
> > > >
> > > > Ack to the code, but there is one thing to fix in the unit test:
> > > > the %lld format specifier for int64_t is correct on 32bit architectures
> > > > only, on 64bit the correct specifier is %ld - so the code emits a
> > > > compiler warning on 64bit.
> > > >
> > > > Martin suggested that the values should be always casted up to long
> >
> > long
> > > > it will have some performance penalty, but since this is tracing code
> > > >
> > > > only, it doesn't matter. If we ever need to print exact-sized values in
> > > > production code, we can use the C99 set of PRId64/PRId32 macros.
> > >
> > > Can you post the warning?
> > > I do not understand what line generates it.
> > > All macros have explicit type cast.
> > > There should not be a warning.
> > > May be I am using signed when it should be unsigned? Then it should be a
> > > bug...
> >
> > The warnings are as follows:
> >
> > ini_config_ut.c: In function get_test:
> > ini_config_ut.c:1169: warning: format %lld expects type long long
> > int, but argument 2 has type int64_t
> > ini_config_ut.c:1203: warning: format %llu expects type long long
> > unsigned int, but argument 2 has type uint64_t
> >
> > You will not see them on your 32bit system as the formatting string is
> > correct for a 32bit system, it is not for a 64bit one, where a '%ld'
> > would be correct.
> >
> > Either casting the value "long long" or "unsigned long long" or using
> > the C99 PRI macros would solve the issue.
>
> Ah thanks!
>
> Now I see it:
> COLOUT(printf("Value: %lld\n", val_int64));
>
> Should be:
> COLOUT(printf("Value: %lld\n", (long long)val_int64));
>
> I will send an updated patch shortly

The updated patch attached.
> __
>
> sssd-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/sssd-devel

[Stephen Gallagher]

On 04/13/2010 01:09 PM, Dmitri Pal wrote:
> > Ah thanks!
> >
> > Now I see it:
> >  COLOUT(printf("Value: %lld\n", val_int64));
> >
> > Should be:
> >  COLOUT(printf("Value: %lld\n", (long long)val_int64));
> >
> > I will send an updated patch shortly
>
> The updated patch attached.

Ack

[Stephen Gallagher]

On 05/14/2010 07:26 AM, Stephen Gallagher wrote:
> On 04/13/2010 01:09 PM, Dmitri Pal wrote:
> > > Ah thanks!
> > >
> > > Now I see it:
> > >   COLOUT(printf("Value: %lld\n", val_int64));
> > >
> > > Should be:
> > >   COLOUT(printf("Value: %lld\n", (long long)val_int64));
> > >
> > > I will send an updated patch shortly
> >
> > The updated patch attached.
>
> Ack

Dmitri, it looks like this patch doesn't apply cleanly after you split 
your ini_config files. Please rebase this atop master.

[Dmitri Pal]

Stephen Gallagher wrote:
> On 05/14/2010 07:26 AM, Stephen Gallagher wrote:
> > On 04/13/2010 01:09 PM, Dmitri Pal wrote:
> > > > Ah thanks!
> > > >
> > > > Now I see it:
> > > >   COLOUT(printf("Value: %lld\n", val_int64));
> > > >
> > > > Should be:
> > > >   COLOUT(printf("Value: %lld\n", (long long)val_int64));
> > > >
> > > > I will send an updated patch shortly
> > >
> > > The updated patch attached.
> >
> > Ack
>
> Dmitri, it looks like this patch doesn't apply cleanly after you split 
> your ini_config files. Please rebase this atop master.

A new patch attached.

[Stephen Gallagher]

On 05/17/2010 01:23 PM, Dmitri Pal wrote:
> Stephen Gallagher wrote:
> > Dmitri, it looks like this patch doesn't apply cleanly after you split
> > your ini_config files. Please rebase this atop master.
>
> A new patch attached.

Ack and pushed to master.

[Jeff Schroeder]

On Tue, Apr 13, 2010 at 10:02 AM, Dmitri Pal  wrote:
> Jakub Hrozek wrote: > On 04/13/2010 04:39 PM, Dmitri Pal wrote: > > Jakub Hrozek wrote: > >> On 04/12/2010 05:13 PM, Dmitri Pal wrote: > >>> Hello > >>> The attached patch adds 4 conversion functions capable > >>> of handling the following types of the values: > >>> int32_t, int64_t, uint32_t and uint64_t > >> > >> Ack to the code, but there is one thing to fix in the unit test: > >> the %lld format specifier for int64_t is correct on 32bit architectures > >> only, on 64bit the correct specifier is %ld - so the code emits a > >> compiler warning on 64bit. > >> > >> Martin suggested that the values should be always casted up to long > long > >> - it will have some performance penalty, but since this is tracing code > >> only, it doesn't matter. If we ever need to print exact-sized values in > >> production code, we can use the C99 set of PRId64/PRId32 macros. > > Can you post the warning? > > I do not understand what line generates it. > > All macros have explicit type cast. > > There should not be a warning. > > May be I am using signed when it should be unsigned? Then it should be a > > bug... > The warnings are as follows: > ----- > ini_config_ut.c: In function  get_test > ini_config_ut.c:1169: warning: format  %lld  expects type  long long > int , but argument 2 has type  int64_t > ini_config_ut.c:1203: warning: format  %llu  expects type  long long > unsigned int , but argument 2 has type  uint64_t > ----- > You will not see them on your 32bit system as the formatting string is > correct for a 32bit system, it is not for a 64bit one, where a '%ld' > would be correct. > Either casting the value "long long" or "unsigned long long" or using > the C99 PRI macros would solve the issue. Ah thanks! Now I see it:     COLOUT(printf("Value: %lld\n", val_int64)); Should be:     COLOUT(printf("Value: %lld\n", (long long)val_int64)); I will send an updated patch shortly _____________________________________________ sssd-devel mailing list [e-mail] https://fedorahosted.org/mailman/listinfo/sssd-devel Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ _______________________________________________ sssd-devel mailing list [e-mail] https://fedorahosted.org/mailman/listinfo/sssd-devel

Dmitri, I think your mail client (Thunderbird 2.x) ate this email and
stripped out all new lines. This is unreadable.

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/13/2010 01:10 PM, Jeff Schroeder wrote:
> On Tue, Apr 13, 2010 at 10:02 AM, Dmitri Pal  wrote:
> > Jakub Hrozek wrote: > On 04/13/2010 04:39 PM, Dmitri Pal wrote: > > Jakub Hrozek wrote: > >> On 04/12/2010 05:13 PM, Dmitri Pal wrote: > >>> Hello > >>> The attached patch adds 4 conversion functions capable > >>> of handling the following types of the values: > >>> int32_t, int64_t, uint32_t and uint64_t > >> > >> Ack to the code, but there is one thing to fix in the unit test: > >> the %lld format specifier for int64_t is correct on 32bit architectures > >> only, on 64bit the correct specifier is %ld - so the code emits a > >> compiler warning on 64bit. > >> > >> Martin suggested that the values should be always casted up to long > long > >> - it will have some performance penalty, but since this is tracing code > >> only, it doesn't matter. If we ever need to print exact-sized values in > >> production code, we can use the C99 set of PRId64/PRId32 macros. > > Can you post the warning? > > I do not understand what line generates it. > > All macros have explicit type cast

. > > There should not be a warning. > > May be I am using signed when it should be unsigned? Then it should be a > > bug... > The warnings are as follows: > ----- > ini_config_ut.c: In function  get_test > ini_config_ut.c:1169: warning: format  %lld  expects type  long long > int , but argument 2 has type  int64_t > ini_config_ut.c:1203: warning: format  %llu  expects type  long long > unsigned int , but argument 2 has type  uint64_t > ----- > You will not see them on your 32bit system as the formatting string is > correct for a 32bit system, it is not for a 64bit one, where a '%ld' > would be correct. > Either casting the value "long long" or "unsigned long long" or using > the C99 PRI macros would solve the issue. Ah thanks! Now I see it:     COLOUT(printf("Value: %lld\n", val_int64)); Should be:     COLOUT(printf("Value: %lld\n", (long long)val_int64)); I will send an updated patch shortly _____________________________________________ sssd-devel mailing list sssd-devel@li
sts.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ _______________________________________________ sssd-devel mailing list [e-mail] https://fedorahosted.org/mailman/listinfo/sssd-devel
> Dmitri, I think your mail client (Thunderbird 2.x) ate this email and
> stripped out all new lines. This is unreadable.

Jeff, I can only speak for myself, but both of my mail clients
(Thunderbird 3.x and Zimbra web mail) were able to read it just fine.

Occam's Razor says the problem is on your end, I'm afraid.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvEpuMACgkQeiVVYja6o6Nf+QCePug2MmHUz9IqUyi6qnhHjvA9
> d/AAnRQ7wiNBTEaXBv6/lC2bCkFUgTFR
> =8Bjs
> -----END PGP SIGNATURE-----

[Jeff Schroeder]

On Tue, Apr 13, 2010 at 10:16 AM, Stephen Gallagher  wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 04/13/2010 01:10 PM, Jeff Schroeder wrote:
> > On Tue, Apr 13, 2010 at 10:02 AM, Dmitri Pal  wrote:
> > > Jakub Hrozek wrote: > On 04/13/2010 04:39 PM, Dmitri Pal wrote: > > Jakub Hrozek wrote: > >> On 04/12/2010 05:13 PM, Dmitri Pal wrote: > >>> Hello > >>> The attached patch adds 4 conversion functions capable > >>> of handling the following types of the values: > >>> int32_t, int64_t, uint32_t and uint64_t > >> > >> Ack to the code, but there is one thing to fix in the unit test: > >> the %lld format specifier for int64_t is correct on 32bit architectures > >> only, on 64bit the correct specifier is %ld - so the code emits a > >> compiler warning on 64bit. > >> > >> Martin suggested that the values should be always casted up to long > long > >> - it will have some performance penalty, but since this is tracing code > >> only, it doesn't matter. If we ever need to print exact-sized values in > >> production code, we can use the C99 set of PRId64/PRId32 macros. > > Can you post the warning? > > I do not understand what line generates it. > > All macros have explicit type cast
>
> . > > There should not be a warning. > > May be I am using signed when it should be unsigned? Then it should be a > > bug... > The warnings are as follows: > ----- > ini_config_ut.c: In function  get_test > ini_config_ut.c:1169: warning: format  %lld  expects type  long long > int , but argument 2 has type  int64_t > ini_config_ut.c:1203: warning: format  %llu  expects type  long long > unsigned int , but argument 2 has type  uint64_t > ----- > You will not see them on your 32bit system as the formatting string is > correct for a 32bit system, it is not for a 64bit one, where a '%ld' > would be correct. > Either casting the value "long long" or "unsigned long long" or using > the C99 PRI macros would solve the issue. Ah thanks! Now I see it:     COLOUT(printf("Value: %lld\n", val_int64)); Should be:     COLOUT(printf("Value: %lld\n", (long long)val_int64)); I will send an updated patch shortly _____________________________________________ sssd-devel mailing list sssd-devel@li
> sts.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ _______________________________________________ sssd-devel mailing list [e-mail] https://fedorahosted.org/mailman/listinfo/sssd-devel
> > Dmitri, I think your mail client (Thunderbird 2.x) ate this email and
> > stripped out all new lines. This is unreadable.
>
> Jeff, I can only speak for myself, but both of my mail clients
> (Thunderbird 3.x and Zimbra web mail) were able to read it just fine.
>
> Occam's Razor says the problem is on your end, I'm afraid.
>
> Stephen Gallagher
> RHCE 804006346421761

Well it is gmail then. Thanks I'll inform their SRE team.