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: "389 Directory server developer discussion." <389-devel@xxxxxxxxxxxxxxxxxxxxxxx>
> Sent: Monday, March 12, 2018 12:50:50 AM
> Subject: [389-devel] Re: Use of int types in the code base,
> 
> On Fri, 2018-03-09 at 02:35 -0500, Simon Pichugin wrote:
> > 
> > ----- 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.).
> 
> It's a bit optimistic to think 128bit computing will arrive anytime
> soon ;)
Yeah...

> 
> > 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/274823860ac98b153e7f0bc84d979861c4ca8
> > 95f
> > 
> > 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);
> 
> I would do it this way (uint64_t for maxsize). It's better to be
> "absolute" and explicit in our types. size_t only makes sense for array
> indexing, nothing else.
Great! I agree with you.
So I've uploaded the change couple of days ago assuming you'll say so. :)
And if you're okay with the rest of the code, I'll merge it.
https://pagure.io/389-ds-base/pull-request/49595

> 
> > 
> > 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@lists.fedoraproject.o
> > rg
> --
> Thanks,
> 
> William Brown
> _______________________________________________
> 389-devel mailing list -- 389-devel@xxxxxxxxxxxxxxxxxxxxxxx
> To unsubscribe send an email to 389-devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
> 
_______________________________________________
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