[squid-dev] [PATCH] Fetch missing certificates

[Alex Rousskov]

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.