On Fri, 2018-03-09 at 02:35 -0500, Simon Pichugin wrote: > > ----- Original Message ----- > > From: "William Brown" <will...@blackhats.net.au> > > To: "Simon Pichugin" <spich...@redhat.com> > > Cc: "389-devel" <389-devel@lists.fedoraproject.org> > > 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 ;) > 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. > > 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" <will...@blackhats.net.au> > > > > To: "389-devel" <389-devel@lists.fedoraproject.org> > > > > 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@lists.fedoraproject.org > To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.o > rg -- Thanks, William Brown _______________________________________________ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org
