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

Alex Rousskov rousskov at measurement-factory.com
Fri Aug 28 00:55:04 UTC 2015


On 08/27/2015 03:51 AM, Kinkie wrote:

>     On 27/08/2015 6:19 a.m., Alex Rousskov wrote:
>     > FYI: If you have missed my last email on this thread, it is
>     > archived at
>     http://lists.squid-cache.org/pipermail/squid-dev/2015-August/003173.html


I cannot tell whether you do not know that you still have the
"onePastLast" problem discussed in that email or you disagree that it is
a problem [worth fixing]. I will not revisit this. Not important to me;
I was just trying to help.


> -                for (op = ICP_INVALID; op < ICP_END; ++op) {
> +                for (icp_opcode op : WholeEnum<icp_opcode>()) {

If we can use "auto" here as you planned and illustrated in the
examples, please do so. Not important.



> + * \see EnumRange(EnumType begin, EnumType one_past_end), WholeEnum


If Doxygen does not require you to list the function arguments, drop
them. Not important.


> -    ICP_END
> -} icp_opcode;
> +    ICP_END,
> +    enumEnd_ = ICP_END

If it is easy to add a "We misuse ICP_END. Do not do this in other
enums." or a similar comment after the assignment, please do so.
Otherwise, others might start copying this pattern, and it is not always
the right thing to do. Not Important.


> +/** generate a continuous range of a whole enum for range-for expressions

Bad copy-paste: s/generate/Class expressing/. Not important.


I have not fully audited your patch, but I do not have any objections to
these changes going in if you are comfortable with them.


Thank you,

Alex.



More information about the squid-dev mailing list