Re: Use of int types in the code base,

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




----- Original Message -----
> From: "William Brown" <william@xxxxxxxxxxxxxxxx>
> To: "Simon Pichugin" <spichugi@xxxxxxxxxx>
> Cc: "389-devel" <389-devel@xxxxxxxxxxxxxxxxxxxxxxx>
> Sent: Friday, March 9, 2018 12:30:40 AM
> Subject: Re: Use of int types in the code base,
> 
> On Thu, 2018-03-08 at 17:33 -0500, Simon Pichugin wrote:
> > Hi William!
> > Thank you for the email. It has clarified the things for me. :)
> > I still have one question though.
> > 
> > Do I understand right that I need also to cast types with these types
> > from inttypes.h?
> > So use it not only in the defenitions.
> > - sprintf(buf, "%lu", (long unsigned int)maxsize);
> > + sprintf(buf, "%" PRIu64, (uint64_t)maxsize);
> 
> There is PRIu32 too :)
> 
> The pattern is:
> 
> PRI<sign><size>
> 
> IE
> 
> PRIu64 - uint64_t
> PRId64 - int64_t
> PRIu32 - uint32_t
> PRId32 - int32_t
> 
> Check what type maxsize is to be sure you use the correct PRI type.
> 
> Hope that helps,
Ok. My question was about a bit different thing :)
But now, after some investigation I have clarified something but not all:

So we have size_t that will have the size of the platform (it's 64 now but later we can have 128 bit platform, etc.).
For now, the code will work like this (without any warning)
    size_t maxsize
    sprintf(buf, "%" PRIu64, maxsize);
And, actually, we already have lines like these in the older code added by you:
https://pagure.io/389-ds-base/c/274823860ac98b153e7f0bc84d979861c4ca895f

Or we can change size_t as you've proposed in pull request:
https://pagure.io/389-ds-base/pull-request/49595
"Ahhh I see the issue here. You have maxsize as size_t, but you have PRIu64. Size_t is platform specific width, so you could change maxsize to be a uint64_t instead to avoid this."
    uint64_t maxsize
    sprintf(buf, "%" PRIu64, maxsize);

So could you please guide me here on what coding style do we have in cases like this?

As I understood from coding style guide you gave, we need to use 64bit types mostly (size_t for arrays indexing and 32bit types for compatibility with some apis only)

Thank you,
Simon

> 
> 
> > 
> > Thanks,
> > Simon
> > 
> > ----- Original Message -----
> > > From: "William Brown" <william@xxxxxxxxxxxxxxxx>
> > > To: "389-devel" <389-devel@xxxxxxxxxxxxxxxxxxxxxxx>
> > > Sent: Thursday, March 8, 2018 12:42:03 AM
> > > Subject: Use of int types in the code base,
> > > 
> > > Hi there,
> > > 
> > > http://www.port389.org/docs/389ds/development/coding-style.html#typ
> > > es
> > > 
> > > In a few reviews I still see this sometimes.
> > > 
> > > It's pretty important that we keep moving our quality standard
> > > higher,
> > > and having known type sizes is important to this. Types like int
> > > and
> > > long are unknown sizes until you compile it depending on platform.
> > > 
> > > As a result, it's really important we use the intX_t and uintX_t
> > > types
> > > so we have guarantees of our values. I would encourage the use of
> > > int64_t and uint64_t, because while they are "larger", it's
> > > significantly faster for a modern 64bit system to process these
> > > values
> > > than their 32bit counterparts.
> > > 
> > > Another note is that arrays index by size_t, not 'int', so we
> > > should
> > > always keep this in mind.
> > > 
> > > Finally, because we are using c99 now, this means we should avoid:
> > > 
> > > size_t i = 0;
> > > 
> > > for (i = 0; i < cond; i++) {
> > >     ...
> > > }
> > > 
> > > When we really should scope our values. Scoping is good because it
> > > limits possibility of data corruption to flow and other mistakes
> > > such
> > > as re-use of values. This means:
> > > 
> > > for (size_t i = 0; i < cond; i++) {
> > >     ...
> > > }
> > > 
> > > Thanks!
> > > 
> > > --
> > > Thanks,
> > > 
> > > William Brown
> > > 
> --
> Thanks,
> 
> William Brown
> 
_______________________________________________
389-devel mailing list -- 389-devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to 389-devel-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Directory Announce]     [Fedora Users]     [Older Fedora Users Mail]     [Fedora Advisory Board]     [Fedora Security]     [Fedora Devel Java]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Mentors]     [Fedora Package Review]     [Fedora Art]     [Fedora Music]     [Fedora Packaging]     [CentOS]     [Fedora SELinux]     [Big List of Linux Books]     [KDE Users]     [Fedora Art]     [Fedora Docs]

  Powered by Linux