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

Kinkie gkinkie at gmail.com
Mon Aug 31 10:10:18 UTC 2015


Hi,
   Rejoining also  diverged thread:

-----
> -                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.
-----
done.

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


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


----
> -    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.
----
Done with a slightly altered text:
"// We misuse ICP_END in stats. Do not do this elsewhere."


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

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

Changed.

Updated patch attached

On Sat, Aug 29, 2015 at 11:12 PM, Kinkie <gkinkie at gmail.com> wrote:
>
>
> On Fri, Aug 28, 2015 at 3:02 AM, Alex Rousskov
> <rousskov at measurement-factory.com> wrote:
>>
>> On 08/27/2015 03:04 AM, Kinkie wrote:
>>
>> >     >     >> + *   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.
>>
>>
>> > Done
>>
>>
>> Not as far as I can see:
>>
>>
>> > + * Typical use:
>> > + * \code
>> > + * enum class EnumType {
>> > + *   zeroth, first, second, onePastLast, fourth
>> > + * };
>
>
> Aha! Found out what the issue was: I had misunderstood where you wanted this
> done, and I had done it in the wrong place.
> Now fixed in the attached patch (in two parts: Makefile.am and everything
> else).
>
> Kinkie



-- 
    Francesco
-------------- next part --------------
A non-text attachment was scrubbed...
Name: coverity-fixes-enumiterator-v4p1.patch
Type: text/x-diff
Size: 16602 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150831/de69bd3b/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: coverity-fixes-enumiterator-makefileam.patch
Type: text/x-diff
Size: 933 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150831/de69bd3b/attachment-0003.patch>


More information about the squid-dev mailing list