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

Amos Jeffries squid3 at treenet.co.nz
Mon Aug 24 18:08:59 UTC 2015


On 25/08/2015 4:49 a.m., Kinkie wrote:
> Hi,
>    this is the first of a series of patches stemming from the
> coverity-fixes effort.
> The aim of this first submission  is to implement WholeEnum and EnumRange
> and supporting unit tests.
> 
> Big-context patch attached
> 

in configure.ac:
* no need for two lines, just add the macro straight after the list of
other types.


in src/Makefile.am:
* since this is scoped at the enum operator please remove the
nodist_tests_testX_SOURCES change
 - I will pick that up in the Makefile.am polishing later

* please do omit tests_testEnumIterator_DEPENDENCIES
 - all it does now is harm repeatability of your test results


in src/base/EnumIterator.h:
* the description of requirements for the marker on WholeEnum is a
little bit mind-boggling.
 - a bullet list might be better here if you can find the doxygen syntax
for that. IIRC it was ' - ', but double check if it needs empty lines
between.

* the iterator_type being used in static_cast before the typedef _looks_
very dangerous (even if its fine and works).
 - I would place typedef up the top in their own protected area to avoid
a) depending on implicit compiler behaviour and b) future developer
mistakes/misunderstanding.


I dont see the first-user code that we normally require for accepting
these. I know its coming with a rather big followup, just would be good
if there was a simple use that could be used as exemplar.

Amos



More information about the squid-dev mailing list