[squid-dev] [PATCH] Coverity-fixes, part 1: EnumIterator

[Alex Rousskov]

On 08/24/2015 01:21 PM, Kinkie wrote:

> On Mon, Aug 24, 2015 at 8:37 PM, Alex Rousskov wrote:
> 
>     On 08/24/2015 12:08 PM, Amos Jeffries wrote:
>     > On 25/08/2015 4:49 a.m., Kinkie wrote:
> 
>     > + * behavior is undefined if the iterator
>     > + * is incremented (or decremented) outside the range representing
>     valid
>     > + * enum symbols
> 
>     I hope that behavior is well defined: For many enums/iterators,
>     iterators are "incremented outside the range representing valid enum
>     symbols" to reach end() and rend(). I asked you about these beyond-range
>     increments, and you assured me that they work fine. We should not add a
>     comment saying otherwise.
> 
> 
> I may have difficulty expressing myself here. The iterators work just
> fine outside the range. Dereferencing them is another matter, as there
> is no valid symbol to cast them to.

Remove that paragraph then. It is a Bad Idea to try to document how
iterators work in general when describing a specific class. Incrementing
or dereferencing end() or equivalent is not going to work well, but we
should not document that.


>     >> + *   zeroth, first, second, onePastLast, fourth
> 
> 
>     Please make onePastLast last.


> I'm trying to highlight the fact that
> the dereferencing will only happen on EnumType::first and
> EnumType::second. Any suggestion on how to clearly express it is welcome.

1. Replace the current enum values with five color names: white, blue,
red, orange, pink.

2. Iterate from blue to orange.

3. Add a do_stuff(v) call comment saying that it will be called with red
and blue values only.

For extra points, order colors by hue :-).


>     You might want to add a comment that EnumRange() cannot be used to
>     iterate whole enums without [end] markers and, hence, all enums that
>     require iteration of the entire range must have markers.


> I'm trying to detail that in the documentation for EnumRangeT. Not
> clearly enough, I guess.


My bad. Sorry I missed that paragraph. It is OK. I would remove the
"Otherwise you will have to ..." suggestion though -- no need to
instruct folks to write bad code.

Note that one _can_ use EnumRange() to iterate whole enums; they just
need a marker to do that. You do not have to polish your comment though.


Thank you,

Alex.