On 07/15/2016 02:14 AM, Amos Jeffries wrote: > On 15/07/2016 3:07 a.m., Alex Rousskov wrote: >> On 07/14/2016 05:16 AM, Amos Jeffries wrote: >>> * since certsDownloads is apparently constrained to values up to >>> MaxCertsDownloads. Can we please use a small integer type like uint8_t >>> instead of a full 'int' ?
>> Please do not use uint8_t. I believe this has been discussed already: >> Size-specific types should only be used when there is a specific correct >> size (e.g., we must replicate a system structure) and/or we must >> minimize memory footprint (e.g., lots of structures containing this >> member are expected, especially inside a large container). > Or when we need some certainty about what the size of the data field > actually is. Agreed, although I cannot think of a good example where this third reason would not be already covered by the first two. > Side track: For sizes of payload objects we should be centering on > uint64_t to handle the large objects instead of size_t or int which > can't handle them. Not exactly: * size_t is for payload kept in RAM as a single contiguous blob. * uint64_t is for all other payload without size-specific constraints; such "other" payload is usually "streamed" using multiple constant blobs or recycling the same buffer. > This situation is ambiguous for Downloader though since its > HTTP_REQBUF_SZ can only able to handle 4 KB or smaller objects. There is no ambiguity: Downloader scope is explicitly defined as payload that can be stored in SBuf. Downloader should use whatever size type SBuf is using. If it does not, it is a bug. Christos has corrected your misunderstanding regarding I/O buffer size vs accumulated payload size, but I am focusing on size _types_ here. >> Using "int" is the correct default because it does not force the reader >> to pause and think "Why 8 bits?", > > Not having a fixed size forces the reader to stop and think "what can > int be?" I disagree. A "natural" or "any-reasonable-size" type like "int" does not force a typical reader to stop and think. Only size-specific types like "uint8_t" do. That is one of the primary points behind avoiding size-specific types (the other one is that it is difficult to write correct code with size-constraint types without special tooling that Squid currently lacks). We have discussed that design principle already. If you continue to disagree with the "use int unless you have specific reasons not to" approach, there is nothing more I can do to convince you [given the time constraints], and we would have to live with the hodgepodge of types -- the developer decides on what it the best in their opinion. > Note that Squid is used on some embeded systems where int != 32-bit. Correct "int" usage would cover any reasonable "int" size, which is guaranteed to be large enough to hold [-32767, 32767] range. When used correctly, "int" is used because "int" is "large enough" not because "int has 32 bits". >> is usually faster, will be printed as >> a number on std::ostream (rather than a character), and has fewer >> chances to accidentally overflow when somebody decides to increase >> MaxCertsDownloads (e.g., by making it configurable) without updating >> certsDownloads. > The person doing that needs to know the value ranges they are changing > and what side effects the change will have. Using "int" just obscures > the issues. Using an "int" does not obscure anything IMO. It says "any reasonable count" should work and the developers did not pay any special attention to the true absolute maximum, whatever it is. Using an 8-bit int, on the other hand, says "there are fundamental reasons why we cannot have more than 255 chained downloads and the code was painstakingly written to control and document that limit". However, this just goes back to the original discussion regarding where "int" should be used. This is just one of the examples for that discussion... Alex. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
